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

Simplify aws-sdk traces #4684

Merged
merged 6 commits into from
Feb 23, 2023
Merged

Simplify aws-sdk traces #4684

merged 6 commits into from
Feb 23, 2023

Conversation

mcculls
Copy link
Contributor

@mcculls mcculls commented Feb 7, 2023

What Does This Do

This PR removes the underlying HTTP client spans beneath AWS-SDK calls. The HTTP client span is an implementation detail of the AWS-SDK call, rather than a request to a different service, and collapsing the trace to just show the AWS-SDK calls makes distributed traces easier to follow.

Users who require the old behaviour can re-enable it with this system property:

-Ddd.aws-sdk.legacy.tracing.enabled=true

or by setting this environment variable:

DD_AWS_SDK_LEGACY_TRACING_ENABLED=true

Motivation

We want to make SQS receive spans more consistent with other messaging integrations. This will involve excluding certain SQS calls from our AWS-SDK instrumentation and adding new SQS specific instrumentation. The new SQS instrumentation will avoid creating spans when no messages are received, but underneath the AWS-SDK is still making an HTTP call to check for new messages. Since we don't know at the time of the underlying HTTP call whether we will actually get any messages we cannot filter out any HTTP span at that point. It is also hard to tell which AWS service is being called without examining the attached entity since AWS-SDK uses POST by default.

The simplest solution was to remove the underlying HTTP client spans beneath AWS-SDK calls.

Before:

Screenshot 2023-02-08 at 00 31 40

After:

Screenshot 2023-02-08 at 00 31 28

Additional Notes

This PR also moves setting of the X-Ray propagation header out from the different low-level HTTP instrumentations and into the AWS-SDK instrumentations where it really belongs. (To avoid adding an extra helper class the propagation setter interface was implemented by the existing AWS-SDK decorators.)

@mcculls mcculls added inst: others All other instrumentations inst: aws sdk AWS SDK instrumentation tag: breaking change Breaking changes labels Feb 7, 2023
@mcculls mcculls marked this pull request as ready for review February 8, 2023 10:14
@mcculls mcculls requested a review from a team as a code owner February 8, 2023 10:14
@mcculls mcculls changed the title [WIP] simplify aws-sdk traces Simplify aws-sdk traces Feb 8, 2023
@mcculls mcculls force-pushed the mcculls/simplify-aws-sdk-traces branch from 31d570a to e17bd77 Compare February 14, 2023 11:03
@mcculls mcculls force-pushed the mcculls/simplify-aws-sdk-traces branch from b2c251e to 5c267d1 Compare February 15, 2023 13:14
@mcculls mcculls changed the title Simplify aws-sdk traces Simplify aws-sdk traces [1 of 3] Feb 17, 2023
Copy link
Contributor

@ygree ygree left a comment

Choose a reason for hiding this comment

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

lgtm

@mcculls mcculls merged commit 3ae5d9f into master Feb 23, 2023
@mcculls mcculls deleted the mcculls/simplify-aws-sdk-traces branch February 23, 2023 00:38
@mcculls mcculls changed the title Simplify aws-sdk traces [1 of 3] Simplify aws-sdk traces Feb 23, 2023
@github-actions github-actions bot added this to the 1.9.0 milestone Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inst: aws sdk AWS SDK instrumentation inst: others All other instrumentations tag: breaking change Breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants