-
Notifications
You must be signed in to change notification settings - Fork 45
Darcy.rayner/dd trace support #53
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
Conversation
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.
Looks good to me! Left a few minor comments.
What will this give us? What metrics? Does it just show us how often other methods are called successfully, how often other methods error, and invocation time? If we wanted metrics that are more granular such as when conditional branches of our lambda code are executed, are custom metrics still the best way? How is this different from: https://github.com/DataDog/datadog-lambda-layer-python#distributed-tracing |
@Jwan622, this is support for using datadog's own tracing library, ddtrace, in lambda. Previously we just supported using the X-Ray sdk for tracing, (which we will continue to suport). The main difference is ddtrace is open tracing compliant and has integrations outside of the AWS ecosystem. We also can provide you better sampling controls. |
datadog_lambda/wrapper.py
Outdated
|
||
self.span = None | ||
if dd_tracing_enabled: | ||
self.span = self._create_dd_trace_py_span(context, dd_context) |
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.
Nits, i feel this function is more about "creating a span for the function execution" rather than "creating a dd trace py span". Perhaps create_function_execution_span
or something better?
datadog_lambda/wrapper.py
Outdated
} | ||
args = { | ||
"service": self.function_name, | ||
"resource": self.handler_name, |
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.
To be future proof, let' use function_name
as resource.
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 does this PR do?
Adds beta dd-trace support to python lambda layer. Two new environment variables are added DD_TRACE_ENABLED (enables dd-trace), and DD_MERGE_XRAY_TRACES, (reparents dd-trace traces to xray trace). Currently waiting for latest version of dd-trace-py with required changes to be available.
Motivation
Testing Guidelines
I modified the integration tests to capture dd trace output
Additional Notes
Anything else we should know when reviewing?
Checklist