[lambda] Print warning if traces merging is skipped for Lambda in Step Function

[lym953]

Background

The context injection for Lambda in Step Function requires the Lambda function requires the Lambda definition to be like

    "Lambda Invoke": {
      ...
      "Parameters": {
        ...
        "Payload.$": "$"
      },

If the user has a custom Payload.$ field, context injection will be skipped, but no error/warning message will be printed.

What

Print a warning message if context injection is skipped if:

1. Parameters field doesn't exist for the Lambda function, or
2. there's a custom Payload.$ field

Why

To make it transparent to users what won't work.

How?

Print the messages

Testing

Missing Parameters field

I wasn't able to test this because Parameters field is required when editing the State Machine from AWS Management Console. Should we remove this if-check?

Update: We are going to keep this if-check as @agocs suggested:

This if statement prevents datadog-ci from crashing if parameters is missing for whatever reason.

Custom Payload field

Message printed as expected:

Happy case

datadog-ci stepfunctions instrument finishes with no error

Review checklist

• [ ] Feature or bugfix MUST have appropriate tests (unit, integration)

[agocs]

Oh, here's the answer. Giving an object a name that ends with .$ means "evaluate the value of this object, rename the object to strip off the ending .$, and set the value of this object to be the evaluated value".

https://states-language.net/#appendix-b:~:text=%7B%22output%22%3A%20%22aaff4a450a104cd177d28d18d7485e8cae074b7%22%20%7D-,States.JsonMerge,-Use%20the%C2%A0

So, what's the right way to merge our Payload.$ with the customer's Payload? Probably something like

if (step.Parameters.hasOwnProperty('Payload')) {
  step.Parameters.Payload['State.$'] = '$$.State'
  step.Parameters.Payload['StateMachine.$'] = '$$.StateMachine'
}

et cetera. Like this: https://docs.datadoghq.com/serverless/step_functions/installation/?tab=custom#:~:text=Alternatively%2C%20if%20you%20have%20business%20logic%20defined%20in%20the%20payload%2C%20you%20could%20also%20use%20the%20following%3A

[lym953]

There are a couple other cases we should consider, I think:

• The customer has a Payload object (not Payload.$). I'm not 100% sure what the .$ does in Cloudformation templates or the Stepfunction DSL, but the outcome seems to be that it turned into "Payload" when the step function is executed.
• If the customer has Payload.$ or Payload, should we merge our payload with theirs? Might be a broader discussion.

Good points! I'll do them in separate PRs to keep this PR minimal.

• If the customer has Payload.$ or Payload, should we merge our payload with theirs? Might be a broader discussion.

I think yes.

1. The customer can indeed have custom Payload, e.g. https://github.com/DataDog/serverless-plugin-datadog/issues/517
2. If we want to support Datadog instrumentation for this case, we will have to merge the payload.

[lym953]

Updated the warning message to what @kimi-p drafted. Thanks for writing the message!

[datadog-datadog-prod-us1[bot]]

Datadog Report

Branch report: yiming.luo/fix-step-func Commit report: 4a069a4 Test service: datadog-ci-tests

:white_check_mark: 0 Failed, 364 Passed, 0 Skipped, 1m 26.51s Total duration (2m 1.59s time saved)

[lym953]

I found a bug: if traces merging is already set up and instrumentation command is run again, the warning will also be printed, which is wrong. I will fix this in a separate PR.