Skip to content

Remove the WithDisabled option from Jaeger exporter#1806

Merged
MrAlias merged 9 commits intoopen-telemetry:mainfrom
MrAlias:jaeger-rm-disabled
Apr 19, 2021
Merged

Remove the WithDisabled option from Jaeger exporter#1806
MrAlias merged 9 commits intoopen-telemetry:mainfrom
MrAlias:jaeger-rm-disabled

Conversation

@MrAlias
Copy link
Copy Markdown
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
@codecov
Copy link
Copy Markdown

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