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

feat(aws-lambda)!: Remove explicit x-ray context in favor of global propagator #2369

Merged

Conversation

martinkuba
Copy link
Contributor

@martinkuba martinkuba commented Aug 4, 2024

This is part of open-telemetry/opentelemetry-js#4494

Currently, the instrumentation is explicitly retrieving context from the X-Ray _X_AMZN_TRACE_ID environment variable. It is possible now to handle this using the xray-lambda propagator, which is the approach described in the specification.

This change removes the explicit handling including the disableAwsContextPropagation configuration option.

Copy link

codecov bot commented Aug 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.83%. Comparing base (1ae98de) to head (310c7fe).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2369      +/-   ##
==========================================
- Coverage   90.85%   90.83%   -0.03%     
==========================================
  Files         159      159              
  Lines        7853     7831      -22     
  Branches     1622     1610      -12     
==========================================
- Hits         7135     7113      -22     
  Misses        718      718              
Files with missing lines Coverage Δ
...-instrumentation-aws-lambda/src/instrumentation.ts 94.57% <100.00%> (-0.64%) ⬇️

Co-authored-by: Tyler Benson <[email protected]>
@martinkuba martinkuba force-pushed the lambda-remove-hardcoded-propagator branch from ab400e8 to 3633a92 Compare August 6, 2024 17:49
Copy link
Member

@blumamir blumamir left a comment

Choose a reason for hiding this comment

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

Thanks @martinkuba .
Great to see this cleaned up, aligned to spec and simplified.

The changes LGTM, I am a bit worried that existing users might not be aware if they should actively take action or will potentially break when upgrading to this new version.

Please mark it as breaking and consider adding some more guidance and examples in the README on how to use it correctly.

@martinkuba martinkuba changed the title feat(aws-lambda): Remove explicit x-ray context in favor of global propagator feat(aws-lambda)!: Remove explicit x-ray context in favor of global propagator Oct 23, 2024
@martinkuba martinkuba requested a review from a team as a code owner October 30, 2024 12:50
Copy link
Member

@blumamir blumamir left a comment

Choose a reason for hiding this comment

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

Thanks @martinkuba this looks great!

Added 2 minor comments about some cleanups that I think we need to apply before merging

@martinkuba martinkuba force-pushed the lambda-remove-hardcoded-propagator branch from 2bab72d to a954164 Compare October 31, 2024 22:32
@blumamir blumamir merged commit a926f53 into open-telemetry:main Nov 1, 2024
23 checks passed
@dyladan dyladan mentioned this pull request Nov 1, 2024
@martinkuba martinkuba deleted the lambda-remove-hardcoded-propagator branch November 1, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants