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

Sqs distributed trace #2204

Merged
merged 14 commits into from
Jan 21, 2025
Merged

Sqs distributed trace #2204

merged 14 commits into from
Jan 21, 2025

Conversation

obenkenobi
Copy link
Contributor

@obenkenobi obenkenobi commented Jan 15, 2025

Overview

Adds distributed trace headers as message attributes for SQS send operations.

(Edit: Distributed tracing is only supported in the v2 sdks for SQS. See the new PR that removed sqs changes for the v1 sdk)

Related Github Issue

#2036

@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.56%. Comparing base (2180493) to head (d68e95d).
Report is 124 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2204      +/-   ##
============================================
- Coverage     70.73%   70.56%   -0.17%     
+ Complexity    10046     9974      -72     
============================================
  Files           842      842              
  Lines         40493    40374     -119     
  Branches       6151     6119      -32     
============================================
- Hits          28641    28490     -151     
- Misses         9098     9107       +9     
- Partials       2754     2777      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@obenkenobi
Copy link
Contributor Author

Class loading errors in instrumentation tests forced me to use inner classes for DT Header classes

* Headers are used as inner classes to avoid class loading errors when running instrumentation tests
* */

public class SQSRequestHeaders implements Headers {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this inner class to its own file and use the same on the async client.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Problem here is when I did that, I got class-loading errors when running instrumentation tests. I suspect its some magic the SDK is using with classloaders that did not mesh well with the agent. This was the one way I got it to work.

Copy link
Contributor

@meiao meiao Jan 17, 2025

Choose a reason for hiding this comment

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

You need to add the package that has the header classes to the @InstrumentationTestConfig
In this case it would look like:

@InstrumentationTestConfig(includePrefixes = { "software.amazon.awssdk.services.sqs", "com.newrelic.utils" }, configName = "dt_enabled.yml")

(if you put the header classes in the same package as the other classes in the module)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I moved the header classes and the tests pass now.

@obenkenobi obenkenobi merged commit d5e2196 into main Jan 21, 2025
114 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants