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

Update propagation to conform with OpenTelemetry specification #1212

Merged
merged 7 commits into from
Oct 2, 2020

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Sep 29, 2020

  • Remove the api/propagation package in favor of its components being a part of the main otel package (part of Move OpenTelemetry API to top level go.opentelemetry.io/otel package #964)
  • Remove the Propagators interface. This collides with the specifications definitions of Propagator types and is not needed given the other changes.
  • Replace all HTTP* propagation interfaces with a unified TextMapPropagator and TextMapCarrier to implement the specifications definition. Instead of breaking out Injector and Extractor functionality (which is optional in the specification), this unifies everything into a single interface. The motivation for this is ...
    • It simplifies use of propagators implementing the TextMapPropagator. Instead of just specifying you want to use the propagator you need to specify you want to use if for injecting and extracting which is not the common case.
    • If a user wants to limit to only injecting or extracting they can wrap the propagator still with a custom TextMapPropagator that limits this functionality. This is not the common case so the higher barrier to use here seems appropriate.
    • If there is user demand to split out the functionality it can still be accomplished in a backwards compatible manner after the GA.
  • The New function is replaced with a NewCompositeTextMapPropagator to define the functionality better and keep it in line with the specification.
  • The explicit ExportHTTP and InjectHTTP function are removed. They were equivalent to method calls on the second parameter.

Resolves #1082

@MrAlias MrAlias added pkg:API Related to an API package area:propagators Part of OpenTelemetry context propagation release:required-for-ga Skip Changelog PRs that do not require a CHANGELOG.md entry labels Sep 29, 2020
@MrAlias MrAlias added this to the RC1 milestone Sep 29, 2020
@MrAlias MrAlias self-assigned this Sep 29, 2020
@codecov
Copy link

codecov bot commented Sep 29, 2020

Codecov Report

Merging #1212 into master will increase coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1212   +/-   ##
======================================
  Coverage    76.5%   76.5%           
======================================
  Files         134     133    -1     
  Lines        5801    5797    -4     
======================================
- Hits         4442    4439    -3     
+ Misses       1110    1109    -1     
  Partials      249     249           
Impacted Files Coverage Δ
api/baggage/baggage_propagator.go 86.3% <ø> (+1.9%) ⬆️
api/global/internal/state.go 82.8% <ø> (+1.0%) ⬆️
api/global/propagation.go 0.0% <ø> (ø)
bridge/opentracing/bridge.go 40.0% <ø> (ø)
propagation.go 100.0% <ø> (ø)
propagators/trace_context.go 65.5% <ø> (ø)
... and 1 more

@MrAlias MrAlias removed the Skip Changelog PRs that do not require a CHANGELOG.md entry label Sep 29, 2020
@MrAlias MrAlias marked this pull request as ready for review September 29, 2020 23:48
@MrAlias MrAlias merged commit 6e184cd into open-telemetry:master Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:propagators Part of OpenTelemetry context propagation pkg:API Related to an API package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename HTTPText to TextMap Propagator
3 participants