DEVELOPER FEEDBACK: android-agent 1.x stable APIs

[breedx-splk]

Hello Android developers! We encourage you to try this project in anger and to specifically provide us with feedback about the use of the android-agent and the OpenTelemetryRumInitializer APIs.

Our intention is to mark these APIs "stable" with a 1.x release in the near future, and your feedback will help inform this decision. ❤️

Please keep this in mind:

• The design of the android-agent is not to solve every odd/bespoke/specific use case. We do hope it will satisfy 90% of most common usages.
• Where more complicated, nonstandard configurations are required, the APIs in the core module MAY be used. Those APIs remain subject to change. We hope we won't need to discuss those (much?) here. Let's focus on the agent.

What do you like? What would make using it easier? Do you have a use case that's not covered that you think should be?

Please leave us a comment below. Thanks!

[svrnm]

Can I suggest that having https://opentelemetry.io/docs/platforms/client-apps/android/ filled with content to be a 1.0 requirement?:-) cc @open-telemetry/docs-approvers

[fractalwrench]

I agree that making the android-agent module stable in the near future is a good goal, and the general idea should be to produce a very minimal API that can have features added on later. I have a few additional suggestions (some of which might have already come up in the SIG meetings)

1. All public APIs should be in Kotlin (I notice that OtelRumConfig is currently in Java, and there are probably a few others)
2. We should probably assess the public API via the api dump files, and reduce the visibility of anything that isn't needed, either on a class or symbol level (e.g. I notice public constructors)
3. The android-agent module exposes several dependencies as api rather than implementation. Do we count those as part of the public API, or could they be hidden?
4. There are several concrete implementations exposed as part of the public API, e.g. SessionStorage.InMemory. Is it worth hiding these behind some interface/enum/alternative approach that would buy us flexibility to change the implementation in future?
5. We should document some strategy for API stability after 1.0. I think it's inevitable that we'll be stuck with at least one API that doesn't quite feel right, or needs to change due to requirements shifting, so it'd be good to explicitly mention how breaking changes would be avoided whilst allowing the API surface to evolve

[LikeTheSalad]

I agree with @fractalwrench's points. I think we've briefly talked about most of those topics in a SIG meeting, but we haven't created issues to address them. Generally speaking, I think we're almost there for 1.0.0, but we need the final "polish" where we would most likely add small breaking changes into OpenTelemetryRumInitializer to avoid using Java and also reduce the visibility of options that don't seem crucial, at least not for a first release, which would give us room to expand the API post-1.0.0 as needed, while minimizing the need to add breaking changes later by removing unnecessary APIs.

Still, I think it's good to have this issue, so that anyone interested is aware of the upcoming PRs and can let us know if some of the changes affect their use cases in a bad way, and/or if something is missing in OpenTelemetryRumInitializer right now that they'd like to see in version 1.0.0.

[fractalwrench]

I've thought about the API in some more depth and have a few proposals that I'd be curious to get thoughts on:

1. Change the type of OpenTelemetryRumInitializer.initialize's Application parameter to Context instead. This supertype is less specific & it gives end-users the option of initializing the SDK in places that aren't the application subclass. From my inspection of the codebase none of the specific features of Application are used, so Context should be fine

The potentially more controversial thought (2) is have we considered a DSL for OpenTelemetryRumInitializer.initalize instead of a list of parameters? To illustrate what I mean, this would alter the function signature so that everything is declarative:

OpenTelemetryRumInitializer.initialize(
    application,
    "http://example.com/4318/v1/traces",
    config = {
        endpointHeaders = mapOf("my_key" to "my_attr"),
        sessionConfig = {
            backgroundInactivityTimeout = 15.minutes,
        }
        // etc...
    }
)

The advantages of this approach is it's simple to make additive changes in a non-breaking way, whereas currently when adding a new parameter we'd need to create an overload & deprecate the legacy signature. It also solves the thorny problem of exposing concrete types/constructors in the API, like we currently do with SessionConfig.

And the final thought (3) is whether it should be possible configure multiple exporters. Or is 1 exporter just fine for the majority use-case?

[breedx-splk]

Change the type of OpenTelemetryRumInitializer.initialize's Application parameter to Context instead. This supertype is less specific & it gives end-users the option of initializing the SDK in places that aren't the application subclass.

While I generally agree that favoring less specific types on interfaces is a good clean-code practice, I question why we would want to allow (or encourage?) users to attempt to initialize the agent from anywhere other than as early as possible in the Application implementation? Doing the init much later is prone to causing problems, and this pattern of "you must initialize as early as possible" has been standard for instrumentation libraries for years. Just curious what you're thinking, @fractalwrench ?

[fractalwrench]

Jetpack Startup library is one scenario I was thinking of, although there are other potential use-cases (e.g. ContentProviders, asking for user opt-in before enabling any telemetry library). For Jetpack Startup library only Context is available and it will initialize before Application is instantiated.

To be clear I do think that initializing from an Application subclass remains a sensible default.

[fractalwrench]

I've created #1284 so discussion about Context can be moved there. I'm still curious to hear thoughts on using a DSL for OpenTelemetryRumInitializer.initalize instead of a list of parameters.

[LikeTheSalad]

Regarding @fractalwrench's proposals:

1. I'm a bit hesitant about changing Application by Context because I've had issues before with the Elastic agent, where it initially used Context, but then I had to switch to Application to register activity lifecycle callbacks, which I believe some of the existing instrumentations do. Some workarounds would enable using Application-specific features from a Context, by casting it, in case it is the Application Context, or getting the Application Context out of any Context object and then casting it to Application. I'm not sure the latter is always possible because I haven't tested it in every possible scenario (i.e., getting the application context out of any context), but if it is, then I think it's something that users can also do from when they don't have their app context at hand, such as from the Jetpack startup tool? And if it's not always possible and we decide to switch to Context, then I guess we'd have to document that some instrumentations might not work properly unless the application context is provided. I saw @fractalwrench's created an issue to discuss this further as I was writing this comment, so I'm happy to continue the discussion over there.
2. I'd love this approach. In fact, I've used it on a recent PR to allow configuring disk buffering, and it made configuring instrumentations much cleaner and easier to extend without having to have a huge list of params in the initializer.
3. In my experience, 1 exporter is fine, plus, the OTel API only allows one exporter per processor afaik, so to have multiple exporters in practice, we'd need to merge them into one, which is possible via the core's exporter customizer API. I think we could somehow expose a similar API from the RUM initializer in the future, if needed.