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

Minimize public surface area of OpenTracingShim #5110

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

jack-berg
Copy link
Member

Related to #4911. Trimming down the public API surface area of OpenTracingShim.

OpenTracingShim contains several different factory methods allowing users to create Tracer shims using various combinations of GlobalOpenTelemetry, OpenTelemetry, TracerProvider, and Tracer. Simplified to just two methods, which becomes the total public API surface area of the entire module:

// Create using tracer provider and propagator from an OpenTelemetry instance
io.opentracing.Tracer createTracerShim(OpenTelemetry);
// Create using specific tracer provider, text map propagator, and http propagator
io.opentracing.Tracer createTracerShim(TracerProvider, TextMapPropagator textMapPropagator, TextMapPropagator httpPropagator);

If a user wants to use the global, just call createTracerShim(GlobalOpenTelemetry.get()).

This allows the removal of OpenTracingPropagators and OpenTracingPropagatorsBuilder, which was just a wrapper for a TextMapPropagator and http TextMapPropagator.

cc @carlosalberto

@jack-berg jack-berg requested a review from a team January 10, 2023 22:39
@codecov
Copy link

codecov bot commented Jan 10, 2023

Codecov Report

Base: 91.06% // Head: 91.04% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (0d37484) compared to base (b8f7a87).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5110      +/-   ##
============================================
- Coverage     91.06%   91.04%   -0.02%     
+ Complexity     4887     4874      -13     
============================================
  Files           551      549       -2     
  Lines         14462    14435      -27     
  Branches       1390     1390              
============================================
- Hits          13170    13143      -27     
  Misses          893      893              
  Partials        399      399              
Impacted Files Coverage Δ
...opentelemetry/opentracingshim/OpenTracingShim.java 100.00% <100.00%> (ø)
.../io/opentelemetry/opentracingshim/Propagation.java 88.23% <100.00%> (ø)
...a/io/opentelemetry/opentracingshim/TracerShim.java 78.12% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

You had me at "minimize public surface area"

@jack-berg jack-berg merged commit f981ad2 into open-telemetry:main Jan 12, 2023
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.

3 participants