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

Fix problem in xray-lambda propagator definition #778

Merged
merged 7 commits into from
Mar 1, 2024

Conversation

tylerbenson
Copy link
Member

@tylerbenson tylerbenson commented Feb 27, 2024

the problem comes from this "late" addition:

To avoid potential issues when extracting with an active span context, the xray-lambda propagator SHOULD check if the provided context already has an active span context. If found, the propagator SHOULD return the provided context unmodified.

This is a problem because xray-lambda can't check the original context, only the context that is provided to it, which is modified by prior propagators in the chain.

This means if the original carrier has an x-ray context and an x-ray environment variable set, and if the xray propagator is configured as we expect it to be, then the xray-lambda will be either overwritten (if configured first), or will no-op because it detects an existing span context.

Proposed solution: change xray-lambda to replace xray in the configured list instead of work together with. This way, it is able to check the context before xray modifies it.

Merge requirement checklist

the problem comes from this "late" addition:
> To avoid potential issues when extracting with an active span context, the xray-lambda propagator SHOULD check if the provided context already has an active span context. If found, the propagator SHOULD return the provided context unmodified.

This is a problem because `xray-lambda` can't check the original context, only the context that is provided to it, which is modified by prior propagators in the chain.

This means if the original carrier has an x-ray context and an x-ray environment variable set, and if the xray propagator is configured as we expect it to be, then the xray-lambda will be either overwritten (if configured first), or will no-op because it detects an existing span context.

Proposed solution: change `xray-lambda` to replace `xray` in the configured list instead of work together with. This way, it is able to check the context before `xray` modifies it.
docs/faas/aws-lambda.md Show resolved Hide resolved
docs/faas/aws-lambda.md Outdated Show resolved Hide resolved
docs/faas/aws-lambda.md Outdated Show resolved Hide resolved
docs/faas/aws-lambda.md Outdated Show resolved Hide resolved
docs/faas/aws-lambda.md Outdated Show resolved Hide resolved
Co-authored-by: Raphael Philipe Mendes da Silva <rapphil@gmail.com>
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
docs/faas/aws-lambda.md Outdated Show resolved Hide resolved
@lmolkova lmolkova merged commit 6659dfa into open-telemetry:main Mar 1, 2024
10 checks passed
@tylerbenson tylerbenson deleted the tyler/xray-env branch March 5, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants