-
Notifications
You must be signed in to change notification settings - Fork 804
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
Move xray propagator from contrib (no history) #4603
Move xray propagator from contrib (no history) #4603
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4603 +/- ##
==========================================
+ Coverage 92.83% 92.88% +0.05%
==========================================
Files 329 329
Lines 9528 9604 +76
Branches 2053 2070 +17
==========================================
+ Hits 8845 8921 +76
Misses 683 683
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍 We may need to add/remove some devDependencies
(edit: for the changelog, we could mention that it has moved over, and that versions 1.4 through 1.22 have been skipped)
Note to reviewers: this package is already stable in contrib, so we can only make minimal changes to it.
I have added this as a feature to changelog. Let me know if you want to classify it differently. And I have added a note in the README of the package about the skipped versions. |
Does that mean we could move Google Trace to here too? Would be nice to be able to configure that too |
No. It's not a well-known propagator according to the specification. |
@martinkuba tests are likely failing due to a different version of We'll need to update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with a nit in the rEADME.
|
||
| Maturity | [Component Owner](../../.github/component_owners.yml) | Compatibility | | ||
| ----------------------------------------- | ----------------------------------------------------- | --------------------- | | ||
| [Stable](../../../CONTRIBUTING.md#stable) | @carolabadeer | API 1.0+<br/>SDK 1.0+ | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That link is no longer valid: both the relative path, and there is no "Stable" section in the CONTRIBUTING.md in the core repo.
As well, there is no component_owners.yml
in this repo.
Perhaps drop this "## Status" section? Not sure.
Is there any movement happening on this? Would like to be able to use an official xray propagator in my services. |
It will be included in the next release |
I don't think it's more official than now just because the repo changes. |
@anuraags You can use the xray propagator now - the lambda instrumentation uses it automatically as well as the lambda layer. You can also configure it manually in the TracerProvider. This change will only make it possible to set the propagator via the env variable. |
* moved aws xray propagator from contrib * updated package lock file * updated dev dependencies * added a note about the original location in README * updated changelog * fix: limit package-lock.json changes * removed status section from readme * chore: align versions with current release --------- Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
This is a second alternative to #4544. It adds the opentelemetry-propagator-aws-xray package from contrib without bringing over any history.
part of #4494
It should be possible to configure the
xray
propagator using theOTEL_PROPAGATORS
env variable. This is currently documented here, and it is also part of the lambda spec.