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

Remove the WithDisabled option from Jaeger exporter #1806

Merged
merged 9 commits into from
Apr 19, 2021

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Apr 13, 2021

The WithDisabled option was added to support standard environment variables used by other Jaeger clients here. The goal was to add support for these environment variables to the extent that they make sense in the OpenTelemetry project.

Support for this environment variable, JAEGER_DISABLED, was removed here. This seems appropriate as exporter activation or deactivation is handled by registering or unregistering it with the TracerProvider, or, at an earlier phase of initialization, by using the no-op TracerProvider itself. Given there are already mechanisms to disable the export of the Jaeger exporter in OpenTelemetry it would make sense to remove this duplicate mechanism.

This removes the option that was added in parallel for the support of the environment variable.

Part of #1803

@MrAlias MrAlias added release:1.0.0-rc.1 pkg:exporter:jaeger Related to the Jaeger exporter package labels Apr 13, 2021
@MrAlias MrAlias added this to the RC1 milestone Apr 13, 2021
@MrAlias MrAlias requested a review from Aneurysm9 as a code owner April 13, 2021 17:54
@codecov
Copy link

codecov bot commented Apr 19, 2021

Codecov Report

Merging #1806 (ccc3465) into main (6867faa) will decrease coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #1806     +/-   ##
=======================================
- Coverage   78.4%   78.3%   -0.1%     
=======================================
  Files        135     135             
  Lines       7249    7245      -4     
=======================================
- Hits        5686    5680      -6     
- Misses      1313    1315      +2     
  Partials     250     250             
Impacted Files Coverage Δ
exporters/trace/jaeger/jaeger.go 91.0% <ø> (-0.2%) ⬇️
exporters/otlp/otlpgrpc/connection.go 90.6% <0.0%> (-1.9%) ⬇️

@MrAlias MrAlias merged commit 1b9f16d into open-telemetry:main Apr 19, 2021
@MrAlias MrAlias deleted the jaeger-rm-disabled branch April 19, 2021 16:09
@MrAlias MrAlias mentioned this pull request Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:exporter:jaeger Related to the Jaeger exporter package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants