Skip to content

fix: Step Functions contract — SLA cancel, run IDs, deadlines, ASL guard - #84

Merged
dwsmith1983 merged 11 commits into
mainfrom
fix/sfn-contract
Sep 11, 2026
Merged

fix: Step Functions contract — SLA cancel, run IDs, deadlines, ASL guard#84
dwsmith1983 merged 11 commits into
mainfrom
fix/sfn-contract

Conversation

@dwsmith1983

Copy link
Copy Markdown
Owner

Summary

Fixes four defects that made Step Functions executions die with uncatchable States.Runtime errors or mis-score SLA verdicts, and adds an executable Go↔ASL contract test so the state machine can never again dereference a path the Go producers do not emit.

  • SLA cancel states failed every SLA-configured execution. CancelSLASchedules / CancelSLAOnCompleteTriggerFailure dereferenced $.config.sla.maxDuration, $.config.sla.deadline and $.sensorArrivalAt, all omitempty. New SFNInput/SFNConfig/SFNSLA types emit every dereferenced key unconditionally; config.sla stays optional because the Choice states guard it with IsPresent. Both cancel states now also forward timezone.
  • Run IDs were lost for 5 of 6 async trigger types. ExtractRunID now recognises sfn_execution_arn, emr_step_id, emr_sl_job_run_id, airflow_dag_run_id, databricks_run_id (plus legacy keys); OrchestratorOutput.RunID is always marshaled.
  • Absolute SLA deadlines rolled forward a day when past, so late runs were scored SLA_MET. CalculateAbsoluteDeadline now anchors to the execution date; roll-forward only applies when no date is supplied (and uses AddDate for DST). The shared T+1 rule for sensor-triggered daily pipelines is extracted into ResolveSLADate and applied on the watchdog, cancel and dry-run paths.
  • Orchestrator evaluate/trigger outputs violated the ASL contract. evaluate always emits status (and now logs storage failures); trigger config failures return a Lambda error so TriggerRetryExhausted releases the lock instead of leaving it RUNNING until TTL.
  • New tests in the live packages (internal/lambda/orchestrator, internal/lambda/sla, deploy/) via a small exported internal/store/storetest fake; three wall-clock time-bomb tests now inject NowFunc (one had been failing since 2026-06-16).
  • The cancel path no longer publishes a fabricated SLA_MET when no deadline could be determined.

Release notes

  • Verdicts for pipelines with a non-UTC sla.timezone are now computed in that zone.
  • Absolute SLA deadlines no longer roll forward to the next day when already past.
  • In-flight executions started before deploy are unaffected by the state-machine change (the old input already carried timezone).
  • The sla-monitor reconcile mode has no production invoker today and is not a safety net for deadlines missed while the watchdog was down.

Test plan

  • make audit (golangci-lint 0 issues, go test -race ./...) green at HEAD
  • TestASL_EveryDereferencedPathResolves verified red against the pre-fix sfn.go and evaluate.go, green after
  • New table tests: ExtractRunID over all 9 trigger metadata shapes; CalculateAbsoluteDeadline explicit-date and DST cases; ResolveSLADate; cancel verdicts for sensor-daily and cron pipelines
  • After deploy: confirm an SLA-configured pipeline reaches Done and publishes SLA_MET/SLA_BREACH; confirm an EMR/SFN-triggered pipeline reaches CheckJob

Follow-ups (tracked separately)

Stream-router SequenceNumber + event-name filtering; Terraform/build zips and watchdog IAM; porting the root internal/lambda test suite to the live sub-packages and deleting the deprecated copies; EMR Serverless / Databricks request shapes and authenticated status polling; scheduling and timezone fixes (including breach alerts for cron pipelines first seen after their deadline).

CalculateAbsoluteDeadline rolled an explicit execution date forward by a
day (or an hour) whenever the deadline had already passed. That made
sla-monitor cancel publish SLA_MET for runs that finished late, made the
reconcile breach branch unreachable, and made the watchdog schedule
breach alerts a day late. Roll-forward now only applies when no execution
date was supplied, and uses AddDate so the wall clock survives DST.

Tests that depended on the wall clock now inject NowFunc.
ExtractRunID only matched runId/jobRunId/glue_job_run_id/executionArn/
stepId/dagRunId, but the sfn, emr, emr-serverless, airflow and databricks
executors emit sfn_execution_arn, emr_step_id, emr_sl_job_run_id,
airflow_dag_run_id and databricks_run_id. The resulting empty runId was
omitted from the Lambda result, so the CheckJob state raised
States.Runtime after the external job had already been launched.

OrchestratorOutput.RunID is now always marshaled and an empty metadata
map takes the same sync-sentinel path as nil.
internal/lambda/orchestrator and deploy both need a store.DynamoAPI
double, but every existing mock lives in a package-private _test.go file.
storetest exposes a hook-based fake plus a *store.Store constructor and is
imported only from test files.
handleEvaluate returned an output without status on storage failures, so
the IsReady Choice dereferenced a missing $.evaluateResult.status and
raised States.Runtime. It now always emits a status ('error' routes back
into the evaluation loop).

handleTrigger returned a partial result with a nil error on config
failures. HasTriggerResult saw IsPresent=true, CheckJob dereferenced the
missing runId, and the execution died with the TRIGGER# lock stuck
RUNNING until TTL. Those paths now return a Lambda error so the Trigger
Retry/Catch routes to TriggerRetryExhausted, which releases the lock.

The deprecated root copy keeps the old contract; its tests assert it and
porting them is a separate batch.
types.SLAConfig marks every field omitempty and sensorArrivalAt was also
omitempty, so CancelSLASchedules and CancelSLAOnCompleteTriggerFailure
referenced JSONPaths that were absent for every real config: an
absolute-only SLA has no maxDuration, a relative-only SLA has no
deadline, and sensorArrivalAt was only set for relative SLAs with a
recorded arrival. The resulting States.Runtime is not catchable by
Catch: States.ALL, so every SLA-configured execution failed after
CompleteTrigger.

The SFN input now uses dedicated SFNInput/SFNConfig/SFNSLA types that
emit every dereferenced key unconditionally; config.sla itself stays
omitempty because two Choice states guard it with IsPresent. Both cancel
states now also forward timezone, which handleSLACancel needs when it
recomputes deadlines.
Collects every Parameters reference (including intrinsic arguments), Wait
SecondsPath, Choice comparator path and unguarded Choice Variable from the
rendered state machine and resolves each one against the document Step
Functions actually holds: the marshaled SFNInput from the real builder
plus the marshaled outputs of the real orchestrator handlers. Paths behind
an IsPresent guard may be absent; Parameters paths may not.

Verified red against the pre-fix sfn.go and evaluate.go.
Sensor-triggered daily pipelines run T+1 (data for date D arrives on
D+1), a rule the watchdog already applied when proactively scheduling
SLA alerts. handleSLACancel and the dry-run SLA projection instead
recomputed the deadline from the data date D. Now that an explicit
date no longer rolls forward past a missed deadline, this made cancel
publish a false SLA_BREACH for pipelines that finished on D+1 before
their deadline.

Extract the watchdog's rule into lambda.ResolveSLADate and share it
across the watchdog, sla.handleSLACancel, and
stream.publishDryRunSLAProjection. Only the recalculation input's date
is shifted; the schedule name, GetTrigger lookup, and published event
still use the caller's original date.

Also guard against a fabricated SLA_MET verdict: if neither an
absolute nor a relative deadline can be determined (e.g. a relative
SLA with no recorded sensor arrival), cancel now logs and skips
publishing a verdict instead of defaulting to SLA_MET.
handleEvaluate returned status: "error" on GetConfig failure, missing
config, and GetAllSensors failure with no server-side log line, so the
only record of the failure was the Step Functions execution history.
Log each at error level with the pipeline ID and underlying error
before returning.
@github-actions github-actions Bot added tests Test changes lambda Lambda handlers deploy Deployment and ASL docs Documentation labels Sep 11, 2026
@dwsmith1983 dwsmith1983 self-assigned this Sep 11, 2026
@dwsmith1983
dwsmith1983 merged commit e25b7b2 into main Sep 11, 2026
3 checks passed
@dwsmith1983
dwsmith1983 deleted the fix/sfn-contract branch September 11, 2026 16:56
@dwsmith1983 dwsmith1983 mentioned this pull request Sep 11, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy Deployment and ASL docs Documentation lambda Lambda handlers tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant