fix: Step Functions contract — SLA cancel, run IDs, deadlines, ASL guard - #84
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes four defects that made Step Functions executions die with uncatchable
States.Runtimeerrors 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.CancelSLASchedules/CancelSLAOnCompleteTriggerFailuredereferenced$.config.sla.maxDuration,$.config.sla.deadlineand$.sensorArrivalAt, allomitempty. NewSFNInput/SFNConfig/SFNSLAtypes emit every dereferenced key unconditionally;config.slastays optional because the Choice states guard it withIsPresent. Both cancel states now also forwardtimezone.ExtractRunIDnow recognisessfn_execution_arn,emr_step_id,emr_sl_job_run_id,airflow_dag_run_id,databricks_run_id(plus legacy keys);OrchestratorOutput.RunIDis always marshaled.SLA_MET.CalculateAbsoluteDeadlinenow anchors to the execution date; roll-forward only applies when no date is supplied (and usesAddDatefor DST). The shared T+1 rule for sensor-triggered daily pipelines is extracted intoResolveSLADateand applied on the watchdog, cancel and dry-run paths.evaluatealways emitsstatus(and now logs storage failures); trigger config failures return a Lambda error soTriggerRetryExhaustedreleases the lock instead of leaving itRUNNINGuntil TTL.internal/lambda/orchestrator,internal/lambda/sla,deploy/) via a small exportedinternal/store/storetestfake; three wall-clock time-bomb tests now injectNowFunc(one had been failing since 2026-06-16).SLA_METwhen no deadline could be determined.Release notes
sla.timezoneare now computed in that zone.timezone).reconcilemode 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 HEADTestASL_EveryDereferencedPathResolvesverified red against the pre-fixsfn.goandevaluate.go, green afterExtractRunIDover all 9 trigger metadata shapes;CalculateAbsoluteDeadlineexplicit-date and DST cases;ResolveSLADate; cancel verdicts for sensor-daily and cron pipelinesDoneand publishesSLA_MET/SLA_BREACH; confirm an EMR/SFN-triggered pipeline reachesCheckJobFollow-ups (tracked separately)
Stream-router
SequenceNumber+ event-name filtering; Terraform/build zips and watchdog IAM; porting the rootinternal/lambdatest 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).