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

chore: move opentelemetry-propagator-jaeger to core #351

Merged

Conversation

jtmalinowski
Copy link
Contributor

Move opentelemetry-propagator-jaeger from contrib to core (open-telemetry/opentelemetry-js#1931).
This is to enable turning on the jaeger propagator using ENV variables.
Default Node Tracer Provider needs to depend on Jaeger propagator.

Which problem is this PR solving?

Short description of the changes

  • remove propagators/opentelemetry-propagator-jaeger

@codecov
Copy link

codecov bot commented Feb 15, 2021

Codecov Report

Merging #351 (cc4183b) into main (a40df3c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #351   +/-   ##
=======================================
  Coverage   94.43%   94.43%           
=======================================
  Files          11       11           
  Lines         431      431           
  Branches       48       48           
=======================================
  Hits          407      407           
  Misses         24       24           

@vmarchaud
Copy link
Member

@jtmalinowski looks fine, however tests are failing :/

@jtmalinowski jtmalinowski force-pushed the chore/adopt-jaeger-propagator branch 2 times, most recently from ec8f828 to 18a6819 Compare March 10, 2021 16:20
@dyladan dyladan changed the title chore: disown opentelemetry-propagator-jaeger chore: move opentelemetry-propagator-jaeger to core Mar 10, 2021
@obecny
Copy link
Member

obecny commented Mar 30, 2021

@jtmalinowski can you please resolve conflicts ?, core PR has been merged, thx

Move opentelemetry-propagator-jaeger from contrib to core (open-telemetry#1931).
This is to enable turning on the jaeger propagator using ENV variables.
Default Node Tracer Provider needs to depend on Jaeger propagator.
@jtmalinowski
Copy link
Contributor Author

@obecny done, but now tests are failing; seems unrelated to me, but if there's something I can do to help, let me know

@jtmalinowski
Copy link
Contributor Author

Tests fail because of an issue with transitive dependencies, and they are failing similarly on other PRs.

@obecny obecny merged commit 552d2c0 into open-telemetry:main Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants