Allow Global Access Pattern for OpenTelemetryRum

[RiyazurRazak]

Currently, the OpenTelemetry instance is initialized and accessed via a static reference in MainApplication (as shown in the demo application). This approach introduces tight coupling to the application entry point and requires manual wiring for global access.

Would it be possible to introduce a pattern similar to GlobalOpenTelemetry (from Otel-java), or expose a globally accessible object within OpenTelemetryRum, allowing developers to access the instance directly throughout the application without relying on a static reference in MainApplication?

[fractalwrench]

A complication that opentelemetry-java doesn't have to deal with is that it's necessary to take a Context to do anything useful on Android, namely being able to write to disk, and to setup instrumentation that relies on lifecycle callbacks. As such I think it's always going to be a requirement to initialize opentelemetry-android somewhere that a Context is available (either in an Application subclass, or perhaps an Initializer.

I would personally favour using dependency injection to access this instance given that it's a fairly common pattern to setup DI within the Application subclass (or Initializer). Introducing a singleton with global state feels problematic as (1) it requires managing mutable state, and (2) it forces library consumers to check whether the SDK has been initialized.

[breedx-splk]

Why would accessing a static instance in an otel-android global be better than accessing a static instance in your main application?

One thing that people sometimes want is a way for library code (or 3rd party dependency code, maybe owned by a different team) to be able to use the opentelemetry globals to do manual instrumentation without knowing about the application. I understand that it feels convenient, but this is problematic for several reasons...

I would personally favour using dependency injection to access this instance given that it's a fairly common pattern to setup DI within the Application subclass (or Initializer). Introducing a singleton with global state feels problematic as (1) it requires managing mutable state, and (2) it forces library consumers to check whether the SDK has been initialized.

I agree -- having a global reference is definitely problematic, and DI is your better solution here. The first and most obvious problem (to me) is that having a global/static access mechanism encourages developers to violate the principle of dependency injection. This, in general, makes code much harder to test.

The other thing is that it introduces race conditions where the library code could use the opentelemetry instance(s) before they have been initialized by the application startup code. This is already true in the java repos, because simply calling get() on GlobalOpenTelemetry has the side effect of initializing the sdk...which may not be the best route, especially when other configuration might need to happen first.

[breedx-splk]

I think we could also consider providing something similar to the AutoConfigureListener that the core java sdk provides, but for the OpenTelemetryRum instance. You could imagine a new interface like OpenTelemetryRumReadyListener that, wherever it's implemented, would receive the OpenTelemetryRum instance. This could also then be used by libraries and not require direct injection, but instead, lazy callback/notification. This seems like a reasonable tradeoff to me.

[RiyazurRazak]

Thanks for the clarification, team. I raised this in the context of the AutoConfigure / ZeroCode instrumentation work (issue #1144), where the SDK needs to initialized automatically via an initializer. I was considering whether that flow might justify exposing a more globally accessible entry point, since the SDK instance is created there. That said, I understand and appreciate the concerns about global state and potential initialization-order issues.

[breedx-splk]

I raised this in the context of the AutoConfigure / ZeroCode instrumentation

That's cool. Definitely when that auto-instrumentation (via gradle build-time plugin for auto-wiring/initializing the agent into the app) is built, code will need a way to get a handle on the instance, sure. Maybe a listener is a reasonable way to do that....but we can at least wait on this until that need arises.

[bidetofevil]

Echoing what Jamie said, I think on Android, dependency injection is the way to go to provide instrumentation with the right opentelemetry instance. If the desire is to be able to do zero-code initialization, the instrumentation will still have to be initialized somehow, so the dependency can be provided at instrumentation registration time, much like its done now. And for that, we don't need to fallback on this anti-pattern.

[breedx-splk]

dependency injection is the way to go to provide instrumentation with the right opentelemetry instance. If the desire is to be able to do zero-code initialization, the instrumentation will still have to be initialized somehow, so the dependency can be provided at instrumentation registration time, much like its done now. And for that, we don't need to fallback on this anti-pattern.

So I am a big fan of dependency injection for all the obvious reasons...but let's consider a problem scenario.

Let's consider a large org that builds 10 different android apps, and those apps share a library to encapsulate and reuse some business logic. And let's say that the library has classes that generate mobile telemetry via the public OpenTelemetryRum interface. Like this very cool library class:

class ImportantThinger(
    private val rum: OpenTelemetryRum
) {
    fun doIt() {
        rum.emitEvent("zipperoo")
        // [...] biz logic....
    }
}

so if the Application uses this library which contains ImportantThinger, it MUST initialize OpenTelemetryRum before it can use any library features that might rely on this. In a perfect world, fine, but in many cases, what libraries do is often outside of the Application's control....including things like (unfortunate) static initialization. We also don't necessarily want to enforce an ordering (what if the library wants to be initialized concurrently with the otel initialization?)

So what I was proposing was something more akin to this, which would allow library classes to implement an interface to get notified when the rum instance has been created:

class ImportantThinger(
    private var rum: OpenTelemetryRum = NoOpOpenTelemetryRum()
): OpenTelemetryRumReadyListener {
    fun doIt() {
        rum.emitEvent("zipperoo")
        // [...] biz logic....
    }

    override fun ready(rum: OpenTelemetryRum) {
        this.rum = rum // yadda yadda not threadsafe
    }
}

And then the agent, after creating the rum instance, would find all instances of the OpenTelemetryRumReadyListener interface and then pass them the new instance. This allows the library to start up whenever, effectively generating no events until otel rum is ready.

While this is also not the "make everything global" result that this issue is asking for originally, I feel like it's a reasonable compromise. WDYT?

[fractalwrench]

Another approach would be use a supplier, e.g.:

class ImportantThinger(
    private var rumSupplier: () -> OpenTelemetryRum = {
      if (initialized) {
          rumInstance
      } else {
          NoOpOpenTelemetryRum
      }
  }
)

It would even be possible to pass a 'buffering' opentelemetry instance that simply records the call when the real instance isn't initialized, and then passes calls onto the real instance when that gets initialized. Perhaps that is overkill.

I think given that opentelemetry-android relies on Context to initialize we will need some solution, given that other components can initialize before a Context is available and might want tracing. Searching for listener interfaces is one option however it does have the disadvantage of handing off state management to the end-user, and I'm unsure of what method would be used for finding all those interfaces and invoking them.

After typing all that out, I am more of the opinion that a global access pattern could be useful if we discouraged use within classes and suggested that folks inject the value in with a lambda/alternative. Effectively whatever you do for use-cases early in the application lifecycle, it's currently necessary to check whether the instance is initialized and conditionally return the correct instance based on that.

[LikeTheSalad]

Is the library example meant only for local libraries? Or are we considering a third-party library that wants to use an OpenTelemetry instance as well? If it's the latter, a supplier works if you're the owner of the library; otherwise, I can't see how we could convince every library maintainer to follow the same pattern.

Also, providing OpenTelemetryRum to libraries sounds a bit risky, since that would give them access to shut the instance down. Maybe an alternative could be providing an OpenTelemetry instance that's a delegator, which would initially delegate to a noop instance (or buffer-only impl), and then, when the real instance is provided to it from the app, it would delegate to it. This approach also comes with its downsides, though, as libraries could get objects from within the OpenTelemetry instance, which would also have to be delegators of their own type (which seems to be an issue for the supplier option as well). It's an interesting challenge to provide an OTel instance to libraries.