Enhance instrumentation API

[LikeTheSalad]

Introduction

After going stable with our andrid-agent module, we've discussed plans to stabilize other modules as well, in a recent Android SIG meeting.

One of the modules that could be ideal as a next candidate to become stable is android-instrumentation, where the base abstraction for all of OTel Android instrumentations is defined (AndroidInstrumentation), which would open the possibilities of marking specific instrumentations as stable right after.

The problem

However, before we stabilize the instrumentation API, I think we should make sure that it's flexible enough so that it can be extended in the future without having to introduce breaking changes or having to resort to workarounds, as we currently do with our CrashReporterInstrumentation, where we cast an OpenTelemetry instance to OpenTelemetrySdk for it to work properly:

opentelemetry-android/instrumentation/crash/src/main/java/io/opentelemetry/android/instrumentation/crash/CrashReporterInstrumentation.kt

Lines 29 to 30 in e725d67

	// TODO avoid using OpenTelemetrySdk methods, only use the ones from OpenTelemetry api.
	crashReporter.install(ctx.openTelemetry as OpenTelemetrySdk)

Proposals

I've put together a couple of proposals to make the instrumentation API more flexible, hoping it can be extended as needed in the future:

• Option 1 - Ensures that our OpenTelemetryRum abstraction can support common use cases, such as flushing log records (needed for our crash instrumentation).
• Option 2 - Creates separate/optional APIs that implementations of OpenTelemetryRum would have to opt-in to enable some use cases, such as flushing log records (needed for our crash instrumentation).

Both options extend our OpenTelemetryRum abstraction to make it provide common tools, such as the Clock instance used in our OpenTelemetry instance, needed by our ActivityLifecycleInstrumentation, and a SessionProvider instance, used by our SessionInstrumentation, as well as being a common RUM concept, so it sounds reasonable to make it available in our RUM abstraction.

I'm looking for some feedback on those, to see which one suits us all better, or to see extra alternatives that we can choose from as well.

[fractalwrench]

I'd also suggest that as part of a stable instrumentation API we should ensure it's possible to disable instrumentation from being loaded automatically. This is covered here: #411

[fractalwrench]

Made a couple of suggestions beyond what we discussed in the SIG here: #1545 (review)

[LikeTheSalad]

I'd also suggest that as part of a stable instrumentation API we should ensure it's possible to disable instrumentation from being loaded automatically. This is covered here: #411

I think this could be interpreted in 2 ways:

1. Disabling the service loading process altogether, so that no instrumentation gets installed automatically.
2. Picking a specific instrumentation to disable at runtime.

If I understand correctly, @breedx-splk was talking there more about the second option, because of the part that reads "disable some instrumentation". However, I believe we need to have both options available, so if that issue doesn't cover the first option, we should create another for it.

[LikeTheSalad]

Thank you for the feedback provided so far, @fractalwrench @breedx-splk. Based on the discussions we've had so far, it seems like there are a couple of challenges with the instrumentation APIs, and different solutions to each, that we should address before going stable, which can become a bit messy to discuss without properly identifying what goes where. So I'll try to summarise it all here to see if I properly understood what our shared thoughts are, and identify the exact issues. Please feel free to correct me if I misunderstood something. I think this is important to ensure we're on the same page to make it smoother to decide on the right solutions later.

Definition of end users and end goals

Before diving into the details, I'd like to define a couple of terms for the different kinds of target users of instrumentation APIs, to make it easier to address each later.

• Consumers - users who will install instrumentations.
• Producers - users who will create instrumentations.

And the following are what we all expect from the instrumentation APIs to be for each user. Again, please correct me in case you don't agree with some of these.

Goals for consumers:

1. APIs are easy to use.
2. Ability to get instrumentations automatically installed from the classpath.
3. Control over what gets installed.
   3.1 - Choosing what not to get installed automatically.
   3.2 - Ability to manually install instrumentations.
4. Ability to configure each instrumentation.

Goals for producers:

1. APIs are easy to use.
2. Ability to generate any kind of signal from within the instrumentation.
3. Access to contextual tools needed to generate telemetry.
   3.1 - Access to the Android app context.
   3.2 - Access to components of the OpenTelemetry instance, such as Clock and SessionProvider, and the ability to get more tools available if needed in the future, without introducing breaking changes.
4. Provide configuration options.

Requirements before stabilizing the instrumentation APIs

• To have a surface as small as possible while still meeting the goals for consumers and producers. This has been addressed more in detail by @fractalwrench over here.
• To avoid using workarounds to make our own instrumentations work.
• Prevent producers from using methods that are only available in the OpenTelemetrySdk class.

Current challenges

1. Some OpenTelemetry context tools are not available for producers

Description

When we build our instance of OpenTelemetry via the initializer function, we configure and set up many components that are needed for existing instrumentations to work properly, such as Clock and SessionProvider. We then provide that OpenTelemetry instance to all the producers so that they're able to generate any kind of signal. However, once built, the OpenTelemetry instance doesn't provide a way to get our configured tools, and some aren't even available upstream (such as the SessionProvider), so we need to find a way to make them available to producers.

Current approach

We provide producers with an instance of InstallationContext that holds all the tools we'd like to provide, as well as our OpenTelemetry instance. This approach works well for the consumer goal number 2, but introduces some issues for other goals as well:

• For the producer goal "3.2", regarding the ability to add new tools as needed in the future without introducing breaking changes, it forces us to set any new tool in the future with a default value that might be missed by consumers who might want to manually install instrumentations (and therefore would have to create instances of InstallationContext themselves), which could cause some instrumentations not to behave as expected.
• For the consumer goal "3.2", in case a consumer wants to manually install an instrumentation for an instance of OpenTelemetryRum that they got from our initializer function, they cannot properly build an instance of InstallationContext right now. This is because they have no access to some of the objects required to instantiate it (the Clock and the SessionProvider). This leaves them the option to create their own instances of those objects, which won't be aligned with the behavior that we configured into our OpenTelemetryRum impl, so they won't provide a proper context for instrumentations.

Solution proposals

• Making our OpenTelemetryRum interface able to provide these tools and pass the OpenTelemetryRum instance to the instrumentations. Added in option 1 and 2.
• Adding an OpenTelemetryRum.install(AndroidInstrumentation) function. That way, the OpenTelemetryRum implementation would be responsible for properly creating the InstallationContext object and passing it onto the instrumentation, removing that responsibility from the consumers.

2. Ensuring crash reports are saved as soon as possible

Description

We need to ensure that crash events are saved before the OS kills the host app.

Current approach

We are casting an OpenTelemetry instance to OpenTelemetrySdk to be able to flush logs promptly when a crash happens. This clashes with the requirement of not using methods from the OpenTelemetrySdk or workarounds in general.

Solution proposals

• Making our OpenTelemetryRum abstraction able to flush logs on-demand. As proposed here.
• Making OpenTelemetryRum implementations able to opt-in to support flushing logs on-demand. As proposed here.
• Avoiding on-demand flush and ensuring that our OpenTelemetryRum instance flushes data on a crash.

---

I think that's the high-level overview. Please let me know if I missed something or if there's some requirement/goal/challenge mentioned above that you don't agree with, @breedx-splk, @fractalwrench (or anyone else who might have an opinion on this issue).

[fractalwrench]

Thanks for putting this together @LikeTheSalad - I agree that captures all the requirements that have been discussed already.

For 2, I would add on that the requirement is a subset: really we want to persist all telemetry if the SDK can tell the process is terminating. For the purposes of getting the instrumentation API stable, I think that parity with the current functionality (flushing logs) is acceptable.

[bidetofevil]

Having read all the comments and draft PRs, including Jamie's, I think centralizing the SDK-to-Instrumentation contract in something like InstrumentationParams (nee InstallationContext), and making the Android Agent create that and do the actual installation, is a good strategy exposing what instrumentations can expect from the SDK.

For instrumentation producers, that object will provide everything it can depend on, and it SHOULDN'T include things that affect the whole SDK like flushing exporters, shutdown, or uninstall. Instrumentation shouldn't be able to directly modify the SDK instance on which it runs. For them, InstrumentationParams is the API they depend on.

For instrumentation consumers, having a OpenTelemetryRum.install(AndroidInstrumentation) seems reasonable - the agent can then do the work of calling AndroidInstrumentation.install() with the right InstrumentationParams, which instrumentation consumers shouldn't have to know about. For them, OpenTelemetryRum is the API they can depend on.

If we want to offer a configuration API for instrumentation, it should be a separate parameter on OpenTelemetryRum.install() -- but only if it applies to all instrumentation. Anything particular to a specific instrumentation should be configured on the instance passed into AndroidInstrumentation.install(). This may be out of the scope of this change?

Anyway, tl;dr, I support having separate APIs for producers and consumers (InstrumentationParams and OpenTelemetryRum, respectively), giving just enough access to configure and produce telemetry for the former, and install and configure instrumentation for the latter.

[bidetofevil]

Did some quick testing: OpenTelemetryRum.install(AndroidInstrumentation) can't be done right now because of the how modules depend on each other (i.e. agent-api doesn't depend on android-instrumentation). We basic need an existing module (or new module) that has the interfaces common for both the Producer and Consumer APIs.

This module will need to contain InstrumentationParams and everything it exposes, including AndroidInstrumentation and SessionProvider. It could just be agent-api if it's OK for session and android-instrumentation to depend on it, i.e. there isn't anything in agent-api we want to keep out of the hands of instrumentation producers.

[bidetofevil]

Building off @fractalwrench's example, here's the example of what I mean in PR form

[LikeTheSalad]

Thank you for your input (and for the draft), @bidetofevil. I'll take a detailed look later this week.