-
Notifications
You must be signed in to change notification settings - Fork 318
Pass Lambda Request ID to Extension #8814
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| import static net.bytebuddy.matcher.ElementMatchers.isMethod; | ||
| import static net.bytebuddy.matcher.ElementMatchers.takesArgument; | ||
|
|
||
| import com.amazonaws.services.lambda.runtime.Context; | ||
| import com.amazonaws.services.lambda.runtime.RequestHandler; | ||
| import com.google.auto.service.AutoService; | ||
| import datadog.trace.agent.tooling.Instrumenter; | ||
|
|
@@ -79,20 +80,24 @@ public static class ExtensionCommunicationAdvice { | |
| @OnMethodEnter | ||
| static AgentScope enter( | ||
| @This final Object that, | ||
| @Advice.Argument(0) final Object event, | ||
| @Advice.Argument(0) final Object in, | ||
| @Advice.Argument(1) final Object out, | ||
| @Advice.Argument(2) final Context awsContext, | ||
|
Comment on lines
-82
to
+85
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a Lambda handler method header with 2 arguments and 3 arguments. The 2-arg header calls the 3-arg header under-the-hood, so as long as we instrument the 3-arg header, we support all cases. We were already instrumenting the 3-arg header thanks to #8264; So TLDR this is non-breaking |
||
| @Origin("#m") final String methodName) { | ||
|
|
||
| if (CallDepthThreadLocalMap.incrementCallDepth(RequestHandler.class) > 0) { | ||
| return null; | ||
| } | ||
|
|
||
| AgentSpanContext lambdaContext = AgentTracer.get().notifyExtensionStart(event); | ||
| AgentSpanContext lambdaContext = AgentTracer.get().notifyExtensionStart(in); | ||
| final AgentSpan span; | ||
| if (null == lambdaContext) { | ||
| span = startSpan(INVOCATION_SPAN_NAME); | ||
| } else { | ||
| span = startSpan(INVOCATION_SPAN_NAME, lambdaContext); | ||
| } | ||
| span.setTag("request_id", awsContext.getAwsRequestId()); | ||
|
|
||
| final AgentScope scope = activateSpan(span); | ||
| return scope; | ||
| } | ||
|
|
||
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.
Should we keep calling it
event?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.
The 2-arg handler takes args
(InputType input, Context context)and the 3-arg handler tags args(InputStream input, OutputStream output, Context context)so in/out is better than event hereThere 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.
Ah, got it, thanks!
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.
Maybe add a comment about the different types of handlers? Is it worth adding here?
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.
Oops I already merged, but the PR from https://github.com/DataDog/dd-trace-java/pull/8264/files added some comments explaining this