Skip to content

Ensure lambda instrumentation only notifies extension once. - #5422

Merged
joeyzhao2018 merged 3 commits into
masterfrom
rey.abolofia/double-wrapping
Jun 23, 2023
Merged

Ensure lambda instrumentation only notifies extension once.#5422
joeyzhao2018 merged 3 commits into
masterfrom
rey.abolofia/double-wrapping

Conversation

@purple4reina

Copy link
Copy Markdown
Contributor

What Does This Do

Prevent calling start-invocation and end-invocation on the aws lambda extension multiple times.

The lambda event needs to be serialized to json and shared with the extension. This event was properly serialized for the second wrapping, but not for the first. This means that once the extra handler wrapping is ignored, only one aws.lambda span is being created, but the lack of event json means we won't be able to create inferred spans or properly handle trace context.

The solution for this second problem is to add a new json adapter to serialize these types.

Motivation

In the most recent version of the java tracing layer for aws lambda, the request handler is being wrapped at least twice.

[dd.trace 2023-05-18 21:41:20:530 +0000] [main] DEBUG datadog.trace.agent.tooling.InstrumenterState - Instrumentation applied - instrumentation.names=[aws-lambda] instrumentation.class=datadog.trace.instrumentation.aws.v1.lambda.LambdaHandlerInstrumentation instrumentation.target.classloader=lambdainternal.CustomerClassLoader@4d2a1da3
[dd.trace 2023-05-18 21:41:21:649 +0000] [main] DEBUG datadog.trace.agent.tooling.InstrumenterState - Instrumentation applied - instrumentation.names=[aws-lambda] instrumentation.class=datadog.trace.instrumentation.aws.v1.lambda.LambdaHandlerInstrumentation instrumentation.target.classloader=jdk.internal.loader.ClassLoaders$AppClassLoader@2626b418

This double wrapping is causing the tracer to make calls to start-invocation and end-invocation multiple times. A aws.lambda root span is created in the extension for every call to these endpoints.

Additional Notes

@purple4reina
purple4reina requested a review from a team as a code ownerJune 16, 2023 19:34
@pr-commenter

pr-commenterBot commented Jun 16, 2023

Copy link
Copy Markdown

Benchmarks

Parameters

BaselineCandidate
commit1.17.0-SNAPSHOT~0783f27eca1.17.0-SNAPSHOT~f0880304cc
configbaselinecandidate
See matching parameters
BaselineCandidate
moduleAgentAgent
parentNoneNone

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 22 cases.

@bantonssonbantonsson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

The codenarc failure seems to be an unnecessary import in the test 🤷🏼‍♂️

https://output.circle-artifacts.com/output/job/046ad3e3-a643-4ba2-8ba3-14c0c9f1fa2a/artifacts/0/check_reports/dd-trace-core/codenarc/test.html

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't T be ByteArrayInputStream since you know the type?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. updated.

@joeyzhao2018
joeyzhao2018force-pushed the rey.abolofia/double-wrapping branch 2 times, most recently from 1791176 to 16b48e1CompareJune 23, 2023 02:35
@joeyzhao2018
joeyzhao2018force-pushed the rey.abolofia/double-wrapping branch from 16b48e1 to f088030CompareJune 23, 2023 03:07
@joeyzhao2018joeyzhao2018 added tag: serverless Serverless support tag: no release notes Changes to exclude from release notes labels Jun 23, 2023
@joeyzhao2018
joeyzhao2018 merged commit 1b5f2b2 into masterJun 23, 2023
@joeyzhao2018
joeyzhao2018 deleted the rey.abolofia/double-wrapping branch June 23, 2023 05:51
@github-actionsgithub-actionsBot added this to the 1.17.0 milestone Jun 23, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tag: no release notesChanges to exclude from release notestag: serverlessServerless support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@purple4reina@bantonsson@joeyzhao2018