Remove SPI?

[anuraaga]

I wonder if we can remove the SPI for discovering telemetry components, at least for now. I think using DefaultOpenTelemetry is quite similar to using OpenTelemetrySdkBuilder right now.

return DefaultOpenTelemetry.builder()
  .setTracerProvider(SdkTracerProvider.builder()...)
  .setMeterProvider(MicrometerMeterProvider.builder()...)
  .build();

return OpenTelemetrySdk.builder()
  .setTracerProvider(SdkTracerProvider.builder()...)
  .setMeterProvider(sdkMeterProvider) // Currently not fluent unfortunately
  .build();

Do we need the SPI anymore? @Oberon00 I think you've been interested in the custom SDK configuration so wondering if you have any thoughts.

[iNikem]

I vote in favour.

[Oberon00]

I think the SPIs for the separate components (e.g. TracerProvider found by DefaultOpenTelemetry) make no sense at this point and should be removed, unless we plan to add substantial configurability via environment variables. I think if we remove the addSpanProcessor from TracerProvider (did we already?) the same goes for the OpenTelemetry SPI.

[anuraaga]

Great! Yeah I have an autoconfigure module in mind for potential "substantial configurability via environment variables", which would also handle things like OTEL_EXPORTER. It'd be nice to clean first before trying it.

I'm thinking

openTelemetry = AutoConfiguredOpenTelemetry.create();

Will

• check SPI (exposed by the autoconfigure module, not api module). If any signal is available via SPI, the SDK implementation will not be used. I am thinking of autoconfigure depending on the full SDK and letting unnecessary SDK to be excluded if the binary size impact is annoying (even if they were present, they wouldn't be used if anything was found with SPI).

• For any signal without an SPI implementation, the SDK will be initialized, recognizing all known environment variables. Consumer<SdkTracerProviderBuilder> SPI will also be used to find customizers on the classpath since this artifact has no direct programmatic configuration whatsoever.

• OpenTelemetry will be returned. I guess we would need some way to cast that into a SdkTracerManagement for users of the SDK.

Still mostly brainstorming but let me know if you have any early thoughts.

[jkwatson]

It does seem like there is value in providing some sort of environment/system property based configuration by just depending on the SDK. Is SPI the right way to implement that, or is it ok to make people call a method in their main method, in addition to depending on the SDK module(s)?

I tend to think that requiring people to call a method to auto-configure the SDK is fine for now, but I know I have a limited view of the world. @bogdandrutu you have defended SPI as a very important mechanism, so your thoughts here would be valuable.

[jkwatson]

ah, I gotcha. Can we also make it play this loudly? https://www.youtube.com/watch?v=tBA-1ENblN4

Seriously, though, I'm totally on board with this, although I'm not 100% how to advise someone how to mitigate it, in the case that some instrumentation is doing something naughty before even a static initializer could fire in a main method.

[Oberon00]

in the case that some instrumentation is doing something naughty before even a static initializer could fire in a main method

Maybe the instrumented code is already doing something naughty? E.g. what if you want to instrument the createLogGroup HTTP request in AWS? That one would usually happen before main I think.

[anuraaga]

The only thing that can happen before main is premain, e.g., Java agent I think. Everything else is on classloading. AWS SDK supports instrumenting via a service loader type of thing, but that also could never happen before main.

[Oberon00]

Class-loading for the class containing main also happens before main, including any initialization for statics (recursively). But indeed in Java this is more bounded than e.g. in C++ where any library could have a global variable that is initialized before main and I was not really considering that.

Then again, in AWS Lambda and maybe other FaaS you don't control main, you can only use your request handler or static initializers. But maybe it is reasonable to not support every edge case there and advise people to use an agent that leverages premain.

[anuraaga]

We also talked about app servers like tomcat, and multi-tenancy at today's meeting. We had the feeling that for these sort of cases, there isn't much we can do anyways since it's too easy to clobber. It would be the app server's job to present a way to configure telemetry, somehow. Someday I hope Lambda instruments itself with OTel - when it does it may not allow configuration anyways, like the current state for X-Ray :) But if it does I guess it would also have knobs with a subset of what our SDK provides that could be used.

[jkwatson]

It might be useful to write down the requirements that the SPI in the API is currently trying to fulfill, so here's my attempt:

• API-only-at-runtime usages MUST always return a no-op instance of OpenTelemetry when accessing the global singleton.
• GlobalOpenTelemetry.set(OpenTelemetry) MUST only be called at-most once successfully, when the current global instance is null (this last caveat is support unit testing use-cases in the project itself).
• It MUST be possible to fully initialize the Global instance before any caller of GlobalOpenTelemetry.get(OpenTelemetry) gets a valid instance returned to it. (note: this is currently accomplished via mutability of the SDK instances, so propagators, span processors, trace config can be added after the fact).

Is there anything I missed here?

[anuraaga]

Thanks - these seem correct.

GlobalOpenTelemetry.set(OpenTelemetry) MUST only be called at-most once successfully.

To support unit tests, we may say that it MUST only succeed when the currently set global is null. Tests we can expect to set and reset which would be supported by such a model.