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!: remove 'Http' from W3C propagator names #2429

Merged
merged 2 commits into from Aug 30, 2021

Conversation

aabmass
Copy link
Member

@aabmass aabmass commented Aug 26, 2021

Which problem is this PR solving?

Fixes #2428

Short description of the changes

Renames:

  • HttpTraceContextPropagator to W3CTraceContextPropagator
  • HttpBaggagePropagator to W3CBaggagePropagator

This is a breaking change

Fix in the contrib repo: open-telemetry/opentelemetry-js-contrib#644. I didn't find any places that need updating in opentelemetrry-js-api or opentelemetry.io repos.

@codecov
Copy link

codecov bot commented Aug 26, 2021

Codecov Report

Merging #2429 (115134a) into main (1465865) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2429   +/-   ##
=======================================
  Coverage   92.70%   92.70%           
=======================================
  Files         137      137           
  Lines        4993     4993           
  Branches     1056     1056           
=======================================
  Hits         4629     4629           
  Misses        364      364           
Impacted Files Coverage Δ
...re/src/baggage/propagation/W3CBaggagePropagator.ts 96.96% <100.00%> (ø)
...emetry-core/src/trace/W3CTraceContextPropagator.ts 100.00% <100.00%> (ø)
...elemetry-sdk-trace-base/src/BasicTracerProvider.ts 94.49% <100.00%> (ø)

Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

lgtm

@aabmass
Copy link
Member Author

aabmass commented Aug 26, 2021

PTAL at #2428 (comment) and leave your thoughts. The baggage propagator also needs to be renamed and we need to decide on a name before merging this.

Copy link
Member

@vmarchaud vmarchaud left a comment

Choose a reason for hiding this comment

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

i think we should add this change to the upgrade guidelines in the readme ?

@aabmass aabmass changed the title fix!: rename HttpTraceContextPropagator to TraceContextTextMapPropagator fix!: remove 'Http' from W3C propagator names Aug 30, 2021
- `HttpTraceContextPropagator` to `W3CTraceContextPropagator`
- `HttpBaggagePropagator` to `W3CBaggagePropagator`
@aabmass
Copy link
Member Author

aabmass commented Aug 30, 2021

@vmarchaud thanks, I updated the README

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.

HttpTraceContextPropagator should not have "HTTP" in the name
5 participants