Skip to content
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

Ensure lambda instrumentation only notifies extension once. #5422

Merged
merged 3 commits into from
Jun 23, 2023

Conversation

purple4reina
Copy link
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 owner June 16, 2023 19:34
@pr-commenter
Copy link

pr-commenter bot commented Jun 16, 2023

Benchmarks

Parameters

Baseline Candidate
commit 1.17.0-SNAPSHOT~0783f27eca 1.17.0-SNAPSHOT~f0880304cc
config baseline candidate
See matching parameters
Baseline Candidate
module Agent Agent
parent None None

Summary

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

Copy link
Contributor

@bantonsson bantonsson left a comment

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

import java.util.Set;
import okio.BufferedSink;

public final class ReadFromInputStreamJsonAdapter<T> extends JsonAdapter<T> {
Copy link
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
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 joeyzhao2018 force-pushed the rey.abolofia/double-wrapping branch 2 times, most recently from 1791176 to 16b48e1 Compare June 23, 2023 02:35
@joeyzhao2018 joeyzhao2018 force-pushed the rey.abolofia/double-wrapping branch from 16b48e1 to f088030 Compare June 23, 2023 03:07
@joeyzhao2018 joeyzhao2018 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 master Jun 23, 2023
@joeyzhao2018 joeyzhao2018 deleted the rey.abolofia/double-wrapping branch June 23, 2023 05:51
@github-actions github-actions bot added this to the 1.17.0 milestone Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: no release notes Changes to exclude from release notes tag: serverless Serverless support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants