Move AWS X-Ray Propagator into its own opentelemetry-propagators-aws package
#720
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NathanielRN
force-pushed
the
move-aws-xray-propagator-into-own-package
branch
2 times, most recently
from
20ec83f
to
13f6f86
Compare
In that case shouldn't the next version of |
owais
approved these changes
Oct 11, 2021
NathanielRN
force-pushed
the
move-aws-xray-propagator-into-own-package
branch
4 times, most recently
from
608d87a
to
7697f30
Compare
Yeah this makes sense! It's unfortunate because basically no code changed... but it's a breaking API change so we should bump to |
NathanielRN
force-pushed
the
move-aws-xray-propagator-into-own-package
branch
from
7697f30
to
da317e5
Compare
lzchen
approved these changes
Oct 11, 2021
|
@NathanielRN |
ocelotl
approved these changes
Oct 12, 2021
This was referenced Oct 12, 2021
NathanielRN
added a commit
to NathanielRN/opentelemetry-python-contrib-1
that referenced
this pull request
Dec 20, 2021
* When we moved the aws-xray propagator in open-telemetry#720, we needed to update the benchmark step of the workflow * Additionally, the GH action has evolved since we added benchmarks so we should use the latest version
3 tasks
NathanielRN
added a commit
to NathanielRN/opentelemetry-python-contrib-1
that referenced
this pull request
Dec 21, 2021
* When we moved the aws-xray propagator in open-telemetry#720, we needed to update the benchmark step of the workflow * Additionally, the GH action has evolved since we added benchmarks so we should use the latest version
owais
pushed a commit
that referenced
this pull request
Dec 23, 2021
* Include propagator benchmarks + latest GH action * When we moved the aws-xray propagator in #720, we needed to update the benchmark step of the workflow * Additionally, the GH action has evolved since we added benchmarks so we should use the latest version * Merge all parallel benchmarks after they complete
ItayGibel-helios
pushed a commit
to helios/opentelemetry-python-contrib
that referenced
this pull request
Dec 26, 2021
…en-telemetry#838) * Include propagator benchmarks + latest GH action * When we moved the aws-xray propagator in open-telemetry#720, we needed to update the benchmark step of the workflow * Additionally, the GH action has evolved since we added benchmarks so we should use the latest version * Merge all parallel benchmarks after they complete
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Recently the AWS team discovered that we made the error of bundling the
AWSXrayFormatpropagator with the rest of the code in theopentelemetry-sdk-extension-awspackage. While theAWSXrayIdGeneratorandAws<Service>ResourceDetectorneed a direct dependency on theopentelemetry-sdkpackage, theAWSXrayFormatpropagator should only depend on theopentelemetry-apipackage.This became obvious to us when we started making plans to upstream the OpenTelemetry Lambda Python Package and to get the propagator, would have to take a dependency on
opentelemetry-sdk. This is wrong because none of the instrumentation depend onopentelemetry-sdk.This matches the pattern we have in other languages:
In JavaScript:
(JS splits the detectors & ID Generator but we shouldn't have to)
In Java:
etc.
The solution we propose is a new
opentelemetry-propagators-awspackage to hold just the propagator.Since the
opentelemetry-sdk-extension-awspackage is already released as1.0.1, this must be a breaking change to1.1.0.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
We moved all the unit tests to
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.