Relax the ServiceManager singleton nature (#671)

* allow setting the ServiceManager and only create if needed at build() time.

* create InstallationContext

* update AndroidInstrumentation.install() to take the InstallationContext.

* decouple instrumentations from ServiceManager singleton

* pull static usage up

* remove static create method

* remove static create() in favor of constructor

* pull operational object out of disk buffer config and promote to OTRB setter with same sane defaults.

* move opinionated default list of services out of ServiceManager interface and into impl

* remove static call and just test the impl

* cleanup

* eliminate test dep on ServiceManager static usage

* finally remove singleton nature from ServiceManager.

* add period

core/src/main/java/io/opentelemetry/android/OpenTelemetryRum.java

@@ -61,14 +61,16 @@ static OpenTelemetryRumBuilder builder(Application application, OtelRumConfig co
      * @param openTelemetrySdk The {@link OpenTelemetrySdk} that the user has already created.
      * @param discoverInstrumentations TRUE to look for instrumentations in the classpath and
      *     applying them automatically.
+     * @param serviceManager The ServiceManager instance
      */
     static SdkPreconfiguredRumBuilder builder(
             Application application,
             OpenTelemetrySdk openTelemetrySdk,
-            boolean discoverInstrumentations) {
-        ServiceManager.initialize(application);
+            boolean discoverInstrumentations,
+            ServiceManager serviceManager) {
+
         return new SdkPreconfiguredRumBuilder(
-                application, openTelemetrySdk, discoverInstrumentations, ServiceManager.get());
+                application, openTelemetrySdk, discoverInstrumentations, serviceManager);
     }
 
     /** Returns a no-op implementation of {@link OpenTelemetryRum}. */

core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java

@@ -9,10 +9,13 @@
 
 import android.app.Application;
 import android.util.Log;
+import androidx.annotation.NonNull;
 import io.opentelemetry.android.common.RumConstants;
 import io.opentelemetry.android.config.OtelRumConfig;
 import io.opentelemetry.android.features.diskbuffering.DiskBufferingConfiguration;
 import io.opentelemetry.android.features.diskbuffering.SignalFromDiskExporter;
+import io.opentelemetry.android.features.diskbuffering.scheduler.DefaultExportScheduleHandler;
+import io.opentelemetry.android.features.diskbuffering.scheduler.DefaultExportScheduler;
 import io.opentelemetry.android.features.diskbuffering.scheduler.ExportScheduleHandler;
 import io.opentelemetry.android.instrumentation.AndroidInstrumentation;
 import io.opentelemetry.android.internal.features.networkattrs.NetworkAttributesSpanAppender;
@@ -23,6 +26,8 @@
 import io.opentelemetry.android.internal.services.CacheStorage;
 import io.opentelemetry.android.internal.services.Preferences;
 import io.opentelemetry.android.internal.services.ServiceManager;
+import io.opentelemetry.android.internal.services.ServiceManagerImpl;
+import io.opentelemetry.android.internal.services.periodicwork.PeriodicWorkService;
 import io.opentelemetry.android.session.SessionManager;
 import io.opentelemetry.android.session.SessionProvider;
 import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
@@ -57,6 +62,7 @@
 import java.util.function.Consumer;
 import java.util.function.Function;
 import javax.annotation.Nullable;
+import kotlin.jvm.functions.Function0;
 
 /**
  * A builder of {@link OpenTelemetryRum}. It enabled configuring the OpenTelemetry SDK and disabling
@@ -86,6 +92,9 @@ public final class OpenTelemetryRumBuilder {
 
     private Resource resource;
 
+    @Nullable private ServiceManager serviceManager;
+    @Nullable private ExportScheduleHandler exportScheduleHandler;
+
     private static TextMapPropagator buildDefaultPropagator() {
         return TextMapPropagator.composite(
                 W3CTraceContextPropagator.getInstance(), W3CBaggagePropagator.getInstance());
@@ -265,13 +274,8 @@ public OpenTelemetryRumBuilder addLogRecordExporterCustomizer(
      * @return A new {@link OpenTelemetryRum} instance.
      */
     public OpenTelemetryRum build() {
-        ServiceManager.initialize(application);
-        return build(ServiceManager.get());
-    }
-
-    OpenTelemetryRum build(ServiceManager serviceManager) {
         InitializationEvents initializationEvents = InitializationEvents.get();
-        applyConfiguration(serviceManager, initializationEvents);
+        applyConfiguration(initializationEvents);
 
         DiskBufferingConfiguration diskBufferingConfiguration =
                 config.getDiskBufferingConfiguration();
@@ -280,8 +284,7 @@ OpenTelemetryRum build(ServiceManager serviceManager) {
         SignalFromDiskExporter signalFromDiskExporter = null;
         if (diskBufferingConfiguration.isEnabled()) {
             try {
-                StorageConfiguration storageConfiguration =
-                        createStorageConfiguration(serviceManager);
+                StorageConfiguration storageConfiguration = createStorageConfiguration();
                 final SpanExporter originalSpanExporter = spanExporter;
                 spanExporter =
                         SpanToDiskExporter.create(originalSpanExporter, storageConfiguration);
@@ -315,7 +318,7 @@ OpenTelemetryRum build(ServiceManager serviceManager) {
 
         otelSdkReadyListeners.forEach(listener -> listener.accept(sdk));
 
-        scheduleDiskTelemetryReader(signalFromDiskExporter, diskBufferingConfiguration);
+        scheduleDiskTelemetryReader(signalFromDiskExporter);
 
         SdkPreconfiguredRumBuilder delegate =
                 new SdkPreconfiguredRumBuilder(
@@ -324,15 +327,37 @@ OpenTelemetryRum build(ServiceManager serviceManager) {
                         timeoutHandler,
                         sessionManager,
                         config.shouldDiscoverInstrumentations(),
-                        serviceManager);
+                        getServiceManager());
         instrumentations.forEach(delegate::addInstrumentation);
         return delegate.build();
     }
 
-    private StorageConfiguration createStorageConfiguration(ServiceManager serviceManager)
-            throws IOException {
-        Preferences preferences = serviceManager.getPreferences();
-        CacheStorage storage = serviceManager.getCacheStorage();
+    @NonNull
+    private ServiceManager getServiceManager() {
+        if (serviceManager == null) {
+            serviceManager = ServiceManagerImpl.Companion.create(application);
+        }
+        return serviceManager;
+    }
+
+    public OpenTelemetryRumBuilder setServiceManager(ServiceManager serviceManager) {
+        this.serviceManager = serviceManager;
+        return this;
+    }
+
+    /**
+     * Sets a scheduler that will take care of periodically read data stored in disk and export it.
+     * If not specified, the default schedule exporter will be used.
+     */
+    public OpenTelemetryRumBuilder setExportScheduleHandler(
+            ExportScheduleHandler exportScheduleHandler) {
+        this.exportScheduleHandler = exportScheduleHandler;
+        return this;
+    }
+
+    private StorageConfiguration createStorageConfiguration() throws IOException {
+        Preferences preferences = getServiceManager().getPreferences();
+        CacheStorage storage = getServiceManager().getCacheStorage();
         DiskBufferingConfiguration config = this.config.getDiskBufferingConfiguration();
         DiskManager diskManager = new DiskManager(storage, preferences, config);
         return StorageConfiguration.builder()
@@ -347,11 +372,18 @@ private StorageConfiguration createStorageConfiguration(ServiceManager serviceMa
                 .build();
     }
 
-    private void scheduleDiskTelemetryReader(
-            @Nullable SignalFromDiskExporter signalExporter,
-            DiskBufferingConfiguration diskBufferingConfiguration) {
-        ExportScheduleHandler exportScheduleHandler =
-                diskBufferingConfiguration.getExportScheduleHandler();
+    private void scheduleDiskTelemetryReader(@Nullable SignalFromDiskExporter signalExporter) {
+
+        if (exportScheduleHandler == null) {
+            ServiceManager serviceManager = getServiceManager();
+            // TODO: Is it safe to get the work service yet here? If so, we can
+            // avoid all this lazy supplier stuff....
+            Function0<PeriodicWorkService> getWorkService = serviceManager::getPeriodicWorkService;
+            exportScheduleHandler =
+                    new DefaultExportScheduleHandler(
+                            new DefaultExportScheduler(getWorkService), getWorkService);
+        }
+
         if (signalExporter == null) {
             // Disabling here allows to cancel previously scheduled exports using tools that
             // can run even after the app has been terminated (such as WorkManager).
@@ -378,8 +410,7 @@ public OpenTelemetryRumBuilder addOtelSdkReadyListener(Consumer<OpenTelemetrySdk
     }
 
     /** Leverage the configuration to wire up various instrumentation components. */
-    private void applyConfiguration(
-            ServiceManager serviceManager, InitializationEvents initializationEvents) {
+    private void applyConfiguration(InitializationEvents initializationEvents) {
         if (config.shouldGenerateSdkInitializationEvents()) {
             initializationEvents.recordConfiguration(config);
         }
@@ -402,7 +433,7 @@ private void applyConfiguration(
                     (tracerProviderBuilder, app) -> {
                         SpanProcessor networkAttributesSpanAppender =
                                 NetworkAttributesSpanAppender.create(
-                                        serviceManager.getCurrentNetworkProvider());
+                                        getServiceManager().getCurrentNetworkProvider());
                         return tracerProviderBuilder.addSpanProcessor(
                                 networkAttributesSpanAppender);
                     });
@@ -415,7 +446,7 @@ private void applyConfiguration(
                     (tracerProviderBuilder, app) -> {
                         SpanProcessor screenAttributesAppender =
                                 new ScreenAttributesSpanProcessor(
-                                        serviceManager.getVisibleScreenService());
+                                        getServiceManager().getVisibleScreenService());
                         return tracerProviderBuilder.addSpanProcessor(screenAttributesAppender);
                     });
         }

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

@@ -8,6 +8,7 @@ package io.opentelemetry.android
 import android.app.Application
 import io.opentelemetry.android.instrumentation.AndroidInstrumentation
 import io.opentelemetry.android.instrumentation.AndroidInstrumentationLoader
+import io.opentelemetry.android.instrumentation.InstallationContext
 import io.opentelemetry.android.internal.services.ServiceManager
 import io.opentelemetry.android.session.SessionManager
 import io.opentelemetry.sdk.OpenTelemetrySdk
@@ -53,8 +54,9 @@ class SdkPreconfiguredRumBuilder
             val openTelemetryRum = OpenTelemetryRumImpl(sdk, sessionManager)
 
             // Install instrumentations
+            val ctx = InstallationContext(application, openTelemetryRum.openTelemetry, serviceManager)
             for (instrumentation in getInstrumentations()) {
-                instrumentation.install(application, openTelemetryRum.openTelemetry)
+                instrumentation.install(ctx)
             }
 
             return openTelemetryRum

core/src/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java

@@ -5,8 +5,6 @@
 
 package io.opentelemetry.android.features.diskbuffering;
 
-import io.opentelemetry.android.features.diskbuffering.scheduler.DefaultExportScheduleHandler;
-import io.opentelemetry.android.features.diskbuffering.scheduler.ExportScheduleHandler;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -18,15 +16,13 @@ public final class DiskBufferingConfiguration {
 
     private final boolean enabled;
     private final int maxCacheSize;
-    private final ExportScheduleHandler exportScheduleHandler;
     private final long maxFileAgeForWriteMillis;
     private final long minFileAgeForReadMillis;
     private final long maxFileAgeForReadMillis;
 
     private DiskBufferingConfiguration(Builder builder) {
         enabled = builder.enabled;
         maxCacheSize = builder.maxCacheSize;
-        exportScheduleHandler = builder.exportScheduleHandler;
         maxFileAgeForWriteMillis = builder.maxFileAgeForWriteMillis;
         minFileAgeForReadMillis = builder.minFileAgeForReadMillis;
         maxFileAgeForReadMillis = builder.maxFileAgeForReadMillis;
@@ -48,10 +44,6 @@ public int getMaxCacheFileSize() {
         return MAX_FILE_SIZE;
     }
 
-    public ExportScheduleHandler getExportScheduleHandler() {
-        return exportScheduleHandler;
-    }
-
     public long getMaxFileAgeForWriteMillis() {
         return maxFileAgeForWriteMillis;
     }
@@ -71,8 +63,6 @@ public static final class Builder {
         private long minFileAgeForReadMillis = TimeUnit.SECONDS.toMillis(33);
         private long maxFileAgeForReadMillis = TimeUnit.HOURS.toMillis(18);
 
-        private ExportScheduleHandler exportScheduleHandler = DefaultExportScheduleHandler.create();
-
         /** Enables or disables disk buffering. */
         public Builder setEnabled(boolean enabled) {
             this.enabled = enabled;
@@ -113,15 +103,6 @@ public Builder setMaxCacheSize(int maxCacheSize) {
             return this;
         }
 
-        /**
-         * Sets a scheduler that will take care of periodically read data stored in disk and export
-         * it.
-         */
-        public Builder setExportScheduleHandler(ExportScheduleHandler exportScheduleHandler) {
-            this.exportScheduleHandler = exportScheduleHandler;
-            return this;
-        }
-
         public DiskBufferingConfiguration build() {
             // See note in StorageConfiguration.getMinFileAgeForReadMillis()
             if (minFileAgeForReadMillis <= maxFileAgeForWriteMillis) {

core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt

@@ -5,7 +5,6 @@
 
 package io.opentelemetry.android.features.diskbuffering.scheduler
 
-import io.opentelemetry.android.internal.services.ServiceManager
 import io.opentelemetry.android.internal.services.periodicwork.PeriodicWorkService
 import java.util.concurrent.atomic.AtomicBoolean
 
@@ -22,15 +21,4 @@ class DefaultExportScheduleHandler(
             periodicWorkService.enqueue(exportScheduler)
         }
     }
-
-    companion object {
-        @JvmStatic
-        fun create(): DefaultExportScheduleHandler {
-            return DefaultExportScheduleHandler(
-                DefaultExportScheduler.create(),
-            ) {
-                ServiceManager.get().getPeriodicWorkService()
-            }
-        }
-    }
 }

core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt

@@ -8,7 +8,6 @@ package io.opentelemetry.android.features.diskbuffering.scheduler
 import android.util.Log
 import io.opentelemetry.android.common.RumConstants.OTEL_RUM_LOG_TAG
 import io.opentelemetry.android.features.diskbuffering.SignalFromDiskExporter
-import io.opentelemetry.android.internal.services.ServiceManager
 import io.opentelemetry.android.internal.services.periodicwork.PeriodicRunnable
 import io.opentelemetry.android.internal.services.periodicwork.PeriodicWorkService
 import java.io.IOException
@@ -18,12 +17,6 @@ class DefaultExportScheduler(periodicWorkServiceProvider: () -> PeriodicWorkServ
     PeriodicRunnable(periodicWorkServiceProvider) {
     companion object {
         private val DELAY_BEFORE_NEXT_EXPORT_IN_MILLIS = TimeUnit.SECONDS.toMillis(10)
-
-        fun create(): DefaultExportScheduler {
-            return DefaultExportScheduler {
-                ServiceManager.get().getPeriodicWorkService()
-            }
-        }
     }
 
     override fun onRun() {

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

@@ -5,9 +5,7 @@
 
 package io.opentelemetry.android.instrumentation
 
-import android.app.Application
 import io.opentelemetry.android.OpenTelemetryRum
-import io.opentelemetry.api.OpenTelemetry
 
 /**
  * This interface defines a tool that automatically generates telemetry for a specific use-case,
@@ -30,11 +28,7 @@ interface AndroidInstrumentation {
      * only be called from [OpenTelemetryRum]'s builder once the [OpenTelemetryRum] instance is initialized and ready
      * to use for generating telemetry.
      *
-     * @param application The Android application being instrumented.
-     * @param openTelemetry The [OpenTelemetry] instance to use for creating signals.
+     * @param ctx The InstallationContext under which the instrumentation is being installed.
      */
-    fun install(
-        application: Application,
-        openTelemetry: OpenTelemetry,
-    )
+    fun install(ctx: InstallationContext)
 }

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

@@ -0,0 +1,16 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.android.instrumentation
+
+import android.app.Application
+import io.opentelemetry.android.internal.services.ServiceManager
+import io.opentelemetry.api.OpenTelemetry
+
+data class InstallationContext(
+    val application: Application,
+    val openTelemetry: OpenTelemetry,
+    val serviceManager: ServiceManager,
+)