Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modularize project so Android instrumentations can used by SDKs that provide their own OpenTelemetry instance #702

Open
bidetofevil opened this issue Nov 20, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@bidetofevil
Copy link
Contributor

Background

Conceptually, the core module provides serves three purposes:

  1. Initializes an instance of OpenTelemetry with reasonable defaults
  2. Defines an API for Android instrumentations and orchestrates the starting of the configured instrumentations
  3. Provides a set of opinionated abstractions over the Android platform that instrumentations can depend on (i.e. Services)

As an app developer, using this project is straightforward: include the android-agent module, configure and start the SDK, profit.

However, as @surbhiia points out in #669 , vendors that have their own version of android-agent that are used with their own custom instrumentation can't use the instrumentations provided by this directly.

Part of that is intentional:- the platform abstraction services are a dependency, so they must be provided by the project to the instrumentations for them to work. But is there a good reason why the OpenTelemetry instance has to be created in core? To put in another way, does the use case outlined in #669 seems reasonable? I think the answer is yes.

This issue is about doing all the work to enable the use of instrumentations defined in this project independent from the initialization of the OTel SDK.

Goals

  • Restructure core so that purpose 1 listed about (i.e. initialize OpenTelemetry instance) is outside of its direct purview
  • Refactor the initialization of the services so they are done lazily, in case they are not all needed for the specific set of instrumentation that are in use.

Non-Goals

  • Make the interfaces of the platform abstractions public, i.e. they should remain internal and not be part of the supported public API. In other words, those interfaces and implementations should not be explicitly depended on by app and they can change form version to version with no warning.
  • Make the interface for the Android instrumentation public, i.e. apps can't create their own instances of instrumentation that are then managed by core. In other words, AndroidInstrumentation will NOT be a publicly supported API.
@LikeTheSalad
Copy link
Contributor

Thank you, @bidetofevil!

It'd be awesome if we could come up with a way to use instrumentations independently 👍

You touch on different topics, some of which I fully understand and agree with (such as the part that reads: "Refactor the initialization of the services so they are done lazily, in case they are not all needed for the specific set of instrumentation that are in use") though I'm not fully sure I follow some others, which are the ones I'd like to expand on below:

Restructure core so that purpose 1 listed about (i.e. initialize OpenTelemetry instance) is outside of its direct purview

I can't find the connection between initializing the OpenTelemetry instance in core, with not being able to use instrumentations independently. My understanding is that instrumentations make use of an OpenTelemetry instance regardless of how it was created, so for example, one way to use an instrumentation independently could be by calling its AndroidInstrumentation.install implementation and passing any OpenTelemetry instance to it, even if it doesn't come from core.

The problem with calling AndroidInstrumentation.install right now is that it requires passing an instance of the services manager, which is something users shouldn't know about.

AndroidInstrumentation will NOT be a publicly supported API.

The purpose of AndroidInstrumentation is to have a "handle" to initialize an instrumentation in a way that we can provide the OTel instance to it so that it can start sending telemetry. So if we remove this from the public API, how would users be able to initialize an instrumentation with their OTel instance?


Those are my questions based on what I understood from your proposal, though I might've misunderstood something so please let me know if that's the case 😅

@TomasChladekSL
Copy link

Here is a description of my proposed solution.

Every instrumentation will implement the AndroidInstrumentation interface from the new instrumentation module. This interface will have a function onInstall(context: Context) that will be called from core and a property listeners that will be used to emit events. This allows using instrumentation completely independently from core and allows using your any OpenTelemetry implementation.

The argument named context is from the android.content.Context package. This means application, openTelemetry, and serviceManager will not be provided. The ServiceManager class offers VisibleScreenService, AppLifecycleService, CurrentNetworkProvider, etc. These classes can be rewritten into instrumentation, and if another instrumentation wants to know, for example, the current network provider, it will depend on that instrumentation and listen for events.

@surbhiia
Copy link
Contributor

surbhiia commented Dec 3, 2024

@TomasChladekSL Thanks for your solution proposal. I have a few questions on same:

This means application, openTelemetry, and serviceManager will not be provided

How will the instrumenter for a given instrumentation be initialized without the opentelemetry sdk instance? For example: ANR instrumenter initialization

These classes can be rewritten into instrumentation, and if another instrumentation wants to know, for example, the current network provider, it will depend on that instrumentation and listen for events.

  • VisibleServiceScreen also supplies screen attributes for ScreenAttributesSpanProcessor and similarly CurrentNetworkProvider for NetworkAttributesSpanProcessor. :core would need to depend on the said instrumentations (activity lifecycle instrumentation or fragment lifecycle for screen and network change detection instrumentation for network attributes) for these attributes. That might not be ideal, any workaround for this?
  • If I understand the solution correctly, this seems like adding additional complexity by forcing instrumentations to depend on one another for screen/network attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants