Consider not using statics everywhere

[marcingrzejszczak]

Is your feature request related to a problem? Please describe.
The current 0.10.0 version is using a lot of static calls

class MyClass {
 *   private static final Tracer tracer = OpenTelemetry.getTracer();
 *   void doWork() {
 *     Span span = tracer.spanBuilder("MyClass.DoWork").startSpan();
 *     try(Scope ss = TracingContextUtils.currentContextWith(span)) {
 *       TracingContextUtils.getCurrentSpan().addEvent("Starting the work.");
 *       doWorkInternal();
 *       TracingContextUtils.getCurrentSpan().addEvent("Finished working.");
 *     } finally {
 *       span.end();
 *     }
 *   }
 * }

vs 0.9.1

class MyClass {
 *   private static final Tracer tracer = OpenTelemetry.getTracer();
 *   void doWork() {
 *     Span span = tracer.spanBuilder("MyClass.DoWork").startSpan();
 *     try(Scope ss = tracer.withSpan(span)) {
 *       tracer.getCurrentSpan().addEvent("Starting the work.");
 *       doWorkInternal();
 *       tracer.getCurrentSpan().addEvent("Finished working.");
 *     } finally {
 *       span.end();
 *     }
 *   }
 * }

Also the whole Context API is done via statics - it's difficult to put in place any other implementation for testing purposes for example. You can check Brave's CurrentTraceContext API for reference on how they did this.

Describe the solution you'd like
Using the tracer to retrieve the current span was a much better idea. I don't understand the rationale behind such changes.

Describe alternatives you've considered
I have to wrap the static calls into my own abstractions so that the users don't have to call statics.

[Oberon00]

Using the tracer to retrieve the current span was a much better idea. I don't understand the rationale behind such changes.

Please see here: open-telemetry/opentelemetry-specification#1063 (comment) and the links from there.

The problem with the previous solution was also that using the tracer was only one possibility, so even if you replace that implementation, code could also be using TracingContextUtils directly which would result in a mess.

[marcingrzejszczak]

Fair enough I'll hide the statics in my API then. We'll see how it goes once open-telemetry/opentelemetry-java-instrumentation#1464 gets finished and the code will compile again.

[anuraaga]

@marcingrzejszczak Can you describe the reason for exposing current context on tracer? Is it to mock the method in tests? If so, what if you just use the normal context API in the tests? It's not very expensive or complex I think. Just want to better understand what you're thinking :)

For reference, the armeria brave integration implements CurrentTraceContext using Armeria's static APIs and it works great - I don't think Armeria's use of static has had any negative impact on integrating with Brave.

[marcingrzejszczak]

@marcingrzejszczak Can you describe the reason for exposing current context on tracer? Is it to mock the method in tests? If so, what if you just use the normal context API in the tests? It's not very expensive or complex I think. Just want to better understand what you're thinking :)

Brave users are accustomed to it and by injecting Tracer I have access to everything really that I might need. Also one controls the full lifecycle of tracing operations and users might want to override them if necessary just by implementing a bean of Tracer type. From the point of view of Sleuth, since we're hiding the static calls behind an abstraction anyways this might not matter much cause the users might want to just override our API. If they're using OTel directly then we don't care really.

[carlosalberto]

Brave users are accustomed to it and by injecting Tracer I have access to everything really that I might need. Also one controls the full lifecycle of tracing operations and users might want to override them if necessary just by implementing a bean of Tracer type.

Not a strong opinion at this point, but FYI this is exactly why Ruby kept the operations in Tracer (although the part regarding override seems dangerous ;) )

[Oberon00]

There were differing opinions, also for ruby: open-telemetry/opentelemetry-specification#1063 (comment)

[carlosalberto]

@Oberon00 Although I'd think that at least you still have everything in the Tracer (as a component/class). Anyway, not a strong opinion on this matter anymore.

[Oberon00]

I think we must not add it to tracers, this causes confusion, e.g. to open-telemetry/opentelemetry-specification#586. We could add it to TracerProvider, but then that should be the one and only possibility to get the active span. And that would be worth a spec update.

[marcingrzejszczak]

Using statics requires everyone (at least in the Java world) to create a wrapper around that static method to stub it out. I don't think that it's a good idea and it's counter-intuitive. AFAIR @anuraaga has created some non-static methods on Span to e.g. put it in scope and this is a better idea. IMO having the methods inside the Tracer API was a better design idea, already known to the community.

[jkwatson]

Closing, since we now fully support instance-based OpenTelemetry access, and the static global is purely optional.