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

Fixing w3c baggage support in opentelemetry-instrumentation-aws-lambda #2589

Merged
merged 4 commits into from
Aug 1, 2024

Conversation

dtorok
Copy link
Contributor

@dtorok dtorok commented Jun 7, 2024

Description

Fixing baggage support in opentelemetry-instrumentation-aws-lambda package.

After determining the parent context (_determine_parent_context()), it was only used to create the wrapper span, but wasn't attached to the global context. Although the start_as_current_span() call attaches the span's context, but because the SpanContext only contains span-related info, the baggage content was lost, and the propagators couldn't pick it up.
This change attaches/detaches the baggage to the context in a try/finally clause. Other instrumentations do the same either directly, or through _start_internal_or_server_span().

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Implemented a unit test, and ran all the relevant tests:

tox -e lint-instrumentation-aws-lambda
tox -e py38-test-instrumentation-aws-lambda
tox -e py39-test-instrumentation-aws-lambda
tox -e py310-test-instrumentation-aws-lambda
tox -e py311-test-instrumentation-aws-lambda

Does This PR Require a Core Repo Change?

  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Copy link

linux-foundation-easycla bot commented Jun 7, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@dtorok dtorok requested a review from a team June 7, 2024 11:04
@wangzlei
Copy link

wangzlei commented Jun 12, 2024

LGTM!

@nemoshlag @NathanielRN

@xrmx
Copy link
Contributor

xrmx commented Jul 25, 2024

@dtorok please rebase and move the CHANGELOG entry to a new unreleased section

@dtorok dtorok force-pushed the aws-lambda-baggage-support branch from c25ef63 to bc1cbbc Compare July 25, 2024 10:11
Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi left a comment

Choose a reason for hiding this comment

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

Lgtm, please resolve the small merge conflict after Changelog updated by merge from another PR.

@ocelotl ocelotl merged commit 4ea9e5a into open-telemetry:main Aug 1, 2024
389 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.

6 participants