Instrumentation registry changes (#516)

* Moving AndroidInstrumentationRegistryImpl to an internal package

* Removing register method from AndroidInstrumentationServices

* Updating tests

* Updating docs

* Updating docs

* Convenience method AndroidInstrumentationServices.getService

* Renaming AndroidInstrumentationServicesImpl.register to registerForTest

* Renaming AndroidInstrumentationServices to AndroidInstrumentationLoader

* Renaming vars

* Renaming AndroidInstrumentationLoader.Companion.getInstrumentation

android-agent/src/main/kotlin/io/opentelemetry/android/agent/OtelRumConfigExtensions.kt

@@ -6,7 +6,7 @@
 package io.opentelemetry.android.agent
 
 import io.opentelemetry.android.config.OtelRumConfig
-import io.opentelemetry.android.instrumentation.AndroidInstrumentationRegistry
+import io.opentelemetry.android.instrumentation.AndroidInstrumentationLoader
 import io.opentelemetry.android.instrumentation.activity.ActivityLifecycleInstrumentation
 import io.opentelemetry.android.instrumentation.anr.AnrInstrumentation
 import io.opentelemetry.android.instrumentation.common.ScreenNameExtractor
@@ -32,49 +32,49 @@ import java.time.Duration
  */
 
 fun OtelRumConfig.setActivityTracerCustomizer(customizer: (Tracer) -> Tracer): OtelRumConfig {
-    AndroidInstrumentationRegistry.get().get(ActivityLifecycleInstrumentation::class.java)
+    AndroidInstrumentationLoader.getInstrumentation(ActivityLifecycleInstrumentation::class.java)
         ?.setTracerCustomizer(customizer)
     return this
 }
 
 fun OtelRumConfig.setActivityNameExtractor(screenNameExtractor: ScreenNameExtractor): OtelRumConfig {
-    AndroidInstrumentationRegistry.get().get(ActivityLifecycleInstrumentation::class.java)
+    AndroidInstrumentationLoader.getInstrumentation(ActivityLifecycleInstrumentation::class.java)
         ?.setScreenNameExtractor(screenNameExtractor)
     return this
 }
 
 fun OtelRumConfig.setFragmentTracerCustomizer(customizer: (Tracer) -> Tracer): OtelRumConfig {
-    AndroidInstrumentationRegistry.get().get(FragmentLifecycleInstrumentation::class.java)
+    AndroidInstrumentationLoader.getInstrumentation(FragmentLifecycleInstrumentation::class.java)
         ?.setTracerCustomizer(customizer)
     return this
 }
 
 fun OtelRumConfig.setFragmentNameExtractor(screenNameExtractor: ScreenNameExtractor): OtelRumConfig {
-    AndroidInstrumentationRegistry.get().get(FragmentLifecycleInstrumentation::class.java)
+    AndroidInstrumentationLoader.getInstrumentation(FragmentLifecycleInstrumentation::class.java)
         ?.setScreenNameExtractor(screenNameExtractor)
     return this
 }
 
 fun OtelRumConfig.addAnrAttributesExtractor(extractor: AttributesExtractor<Array<StackTraceElement>, Void>): OtelRumConfig {
-    AndroidInstrumentationRegistry.get().get(AnrInstrumentation::class.java)
+    AndroidInstrumentationLoader.getInstrumentation(AnrInstrumentation::class.java)
         ?.addAttributesExtractor(extractor)
     return this
 }
 
 fun OtelRumConfig.addCrashAttributesExtractor(extractor: AttributesExtractor<CrashDetails, Void>): OtelRumConfig {
-    AndroidInstrumentationRegistry.get().get(CrashReporterInstrumentation::class.java)
+    AndroidInstrumentationLoader.getInstrumentation(CrashReporterInstrumentation::class.java)
         ?.addAttributesExtractor(extractor)
     return this
 }
 
 fun OtelRumConfig.addNetworkChangeAttributesExtractor(extractor: AttributesExtractor<CurrentNetwork, Void>): OtelRumConfig {
-    AndroidInstrumentationRegistry.get().get(NetworkChangeInstrumentation::class.java)
+    AndroidInstrumentationLoader.getInstrumentation(NetworkChangeInstrumentation::class.java)
         ?.addAttributesExtractor(extractor)
     return this
 }
 
 fun OtelRumConfig.setSlowRenderingDetectionPollInterval(interval: Duration): OtelRumConfig {
-    AndroidInstrumentationRegistry.get().get(SlowRenderingInstrumentation::class.java)
+    AndroidInstrumentationLoader.getInstrumentation(SlowRenderingInstrumentation::class.java)
         ?.setSlowRenderingDetectionPollInterval(interval)
     return this
 }

core/src/main/java/io/opentelemetry/android/SdkPreconfiguredRumBuilder.kt

@@ -7,7 +7,7 @@ package io.opentelemetry.android
 
 import android.app.Application
 import io.opentelemetry.android.instrumentation.AndroidInstrumentation
-import io.opentelemetry.android.instrumentation.AndroidInstrumentationRegistry
+import io.opentelemetry.android.instrumentation.AndroidInstrumentationLoader
 import io.opentelemetry.android.internal.services.ServiceManager
 import io.opentelemetry.android.internal.services.applifecycle.AppLifecycleService
 import io.opentelemetry.sdk.OpenTelemetrySdk
@@ -68,7 +68,7 @@ class SdkPreconfiguredRumBuilder
 
         private fun getInstrumentations(): List<AndroidInstrumentation> {
             if (discoverInstrumentations) {
-                instrumentations.addAll(AndroidInstrumentationRegistry.get().getAll())
+                instrumentations.addAll(AndroidInstrumentationLoader.get().getAll())
             }
 
             return instrumentations

core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentation.kt

@@ -22,9 +22,6 @@ import io.opentelemetry.android.OpenTelemetryRum
  * Even though users shouldn't have to write code to make an AndroidInstrumentation implementation work,
  * implementations should expose configurable options whenever possible to allow users to customize relevant
  * options depending on the use-case.
- *
- * Access to an implementation, either to configure it or to install it, must be made through
- * [AndroidInstrumentationRegistry.get] or [AndroidInstrumentationRegistry.getAll].
  */
 interface AndroidInstrumentation {
     /**

core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentationRegistry.kt → core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentationLoader.kt

@@ -5,17 +5,19 @@
 
 package io.opentelemetry.android.instrumentation
 
+import io.opentelemetry.android.internal.instrumentation.AndroidInstrumentationLoaderImpl
+
 /**
- * Stores and provides all the available instrumentations.
+ * Loads and provides [AndroidInstrumentation] instances from the runtime classpath.
  */
-interface AndroidInstrumentationRegistry {
+interface AndroidInstrumentationLoader {
     /**
      * Provides a single instrumentation if available.
      *
      * @param type The type of the instrumentation to retrieve.
      * @return The instrumentation instance if available, null otherwise.
      */
-    fun <T : AndroidInstrumentation> get(type: Class<out T>): T?
+    fun <T : AndroidInstrumentation> getByType(type: Class<out T>): T?
 
     /**
      * Provides all registered instrumentations.
@@ -24,26 +26,25 @@ interface AndroidInstrumentationRegistry {
      */
     fun getAll(): Collection<AndroidInstrumentation>
 
-    /**
-     * Stores an instrumentation as long as there is not other instrumentation already registered with the same
-     * type.
-     *
-     * @param instrumentation The instrumentation to register.
-     * @throws IllegalStateException If the instrumentation couldn't be registered.
-     */
-    fun register(instrumentation: AndroidInstrumentation)
-
     companion object {
-        private var instance: AndroidInstrumentationRegistry? = null
+        private var instance: AndroidInstrumentationLoader? = null
 
         @JvmStatic
-        fun get(): AndroidInstrumentationRegistry {
+        fun get(): AndroidInstrumentationLoader {
             if (instance == null) {
-                instance = AndroidInstrumentationRegistryImpl()
+                instance = AndroidInstrumentationLoaderImpl()
             }
             return instance!!
         }
 
+        /**
+         * Convenience method for [AndroidInstrumentationLoader.getByType].
+         */
+        @JvmStatic
+        fun <T : AndroidInstrumentation> getInstrumentation(type: Class<out T>): T? {
+            return get().getByType(type)
+        }
+
         @JvmStatic
         fun resetForTest() {
             instance = null

core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentationRegistryImpl.kt → core/src/main/java/io/opentelemetry/android/internal/instrumentation/AndroidInstrumentationLoaderImpl.kt

@@ -3,18 +3,24 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.android.instrumentation
+package io.opentelemetry.android.internal.instrumentation
 
+import io.opentelemetry.android.instrumentation.AndroidInstrumentation
+import io.opentelemetry.android.instrumentation.AndroidInstrumentationLoader
 import java.util.ServiceLoader
 
-internal class AndroidInstrumentationRegistryImpl : AndroidInstrumentationRegistry {
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change
+ * at any time.
+ */
+class AndroidInstrumentationLoaderImpl : AndroidInstrumentationLoader {
     private val instrumentations: MutableMap<Class<out AndroidInstrumentation>, AndroidInstrumentation> by lazy {
         ServiceLoader.load(AndroidInstrumentation::class.java).associateBy { it.javaClass }
             .toMutableMap()
     }
 
     @Suppress("UNCHECKED_CAST")
-    override fun <T : AndroidInstrumentation> get(type: Class<out T>): T? {
+    override fun <T : AndroidInstrumentation> getByType(type: Class<out T>): T? {
         return instrumentations[type] as? T
     }
 
@@ -23,7 +29,7 @@ internal class AndroidInstrumentationRegistryImpl : AndroidInstrumentationRegist
     }
 
     @Throws(IllegalStateException::class)
-    override fun register(instrumentation: AndroidInstrumentation) {
+    fun registerForTest(instrumentation: AndroidInstrumentation) {
         if (instrumentation::class.java in instrumentations) {
             throw IllegalStateException("Instrumentation with type '${instrumentation::class.java}' already exists.")
         }

core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java

@@ -34,8 +34,9 @@
 import io.opentelemetry.android.features.diskbuffering.SignalFromDiskExporter;
 import io.opentelemetry.android.features.diskbuffering.scheduler.ExportScheduleHandler;
 import io.opentelemetry.android.instrumentation.AndroidInstrumentation;
-import io.opentelemetry.android.instrumentation.AndroidInstrumentationRegistry;
+import io.opentelemetry.android.instrumentation.AndroidInstrumentationLoader;
 import io.opentelemetry.android.internal.initialization.InitializationEvents;
+import io.opentelemetry.android.internal.instrumentation.AndroidInstrumentationLoaderImpl;
 import io.opentelemetry.android.internal.services.CacheStorage;
 import io.opentelemetry.android.internal.services.Preferences;
 import io.opentelemetry.android.internal.services.ServiceManager;
@@ -116,7 +117,7 @@ public void setup() {
     public void tearDown() throws Exception {
         SignalFromDiskExporter.resetForTesting();
         InitializationEvents.resetForTest();
-        AndroidInstrumentationRegistry.resetForTest();
+        AndroidInstrumentationLoader.resetForTest();
         mocks.close();
     }
 
@@ -198,7 +199,9 @@ public void shouldInstallInstrumentation() {
         SessionIdTimeoutHandler timeoutHandler = mock();
         AndroidInstrumentation localInstrumentation = mock();
         AndroidInstrumentation classpathInstrumentation = mock();
-        AndroidInstrumentationRegistry.get().register(classpathInstrumentation);
+        AndroidInstrumentationLoaderImpl androidInstrumentationServices =
+                (AndroidInstrumentationLoaderImpl) AndroidInstrumentationLoader.get();
+        androidInstrumentationServices.registerForTest(classpathInstrumentation);
 
         new OpenTelemetryRumBuilder(application, buildConfig(), timeoutHandler)
                 .addInstrumentation(localInstrumentation)
@@ -216,7 +219,9 @@ public void shouldInstallInstrumentation_excludingClasspathImplsWhenRequestedInC
         SessionIdTimeoutHandler timeoutHandler = mock();
         AndroidInstrumentation localInstrumentation = mock();
         AndroidInstrumentation classpathInstrumentation = mock();
-        AndroidInstrumentationRegistry.get().register(classpathInstrumentation);
+        AndroidInstrumentationLoaderImpl androidInstrumentationServices =
+                (AndroidInstrumentationLoaderImpl) AndroidInstrumentationLoader.get();
+        androidInstrumentationServices.registerForTest(classpathInstrumentation);
 
         new OpenTelemetryRumBuilder(
                         application,

core/src/test/java/io/opentelemetry/android/instrumentation/AndroidInstrumentationRegistryTest.kt → core/src/test/java/io/opentelemetry/android/instrumentation/AndroidInstrumentationLoaderTest.kt

@@ -8,11 +8,11 @@ package io.opentelemetry.android.instrumentation
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
 
-class AndroidInstrumentationRegistryTest {
+class AndroidInstrumentationLoaderTest {
     @Test
     fun `Verify singleton`() {
-        val registry = AndroidInstrumentationRegistry.get()
+        val registry = AndroidInstrumentationLoader.get()
 
-        assertThat(registry).isEqualTo(AndroidInstrumentationRegistry.get())
+        assertThat(registry).isEqualTo(AndroidInstrumentationLoader.get())
     }
 }

core/src/test/java/io/opentelemetry/android/internal/instrumentation/AndroidInstrumentationLoaderImplTest.kt

@@ -0,0 +1,40 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.android.internal.instrumentation
+
+import io.mockk.mockk
+import io.opentelemetry.android.instrumentation.TestAndroidInstrumentation
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+
+class AndroidInstrumentationLoaderImplTest {
+    private lateinit var loader: AndroidInstrumentationLoaderImpl
+
+    @BeforeEach
+    fun setUp() {
+        loader = AndroidInstrumentationLoaderImpl()
+    }
+
+    @Test
+    fun `Find implementations available in the classpath when querying an instrumentation`() {
+        val instrumentation = loader.getByType(TestAndroidInstrumentation::class.java)!!
+
+        assertThat(instrumentation.installed).isFalse()
+
+        instrumentation.install(mockk(), mockk())
+
+        assertThat(loader.getByType(TestAndroidInstrumentation::class.java)!!.installed).isTrue()
+    }
+
+    @Test
+    fun `Find implementations available in the classpath when querying all instrumentations`() {
+        val instrumentations = loader.getAll()
+
+        assertThat(instrumentations).hasSize(1)
+        assertThat(instrumentations.first()).isInstanceOf(TestAndroidInstrumentation::class.java)
+    }
+}