DataDog / datadog-ci

Use Datadog from your CI.
https://datadoghq.com
Apache License 2.0
129 stars 55 forks source link

Step Function context is not injected into lambda payload #1429

Closed brainsiq closed 2 months ago

brainsiq commented 2 months ago

Bug description

With a trivial step function definition containing only a single Lambda task, running the instrument command with propagate/merge flags does not result in any update of the Lambda payload in the definition.

Describe what you expected

Payload should be replaced with States.JsonMerge($$, $, false)

Steps to reproduce the issue

Example step function ASL:

{
  "Comment": "An example step function",
  "StartAt": "ExampleLambda",
  "States": {
    "ExampleLambda": {
      "Type": "Task",
      "Resource": "arn:aws:states:::lambda:invoke",
      "OutputPath": "$.Payload",
      "Parameters": {
        "Payload.$": "$",
        "FunctionName": "arn:aws:lambda:{region}:{account}:function:{name}:{alias}"
      },
      "End": true
    }
  }
}

Command:

datadog-ci stepfunctions instrument \
  --step-function {function-arn} \
  --forwarder {forwarder-arn} \
  --propagate-upstream-trace \
  -mlt

Additional context

OS: Mac OS Sonoma Node version: 20 Datadog-CI version: 2.42.0

Command

stepfunctions instrument

brainsiq commented 2 months ago

I cloned the project and think I have managed to determine the cause: https://github.com/DataDog/datadog-ci/blob/d74e427ef5a569569fdd4e4041545bdccb0df4b5/src/commands/stepfunctions/helpers.ts#L96-L98

I believe it will only work when the state machine uses arn:aws:states:::states:startExecution to execute another state machine, which ensures that after the second assignment the variable still contains true. I think the fix is replacing L98 with:

definitionHasBeenUpdated = definitionHasBeenUpdated || injectContextForStepFunctions(step)

I executed this modified code and it performed the update of the Lambda on the same step function definition. I haven't yet been able to verify that the general feature works when the definition is updated.

nine5two7 commented 2 months ago

@brainsiq It is expected to inject execution context in the payload when the Step Function invokes either Lambda or another Step Function. We are currently working on a fix for the CI.