-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Explicit trace ID propagation for SFN w/o Hashing #537
Conversation
Add route tags
@@ -415,7 +452,9 @@ def is_legacy_lambda_step_function(event): | |||
return False | |||
|
|||
event = event.get("Payload") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -254,6 +256,14 @@ def parse_event_source_arn(source: _EventSource, event: dict, context: Any) -> s | |||
if source.event_type == EventTypes.CLOUDWATCH_EVENTS and event.get("resources"): | |||
return event.get("resources")[0] | |||
|
|||
# todo: testme | |||
# e.g. arn:aws:states:us-east-1:123456789012:stateMachine:stateMachineName | |||
if source.event_type == EventTypes.STEPFUNCTIONS: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return "" | ||
|
||
|
||
def _generate_sfn_parent_id(context: dict) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created these helpers to pull hashing and hex related details out of extract_context_from_step_functions
as it was getting pretty verbose
datadog_lambda/tracing.py
Outdated
meta = {} | ||
dd_data = event.get("_datadog") | ||
|
||
if dd_data and dd_data.get("serverless-version") == "v2": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between v1 and v2? Should we start from version v2 or v1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no v1, I think we can start from v1 instead if we want
I think @kimi-p suggested v2, maybe because the "legacy" context is implied to be v1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, "V2" may be somewhat confusing to customers because there is no explicit reference to "V1" at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question regarding the legacy lambda check that I think we need to update, otherwise LGTM! Thank you!
Co-authored-by: kimi <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joeyzhao2018 Could you take a look to confirm that we are using the right extraction pattern in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What does this PR do?
Forked off of #526, this PR will use context objects to generate traceIDs rather than generating them form hashes
Add logic to extract trace context from
_datadog
for Step Functions cases where...x-datadog-trace-id
but we need to compute thex-datadog-parent-id
using parent SFN context objectx-datadog-trace-id
andx-datadog-parent-id
fromRootExecutionId
and the parent SFN context object_datadog
and we instead figure out context from the Execution and State infoAlso fixed the trigger tags parsing for our use case!
Motivation
Testing Guidelines
Additional Notes
Types of Changes
Check all that apply