Enable step function tracing at forwarder level #831

avedmala · 2024-07-30T20:23:51Z

What does this PR do?

This PR adds logic to allow users to enable tracing for all their Step Functions via the dd_step_function_trace_enabled tag

This tag can be set via the DdStepFunctionTraceEnabled CloudFormation parameter which is used to set the DD_STEP_FUNCTION_TRACE_ENABLED env var

Link to log with the new tag and screenshot of the tag

Motivation

Testing Guidelines

Additional Notes

Types of changes

Bug fix
New feature
Breaking change
Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

This PR's description is comprehensive
This PR contains breaking changes that are documented in the description
This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
This PR impacts documentation, and it has been updated (or a ticket has been logged)
This PR's changes are covered by the automated tests
This PR collects user input/sensitive content into Datadog
This PR passes the integration tests (ask a Datadog member to run the tests)
This PR passes the unit tests
This PR passes the installation tests (ask a Datadog member to run the tests)

aws/logs_monitoring/template.yaml

nine5two7 · 2024-07-31T20:53:30Z

aws/logs_monitoring/steps/enrichment.py


+        # Set tracing behavior for all step functions
+        if DD_STEP_FUNCTION_TRACE_ENABLED:
+            event[DD_CUSTOM_TAGS] = ",".join(


Null check for event[DD_CUSTOM_TAGS] ?

Will be moving this logic as per Kimi's comment and adding a null check on the entry

nine5two7 · 2024-07-31T20:58:07Z

aws/logs_monitoring/template.yaml

+  SetDdStepFunctionTraceEnabled:
+    Fn::Equals:
+      - Ref: DdStepFunctionTraceEnabled
+      - true


What will be the default value in CloudFormation console (shown as below) when customer install a new DD forwarder?
It should be default false, but customers can set it to be true if they want.

The console will use the default value in Parameters: section, so in this case it would be false.

https://github.com/DataDog/datadog-serverless-functions/pull/831/files#diff-7c5bda646bf6435d9285913888d0473f0ac98ed0fdd025ae5d9ea357ad9b3ffaR164-R166

This should be the default behavior, here is the flow

Template has the default for the DdStepFunctionTraceEnabled to be false

SetDdStepFunctionTraceEnabled condition will then be false because DdStepFunctionTraceEnabled != true

DD_STEP_FUNCTION_TRACE_ENABLED will be AWS::NoValue, which means the env var won't be set

aws/logs_monitoring/README.md

kimi-p

Sorry that I forgot to share the original PR with you before. Could you also add a test like this test from the previous PR?

https://github.com/DataDog/datadog-serverless-functions/pull/618/files#diff-caaaee34b52da49a4f9f45b1d6f59c91d60b24d7570e75aab8876df91f5324a7R874

aws/logs_monitoring/README.md

kimi-p · 2024-07-31T22:47:42Z

aws/logs_monitoring/steps/enrichment.py

+        if DD_STEP_FUNCTION_TRACE_ENABLED:
+            event[DD_CUSTOM_TAGS] = ",".join(
+                [event[DD_CUSTOM_TAGS]] + ["dd_step_function_trace_enabled:true"]
+            )


I think we should to add the step functions specific logic to handle_step_function_source().

datadog-serverless-functions/aws/logs_monitoring/steps/handlers/awslogs_handler.py

Line 161 in 93c2281

def handle_step_function_source(self):

We'd also want to do the empty value (e.g. "") check like this one:

datadog-serverless-functions/aws/logs_monitoring/steps/handlers/awslogs_handler.py

Line 181 in 93c2281

if not self.metadata[DD_CUSTOM_TAGS]

I moved the logic to the handle_step_function_source() but I used a slightly different approach with the join so I don't think need the empty check

kimi-p · 2024-07-31T23:02:05Z

aws/logs_monitoring/template.yaml

+  SetDdStepFunctionTraceEnabled:
+    Fn::Equals:
+      - Ref: DdStepFunctionTraceEnabled
+      - true


The console will use the default value in Parameters: section, so in this case it would be false.

https://github.com/DataDog/datadog-serverless-functions/pull/831/files#diff-7c5bda646bf6435d9285913888d0473f0ac98ed0fdd025ae5d9ea357ad9b3ffaR164-R166

nine5two7 · 2024-08-01T14:19:01Z

aws/logs_monitoring/steps/handlers/awslogs_handler.py

                + ",".join(formatted_stepfunctions_tags)
            )

+        if DD_STEP_FUNCTION_TRACE_ENABLED:


nine5two7 · 2024-08-01T14:26:36Z

LGTM.
Nit: We typically use the term 'Step Functions' instead of 'Step Function' to be consistent with AWS naming conventions.

nine5two7

LGTM

Initial logic to add fetch and add the lambda tag

eddcf5d

github-actions bot added the aws label Jul 30, 2024

avedmala added 3 commits July 30, 2024 17:14

Set tag in env var to avoid fetching everytime

10902be

Moved tag to cloudformation param

b2fe35e

Fixed readme, moved param under advanced param

cff1a0e

avedmala commented Jul 31, 2024

View reviewed changes

aws/logs_monitoring/template.yaml Show resolved Hide resolved

avedmala added 2 commits July 31, 2024 10:43

Removed context passing

3806fc5

Removed unused import

f330f88

nine5two7 reviewed Jul 31, 2024

View reviewed changes

aws/logs_monitoring/template.yaml Outdated Show resolved Hide resolved

avedmala added 2 commits July 31, 2024 13:58

Fixed template condition

2f0555c

Update readme language for consistency

994d0b5

avedmala marked this pull request as ready for review July 31, 2024 18:08

avedmala requested a review from a team as a code owner July 31, 2024 18:08

avedmala added 2 commits July 31, 2024 14:55

Moved tagging outside of just lambda

9e13267

black formatting

cbcef0c

This was referenced Jul 31, 2024

[WIP] Enable step function tracing at forwarder level #832

Closed

Enable step function tracing at forwarder level DataDog/documentation#24416

Merged

nine5two7 reviewed Jul 31, 2024

View reviewed changes

aws/logs_monitoring/README.md Outdated Show resolved Hide resolved

kimi-p reviewed Jul 31, 2024

View reviewed changes

avedmala added 2 commits August 1, 2024 09:33

Updated language to specify all step functions

db81dc0

Updated language to call it tracing instead of l2t

3c62195

ViBiOh approved these changes Aug 1, 2024

View reviewed changes

Moved step functions specific logic to handle_step_function_source()

2e7ff49

nine5two7 reviewed Aug 1, 2024

View reviewed changes

nine5two7 approved these changes Aug 1, 2024

View reviewed changes

avedmala added 2 commits August 1, 2024 11:26

Updated tests to check for new tag

dbd763b

renamed to 'Step Functions', missing 's' before

587c198

avedmala changed the title ~~[SVLS-5044] Enable step function tracing at forwarder level~~ Enable step function tracing at forwarder level Aug 1, 2024

avedmala merged commit d507e14 into master Aug 1, 2024

avedmala deleted the avedmala/step-fn-tracing branch August 1, 2024 20:56

nine5two7 mentioned this pull request Aug 5, 2024

Step Functions updates DataDog/documentation#24438

Merged

1 task

Uh oh!

Enable step function tracing at forwarder level #831

Enable step function tracing at forwarder level #831

Uh oh!

Conversation

avedmala commented Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Testing Guidelines

Additional Notes

Types of changes

Check all that apply

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kimi-p left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nine5two7 commented Aug 1, 2024

Uh oh!

nine5two7 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

avedmala commented Jul 30, 2024 •

edited

Loading