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.

@aabmass aabmass force-pushed the rename-tracecontext-2428 branch from 7d201d3 to 492107a Compare August 26, 2021 16:35
@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%> (ø)

@aabmass aabmass marked this pull request as ready for review August 26, 2021 19:23
@aabmass aabmass requested a review from a team August 26, 2021 19:23
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 force-pushed the rename-tracecontext-2428 branch from c1b429a to 280488c Compare August 30, 2021 18:01
@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 aabmass force-pushed the rename-tracecontext-2428 branch from 280488c to 881703e Compare August 30, 2021 18:03
@aabmass aabmass force-pushed the rename-tracecontext-2428 branch from 881703e to 115134a Compare August 30, 2021 18:07
@aabmass aabmass requested a review from vmarchaud August 30, 2021 18:08
@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