From ef9612605713cdd26635f89a724ed21cc008f6fd Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 15:48:49 -0700 Subject: [PATCH 01/14] allow setting the ServiceManager and only create if needed at build() time. --- .../android/OpenTelemetryRumBuilder.java | 44 ++++++++++++------- .../android/OpenTelemetryRumBuilderTest.java | 19 +++++--- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java index 1b33bd02..bdc6af7d 100644 --- a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java +++ b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java @@ -9,6 +9,9 @@ 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; @@ -86,6 +89,9 @@ public final class OpenTelemetryRumBuilder { private Resource resource; + @Nullable + private ServiceManager serviceManager; + private static TextMapPropagator buildDefaultPropagator() { return TextMapPropagator.composite( W3CTraceContextPropagator.getInstance(), W3CBaggagePropagator.getInstance()); @@ -265,13 +271,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(); @@ -281,7 +282,7 @@ OpenTelemetryRum build(ServiceManager serviceManager) { if (diskBufferingConfiguration.isEnabled()) { try { StorageConfiguration storageConfiguration = - createStorageConfiguration(serviceManager); + createStorageConfiguration(); final SpanExporter originalSpanExporter = spanExporter; spanExporter = SpanToDiskExporter.create(originalSpanExporter, storageConfiguration); @@ -324,15 +325,29 @@ OpenTelemetryRum build(ServiceManager serviceManager) { timeoutHandler, sessionManager, config.shouldDiscoverInstrumentations(), - serviceManager); + getServiceManager()); instrumentations.forEach(delegate::addInstrumentation); return delegate.build(); } - private StorageConfiguration createStorageConfiguration(ServiceManager serviceManager) + @NonNull + private ServiceManager getServiceManager() { + if(serviceManager == null){ + ServiceManager.initialize(application); + serviceManager = ServiceManager.get(); + } + return serviceManager; + } + + public OpenTelemetryRumBuilder setServiceManager(ServiceManager serviceManager) { + this.serviceManager = serviceManager; + return this; + } + + private StorageConfiguration createStorageConfiguration() throws IOException { - Preferences preferences = serviceManager.getPreferences(); - CacheStorage storage = serviceManager.getCacheStorage(); + Preferences preferences = getServiceManager().getPreferences(); + CacheStorage storage = getServiceManager().getCacheStorage(); DiskBufferingConfiguration config = this.config.getDiskBufferingConfiguration(); DiskManager diskManager = new DiskManager(storage, preferences, config); return StorageConfiguration.builder() @@ -378,8 +393,7 @@ public OpenTelemetryRumBuilder addOtelSdkReadyListener(Consumer { SpanProcessor networkAttributesSpanAppender = NetworkAttributesSpanAppender.create( - serviceManager.getCurrentNetworkProvider()); + getServiceManager().getCurrentNetworkProvider()); return tracerProviderBuilder.addSpanProcessor( networkAttributesSpanAppender); }); @@ -415,7 +429,7 @@ private void applyConfiguration( (tracerProviderBuilder, app) -> { SpanProcessor screenAttributesAppender = new ScreenAttributesSpanProcessor( - serviceManager.getVisibleScreenService()); + getServiceManager().getVisibleScreenService()); return tracerProviderBuilder.addSpanProcessor(screenAttributesAppender); }); } diff --git a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java index 6ada4aba..265301d6 100644 --- a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java +++ b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java @@ -124,7 +124,9 @@ public void shouldRegisterApplicationStateWatcher() { ServiceManager serviceManager = createServiceManager(); AppLifecycleService appLifecycleService = serviceManager.getAppLifecycleService(); - makeBuilder().build(serviceManager); + makeBuilder() + .setServiceManager(serviceManager) + .build(); verify(appLifecycleService).registerListener(isA(ApplicationStateListener.class)); } @@ -203,7 +205,8 @@ public void shouldInstallInstrumentation() { new OpenTelemetryRumBuilder(application, buildConfig(), timeoutHandler) .addInstrumentation(localInstrumentation) - .build(serviceManager); + .setServiceManager(serviceManager) + .build(); verify(serviceManager.getAppLifecycleService()).registerListener(timeoutHandler); @@ -226,7 +229,8 @@ public void shouldInstallInstrumentation_excludingClasspathImplsWhenRequestedInC buildConfig().disableInstrumentationDiscovery(), timeoutHandler) .addInstrumentation(localInstrumentation) - .build(serviceManager); + .setServiceManager(serviceManager) + .build(); verify(serviceManager.getAppLifecycleService()).registerListener(timeoutHandler); @@ -328,7 +332,7 @@ public void diskBufferingEnabled() { .build()); ArgumentCaptor exporterCaptor = ArgumentCaptor.forClass(SpanExporter.class); - OpenTelemetryRum.builder(application, config).build(serviceManager); + OpenTelemetryRum.builder(application, config).setServiceManager(serviceManager).build(); assertThat(SignalFromDiskExporter.get()).isNotNull(); verify(scheduleHandler).enable(); @@ -357,7 +361,7 @@ public void diskBufferingEnabled_when_exception_thrown() { .setExportScheduleHandler(scheduleHandler) .build()); - OpenTelemetryRum.builder(application, config).build(serviceManager); + OpenTelemetryRum.builder(application, config).setServiceManager(serviceManager).build(); verify(initializationEvents).spanExporterInitialized(exporterCaptor.capture()); verify(scheduleHandler, never()).enable(); @@ -372,7 +376,8 @@ public void sdkReadyListeners() { AtomicReference seen = new AtomicReference<>(); OpenTelemetryRum.builder(application, config) .addOtelSdkReadyListener(seen::set) - .build(createServiceManager()); + .setServiceManager(createServiceManager()) + .build(); assertThat(seen.get()).isNotNull(); } @@ -437,7 +442,7 @@ public void verifyServicesAreStarted() { ServiceManager serviceManager = mock(); doReturn(mock(AppLifecycleService.class)).when(serviceManager).getAppLifecycleService(); - makeBuilder().build(serviceManager); + makeBuilder().setServiceManager(serviceManager).build(); verify(serviceManager).start(); } From 6510fbcad433728581809594454f23f19f875afe Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 15:54:57 -0700 Subject: [PATCH 02/14] create InstallationContext --- .../android/instrumentation/InstallationContext.kt | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 core/src/main/java/io/opentelemetry/android/instrumentation/InstallationContext.kt diff --git a/core/src/main/java/io/opentelemetry/android/instrumentation/InstallationContext.kt b/core/src/main/java/io/opentelemetry/android/instrumentation/InstallationContext.kt new file mode 100644 index 00000000..cd367039 --- /dev/null +++ b/core/src/main/java/io/opentelemetry/android/instrumentation/InstallationContext.kt @@ -0,0 +1,11 @@ +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 +) \ No newline at end of file From 4028251c6afd2adf537e4a362de4c4f2f8e111f4 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 16:25:03 -0700 Subject: [PATCH 03/14] update AndroidInstrumentation.install() to take the InstallationContext. --- .../android/OpenTelemetryRumBuilder.java | 13 ++--- .../android/SdkPreconfiguredRumBuilder.kt | 4 +- .../instrumentation/AndroidInstrumentation.kt | 10 +--- .../instrumentation/InstallationContext.kt | 9 +++- .../android/OpenTelemetryRumBuilderTest.java | 54 +++++++++---------- .../TestAndroidInstrumentation.kt | 8 +-- .../AndroidInstrumentationLoaderImplTest.kt | 2 +- .../ActivityLifecycleInstrumentation.kt | 13 ++--- .../activity/ActivityInstrumentationTest.kt | 4 +- .../anr/AnrInstrumentation.java | 10 ++-- .../crash/CrashReporterInstrumentation.java | 9 ++-- .../crash/CrashReporterTest.java | 10 +++- .../FragmentLifecycleInstrumentation.kt | 9 ++-- .../HttpUrlInstrumentation.java | 7 ++- .../network/NetworkChangeInstrumentation.java | 7 ++- .../okhttp/v3_0/OkHttpInstrumentation.java | 7 ++- .../SlowRenderingInstrumentation.java | 9 ++-- .../SlowRenderingInstrumentationTest.kt | 8 +-- .../startup/StartupInstrumentation.kt | 10 ++-- .../startup/StartupInstrumentationTest.kt | 17 ++++-- 20 files changed, 107 insertions(+), 113 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java index bdc6af7d..674fe5f6 100644 --- a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java +++ b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java @@ -9,9 +9,7 @@ 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; @@ -89,8 +87,7 @@ public final class OpenTelemetryRumBuilder { private Resource resource; - @Nullable - private ServiceManager serviceManager; + @Nullable private ServiceManager serviceManager; private static TextMapPropagator buildDefaultPropagator() { return TextMapPropagator.composite( @@ -281,8 +278,7 @@ public OpenTelemetryRum build() { SignalFromDiskExporter signalFromDiskExporter = null; if (diskBufferingConfiguration.isEnabled()) { try { - StorageConfiguration storageConfiguration = - createStorageConfiguration(); + StorageConfiguration storageConfiguration = createStorageConfiguration(); final SpanExporter originalSpanExporter = spanExporter; spanExporter = SpanToDiskExporter.create(originalSpanExporter, storageConfiguration); @@ -332,7 +328,7 @@ public OpenTelemetryRum build() { @NonNull private ServiceManager getServiceManager() { - if(serviceManager == null){ + if (serviceManager == null) { ServiceManager.initialize(application); serviceManager = ServiceManager.get(); } @@ -344,8 +340,7 @@ public OpenTelemetryRumBuilder setServiceManager(ServiceManager serviceManager) return this; } - private StorageConfiguration createStorageConfiguration() - throws IOException { + private StorageConfiguration createStorageConfiguration() throws IOException { Preferences preferences = getServiceManager().getPreferences(); CacheStorage storage = getServiceManager().getCacheStorage(); DiskBufferingConfiguration config = this.config.getDiskBufferingConfiguration(); diff --git a/core/src/main/java/io/opentelemetry/android/SdkPreconfiguredRumBuilder.kt b/core/src/main/java/io/opentelemetry/android/SdkPreconfiguredRumBuilder.kt index a6ae00ba..7bf92695 100644 --- a/core/src/main/java/io/opentelemetry/android/SdkPreconfiguredRumBuilder.kt +++ b/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 diff --git a/core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentation.kt b/core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentation.kt index a6b01806..c9ac7a47 100644 --- a/core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentation.kt +++ b/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) } diff --git a/core/src/main/java/io/opentelemetry/android/instrumentation/InstallationContext.kt b/core/src/main/java/io/opentelemetry/android/instrumentation/InstallationContext.kt index cd367039..826a24a4 100644 --- a/core/src/main/java/io/opentelemetry/android/instrumentation/InstallationContext.kt +++ b/core/src/main/java/io/opentelemetry/android/instrumentation/InstallationContext.kt @@ -1,3 +1,8 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.android.instrumentation import android.app.Application @@ -7,5 +12,5 @@ import io.opentelemetry.api.OpenTelemetry data class InstallationContext( val application: Application, val openTelemetry: OpenTelemetry, - val serviceManager: ServiceManager -) \ No newline at end of file + val serviceManager: ServiceManager, +) diff --git a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java index 265301d6..ed9868ec 100644 --- a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java +++ b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java @@ -15,7 +15,6 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isA; -import static org.mockito.ArgumentMatchers.notNull; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -34,15 +33,14 @@ import io.opentelemetry.android.features.diskbuffering.scheduler.ExportScheduleHandler; import io.opentelemetry.android.instrumentation.AndroidInstrumentation; import io.opentelemetry.android.instrumentation.AndroidInstrumentationLoader; +import io.opentelemetry.android.instrumentation.InstallationContext; 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; -import io.opentelemetry.android.internal.services.ServiceManagerImpl; import io.opentelemetry.android.internal.services.applifecycle.AppLifecycleService; import io.opentelemetry.android.internal.services.applifecycle.ApplicationStateListener; -import io.opentelemetry.android.internal.services.visiblescreen.VisibleScreenService; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.KeyValue; import io.opentelemetry.api.common.Value; @@ -72,7 +70,6 @@ import io.opentelemetry.semconv.incubating.SessionIncubatingAttributes; import java.io.IOException; import java.time.Duration; -import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; @@ -124,9 +121,7 @@ public void shouldRegisterApplicationStateWatcher() { ServiceManager serviceManager = createServiceManager(); AppLifecycleService appLifecycleService = serviceManager.getAppLifecycleService(); - makeBuilder() - .setServiceManager(serviceManager) - .build(); + makeBuilder().setServiceManager(serviceManager).build(); verify(appLifecycleService).registerListener(isA(ApplicationStateListener.class)); } @@ -203,15 +198,18 @@ public void shouldInstallInstrumentation() { (AndroidInstrumentationLoaderImpl) AndroidInstrumentationLoader.get(); androidInstrumentationServices.registerForTest(classpathInstrumentation); - new OpenTelemetryRumBuilder(application, buildConfig(), timeoutHandler) - .addInstrumentation(localInstrumentation) - .setServiceManager(serviceManager) - .build(); + OpenTelemetryRum rum = + new OpenTelemetryRumBuilder(application, buildConfig(), timeoutHandler) + .addInstrumentation(localInstrumentation) + .setServiceManager(serviceManager) + .build(); verify(serviceManager.getAppLifecycleService()).registerListener(timeoutHandler); - verify(localInstrumentation).install(eq(application), notNull()); - verify(classpathInstrumentation).install(eq(application), notNull()); + InstallationContext expectedCtx = + new InstallationContext(application, rum.getOpenTelemetry(), serviceManager); + verify(localInstrumentation).install(eq(expectedCtx)); + verify(classpathInstrumentation).install(eq(expectedCtx)); } @Test @@ -224,17 +222,20 @@ public void shouldInstallInstrumentation_excludingClasspathImplsWhenRequestedInC (AndroidInstrumentationLoaderImpl) AndroidInstrumentationLoader.get(); androidInstrumentationServices.registerForTest(classpathInstrumentation); - new OpenTelemetryRumBuilder( - application, - buildConfig().disableInstrumentationDiscovery(), - timeoutHandler) - .addInstrumentation(localInstrumentation) - .setServiceManager(serviceManager) - .build(); + OpenTelemetryRum rum = + new OpenTelemetryRumBuilder( + application, + buildConfig().disableInstrumentationDiscovery(), + timeoutHandler) + .addInstrumentation(localInstrumentation) + .setServiceManager(serviceManager) + .build(); verify(serviceManager.getAppLifecycleService()).registerListener(timeoutHandler); - verify(localInstrumentation).install(eq(application), notNull()); + InstallationContext expectedCtx = + new InstallationContext(application, rum.getOpenTelemetry(), serviceManager); + verify(localInstrumentation).install(eq(expectedCtx)); verifyNoInteractions(classpathInstrumentation); } @@ -465,12 +466,11 @@ public void verifyPreconfiguredServicesInitialization() { * @noinspection KotlinInternalInJava */ private static ServiceManager createServiceManager() { - return new ServiceManagerImpl( - Arrays.asList( - mock(Preferences.class), - mock(CacheStorage.class), - mock(AppLifecycleService.class), - mock(VisibleScreenService.class))); + ServiceManager serviceManager = mock(ServiceManager.class); + when(serviceManager.getAppLifecycleService()).thenReturn(mock(AppLifecycleService.class)); + when(serviceManager.getCacheStorage()).thenReturn(mock(CacheStorage.class)); + when(serviceManager.getPreferences()).thenReturn(mock(Preferences.class)); + return serviceManager; } @NonNull diff --git a/core/src/test/java/io/opentelemetry/android/instrumentation/TestAndroidInstrumentation.kt b/core/src/test/java/io/opentelemetry/android/instrumentation/TestAndroidInstrumentation.kt index 60a11ff6..deec8cab 100644 --- a/core/src/test/java/io/opentelemetry/android/instrumentation/TestAndroidInstrumentation.kt +++ b/core/src/test/java/io/opentelemetry/android/instrumentation/TestAndroidInstrumentation.kt @@ -5,17 +5,11 @@ package io.opentelemetry.android.instrumentation -import android.app.Application -import io.opentelemetry.api.OpenTelemetry - class TestAndroidInstrumentation : AndroidInstrumentation { var installed = false private set - override fun install( - application: Application, - openTelemetry: OpenTelemetry, - ) { + override fun install(ctx: InstallationContext) { installed = true } } diff --git a/core/src/test/java/io/opentelemetry/android/internal/instrumentation/AndroidInstrumentationLoaderImplTest.kt b/core/src/test/java/io/opentelemetry/android/internal/instrumentation/AndroidInstrumentationLoaderImplTest.kt index a8e1a1b6..a4374859 100644 --- a/core/src/test/java/io/opentelemetry/android/internal/instrumentation/AndroidInstrumentationLoaderImplTest.kt +++ b/core/src/test/java/io/opentelemetry/android/internal/instrumentation/AndroidInstrumentationLoaderImplTest.kt @@ -25,7 +25,7 @@ class AndroidInstrumentationLoaderImplTest { assertThat(instrumentation.installed).isFalse() - instrumentation.install(mockk(), mockk()) + instrumentation.install(mockk()) assertThat(loader.getByType(TestAndroidInstrumentation::class.java)!!.installed).isTrue() } diff --git a/instrumentation/activity/src/main/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentation.kt b/instrumentation/activity/src/main/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentation.kt index b7d3e334..3791af4d 100644 --- a/instrumentation/activity/src/main/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentation.kt +++ b/instrumentation/activity/src/main/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentation.kt @@ -5,10 +5,10 @@ package io.opentelemetry.android.instrumentation.activity -import android.app.Application import android.os.Build import com.google.auto.service.AutoService import io.opentelemetry.android.instrumentation.AndroidInstrumentation +import io.opentelemetry.android.instrumentation.InstallationContext import io.opentelemetry.android.instrumentation.activity.startup.AppStartupTimer import io.opentelemetry.android.instrumentation.common.Constants.INSTRUMENTATION_SCOPE import io.opentelemetry.android.instrumentation.common.ScreenNameExtractor @@ -31,13 +31,10 @@ class ActivityLifecycleInstrumentation : AndroidInstrumentation { this.screenNameExtractor = screenNameExtractor } - override fun install( - application: Application, - openTelemetry: OpenTelemetry, - ) { - startupTimer.start(openTelemetry.getTracer(INSTRUMENTATION_SCOPE)) - application.registerActivityLifecycleCallbacks(startupTimer.createLifecycleCallback()) - application.registerActivityLifecycleCallbacks(buildActivityLifecycleTracer(openTelemetry)) + override fun install(ctx: InstallationContext) { + startupTimer.start(ctx.openTelemetry.getTracer(INSTRUMENTATION_SCOPE)) + ctx.application.registerActivityLifecycleCallbacks(startupTimer.createLifecycleCallback()) + ctx.application.registerActivityLifecycleCallbacks(buildActivityLifecycleTracer(ctx.openTelemetry)) } private fun buildActivityLifecycleTracer(openTelemetry: OpenTelemetry): DefaultingActivityLifecycleCallbacks { diff --git a/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt b/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt index af5581e3..b2b0ae00 100644 --- a/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt +++ b/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt @@ -12,6 +12,7 @@ import io.mockk.mockk import io.mockk.verify import io.opentelemetry.android.OpenTelemetryRum import io.opentelemetry.android.common.RumConstants +import io.opentelemetry.android.instrumentation.InstallationContext import io.opentelemetry.android.internal.services.ServiceManager.Companion import io.opentelemetry.api.trace.Span import io.opentelemetry.api.trace.SpanBuilder @@ -52,7 +53,8 @@ class ActivityInstrumentationTest { ) every { startupSpanBuilder.startSpan() }.returns(startupSpan) - activityLifecycleInstrumentation.install(application, openTelemetry) + val ctx = InstallationContext(application, openTelemetry, mockk()) + activityLifecycleInstrumentation.install(ctx) verify { tracer.spanBuilder("AppStart") diff --git a/instrumentation/anr/src/main/java/io/opentelemetry/android/instrumentation/anr/AnrInstrumentation.java b/instrumentation/anr/src/main/java/io/opentelemetry/android/instrumentation/anr/AnrInstrumentation.java index 81140be9..303df35d 100644 --- a/instrumentation/anr/src/main/java/io/opentelemetry/android/instrumentation/anr/AnrInstrumentation.java +++ b/instrumentation/anr/src/main/java/io/opentelemetry/android/instrumentation/anr/AnrInstrumentation.java @@ -5,13 +5,11 @@ package io.opentelemetry.android.instrumentation.anr; -import android.app.Application; import android.os.Looper; import androidx.annotation.NonNull; import com.google.auto.service.AutoService; import io.opentelemetry.android.instrumentation.AndroidInstrumentation; -import io.opentelemetry.android.internal.services.ServiceManager; -import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.android.instrumentation.InstallationContext; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import java.util.ArrayList; import java.util.List; @@ -47,14 +45,14 @@ AnrInstrumentation setScheduler(ScheduledExecutorService scheduler) { } @Override - public void install(@NonNull Application application, @NonNull OpenTelemetry openTelemetry) { + public void install(@NonNull InstallationContext ctx) { AnrDetector anrDetector = new AnrDetector( additionalExtractors, mainLooper, scheduler, - ServiceManager.get().getAppLifecycleService(), - openTelemetry); + ctx.getServiceManager().getAppLifecycleService(), + ctx.getOpenTelemetry()); anrDetector.start(); } } diff --git a/instrumentation/crash/src/main/java/io/opentelemetry/android/instrumentation/crash/CrashReporterInstrumentation.java b/instrumentation/crash/src/main/java/io/opentelemetry/android/instrumentation/crash/CrashReporterInstrumentation.java index 880de041..13dd2fe9 100644 --- a/instrumentation/crash/src/main/java/io/opentelemetry/android/instrumentation/crash/CrashReporterInstrumentation.java +++ b/instrumentation/crash/src/main/java/io/opentelemetry/android/instrumentation/crash/CrashReporterInstrumentation.java @@ -5,12 +5,11 @@ package io.opentelemetry.android.instrumentation.crash; -import android.app.Application; import androidx.annotation.NonNull; import com.google.auto.service.AutoService; import io.opentelemetry.android.RuntimeDetailsExtractor; import io.opentelemetry.android.instrumentation.AndroidInstrumentation; -import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.android.instrumentation.InstallationContext; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.sdk.OpenTelemetrySdk; import java.util.ArrayList; @@ -28,9 +27,9 @@ public void addAttributesExtractor(AttributesExtractor extra } @Override - public void install(@NonNull Application application, @NonNull OpenTelemetry openTelemetry) { - addAttributesExtractor(RuntimeDetailsExtractor.create(application)); + public void install(@NonNull InstallationContext ctx) { + addAttributesExtractor(RuntimeDetailsExtractor.create(ctx.getApplication())); CrashReporter crashReporter = new CrashReporter(additionalExtractors); - crashReporter.install((OpenTelemetrySdk) openTelemetry); + crashReporter.install((OpenTelemetrySdk) ctx.getOpenTelemetry()); } } diff --git a/instrumentation/crash/src/test/java/io/opentelemetry/android/instrumentation/crash/CrashReporterTest.java b/instrumentation/crash/src/test/java/io/opentelemetry/android/instrumentation/crash/CrashReporterTest.java index fc20cde2..da6de01d 100644 --- a/instrumentation/crash/src/test/java/io/opentelemetry/android/instrumentation/crash/CrashReporterTest.java +++ b/instrumentation/crash/src/test/java/io/opentelemetry/android/instrumentation/crash/CrashReporterTest.java @@ -8,8 +8,11 @@ import static io.opentelemetry.api.common.AttributeKey.stringKey; import static io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor.constant; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; import androidx.test.ext.junit.runners.AndroidJUnit4; +import io.opentelemetry.android.instrumentation.InstallationContext; +import io.opentelemetry.android.internal.services.ServiceManager; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.logs.SdkLoggerProvider; @@ -57,7 +60,12 @@ public void tearDown() { public void integrationTest() throws InterruptedException { CrashReporterInstrumentation instrumentation = new CrashReporterInstrumentation(); instrumentation.addAttributesExtractor(constant(stringKey("test.key"), "abc")); - instrumentation.install(RuntimeEnvironment.getApplication(), openTelemetrySdk); + InstallationContext ctx = + new InstallationContext( + RuntimeEnvironment.getApplication(), + openTelemetrySdk, + mock(ServiceManager.class)); + instrumentation.install(ctx); String exceptionMessage = "boooom!"; RuntimeException crash = new RuntimeException(exceptionMessage); diff --git a/instrumentation/fragment/src/main/java/io/opentelemetry/android/instrumentation/fragment/FragmentLifecycleInstrumentation.kt b/instrumentation/fragment/src/main/java/io/opentelemetry/android/instrumentation/fragment/FragmentLifecycleInstrumentation.kt index 22a74c54..ad0439ab 100644 --- a/instrumentation/fragment/src/main/java/io/opentelemetry/android/instrumentation/fragment/FragmentLifecycleInstrumentation.kt +++ b/instrumentation/fragment/src/main/java/io/opentelemetry/android/instrumentation/fragment/FragmentLifecycleInstrumentation.kt @@ -5,11 +5,11 @@ package io.opentelemetry.android.instrumentation.fragment -import android.app.Application import android.app.Application.ActivityLifecycleCallbacks import android.os.Build import com.google.auto.service.AutoService import io.opentelemetry.android.instrumentation.AndroidInstrumentation +import io.opentelemetry.android.instrumentation.InstallationContext import io.opentelemetry.android.instrumentation.common.Constants.INSTRUMENTATION_SCOPE import io.opentelemetry.android.instrumentation.common.ScreenNameExtractor import io.opentelemetry.android.internal.services.ServiceManager @@ -30,11 +30,8 @@ class FragmentLifecycleInstrumentation : AndroidInstrumentation { this.screenNameExtractor = screenNameExtractor } - override fun install( - application: Application, - openTelemetry: OpenTelemetry, - ) { - application.registerActivityLifecycleCallbacks(buildFragmentRegisterer(openTelemetry)) + override fun install(ctx: InstallationContext) { + ctx.application.registerActivityLifecycleCallbacks(buildFragmentRegisterer(ctx.openTelemetry)) } private fun buildFragmentRegisterer(openTelemetry: OpenTelemetry): ActivityLifecycleCallbacks { diff --git a/instrumentation/httpurlconnection/library/src/main/java/io/opentelemetry/instrumentation/library/httpurlconnection/HttpUrlInstrumentation.java b/instrumentation/httpurlconnection/library/src/main/java/io/opentelemetry/instrumentation/library/httpurlconnection/HttpUrlInstrumentation.java index aebd2d0c..3a0e109c 100644 --- a/instrumentation/httpurlconnection/library/src/main/java/io/opentelemetry/instrumentation/library/httpurlconnection/HttpUrlInstrumentation.java +++ b/instrumentation/httpurlconnection/library/src/main/java/io/opentelemetry/instrumentation/library/httpurlconnection/HttpUrlInstrumentation.java @@ -5,10 +5,9 @@ package io.opentelemetry.instrumentation.library.httpurlconnection; -import android.app.Application; import com.google.auto.service.AutoService; import io.opentelemetry.android.instrumentation.AndroidInstrumentation; -import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.android.instrumentation.InstallationContext; import io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceResolver; import io.opentelemetry.instrumentation.api.internal.HttpConstants; import io.opentelemetry.instrumentation.library.httpurlconnection.internal.HttpUrlConnectionSingletons; @@ -123,8 +122,8 @@ public boolean emitExperimentalHttpClientMetrics() { } @Override - public void install(@NotNull Application application, @NotNull OpenTelemetry openTelemetry) { - HttpUrlConnectionSingletons.configure(this, openTelemetry); + public void install(@NotNull InstallationContext ctx) { + HttpUrlConnectionSingletons.configure(this, ctx.getOpenTelemetry()); } /** diff --git a/instrumentation/network/src/main/java/io/opentelemetry/android/instrumentation/network/NetworkChangeInstrumentation.java b/instrumentation/network/src/main/java/io/opentelemetry/android/instrumentation/network/NetworkChangeInstrumentation.java index 88fe82b6..bddb8e4a 100644 --- a/instrumentation/network/src/main/java/io/opentelemetry/android/instrumentation/network/NetworkChangeInstrumentation.java +++ b/instrumentation/network/src/main/java/io/opentelemetry/android/instrumentation/network/NetworkChangeInstrumentation.java @@ -5,13 +5,12 @@ package io.opentelemetry.android.instrumentation.network; -import android.app.Application; import androidx.annotation.NonNull; import com.google.auto.service.AutoService; import io.opentelemetry.android.instrumentation.AndroidInstrumentation; +import io.opentelemetry.android.instrumentation.InstallationContext; import io.opentelemetry.android.internal.services.ServiceManager; import io.opentelemetry.android.internal.services.network.data.CurrentNetwork; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import java.util.ArrayList; import java.util.Collections; @@ -31,10 +30,10 @@ public NetworkChangeInstrumentation addAttributesExtractor( } @Override - public void install(@NonNull Application application, @NonNull OpenTelemetry openTelemetry) { + public void install(@NonNull InstallationContext ctx) { NetworkChangeMonitor networkChangeMonitor = new NetworkChangeMonitor( - openTelemetry, + ctx.getOpenTelemetry(), ServiceManager.get().getAppLifecycleService(), ServiceManager.get().getCurrentNetworkProvider(), Collections.unmodifiableList(additionalExtractors)); diff --git a/instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/library/okhttp/v3_0/OkHttpInstrumentation.java b/instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/library/okhttp/v3_0/OkHttpInstrumentation.java index b2983aeb..2fa23923 100644 --- a/instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/library/okhttp/v3_0/OkHttpInstrumentation.java +++ b/instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/library/okhttp/v3_0/OkHttpInstrumentation.java @@ -5,10 +5,9 @@ package io.opentelemetry.instrumentation.library.okhttp.v3_0; -import android.app.Application; import com.google.auto.service.AutoService; import io.opentelemetry.android.instrumentation.AndroidInstrumentation; -import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.android.instrumentation.InstallationContext; import io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceResolver; import io.opentelemetry.instrumentation.api.internal.HttpConstants; import io.opentelemetry.instrumentation.library.okhttp.v3_0.internal.OkHttp3Singletons; @@ -123,7 +122,7 @@ public boolean emitExperimentalHttpClientMetrics() { } @Override - public void install(@NotNull Application application, @NotNull OpenTelemetry openTelemetry) { - OkHttp3Singletons.configure(this, openTelemetry); + public void install(@NotNull InstallationContext ctx) { + OkHttp3Singletons.configure(this, ctx.getOpenTelemetry()); } } diff --git a/instrumentation/slowrendering/src/main/java/io/opentelemetry/android/instrumentation/slowrendering/SlowRenderingInstrumentation.java b/instrumentation/slowrendering/src/main/java/io/opentelemetry/android/instrumentation/slowrendering/SlowRenderingInstrumentation.java index d140d773..aae35853 100644 --- a/instrumentation/slowrendering/src/main/java/io/opentelemetry/android/instrumentation/slowrendering/SlowRenderingInstrumentation.java +++ b/instrumentation/slowrendering/src/main/java/io/opentelemetry/android/instrumentation/slowrendering/SlowRenderingInstrumentation.java @@ -5,7 +5,6 @@ package io.opentelemetry.android.instrumentation.slowrendering; -import android.app.Application; import android.os.Build; import android.util.Log; import androidx.annotation.NonNull; @@ -13,7 +12,7 @@ import com.google.auto.service.AutoService; import io.opentelemetry.android.common.RumConstants; import io.opentelemetry.android.instrumentation.AndroidInstrumentation; -import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.android.instrumentation.InstallationContext; import java.time.Duration; /** Entrypoint for installing the slow rendering detection instrumentation. */ @@ -43,7 +42,7 @@ public SlowRenderingInstrumentation setSlowRenderingDetectionPollInterval(Durati @RequiresApi(Build.VERSION_CODES.N) @Override - public void install(@NonNull Application application, @NonNull OpenTelemetry openTelemetry) { + public void install(@NonNull InstallationContext ctx) { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { Log.w( RumConstants.OTEL_RUM_LOG_TAG, @@ -53,10 +52,10 @@ public void install(@NonNull Application application, @NonNull OpenTelemetry ope SlowRenderListener detector = new SlowRenderListener( - openTelemetry.getTracer("io.opentelemetry.slow-rendering"), + ctx.getOpenTelemetry().getTracer("io.opentelemetry.slow-rendering"), slowRenderingDetectionPollInterval); - application.registerActivityLifecycleCallbacks(detector); + ctx.getApplication().registerActivityLifecycleCallbacks(detector); detector.start(); } } diff --git a/instrumentation/slowrendering/src/test/java/io/opentelemetry/android/instrumentation/slowrendering/SlowRenderingInstrumentationTest.kt b/instrumentation/slowrendering/src/test/java/io/opentelemetry/android/instrumentation/slowrendering/SlowRenderingInstrumentationTest.kt index c4562df3..973b3880 100644 --- a/instrumentation/slowrendering/src/test/java/io/opentelemetry/android/instrumentation/slowrendering/SlowRenderingInstrumentationTest.kt +++ b/instrumentation/slowrendering/src/test/java/io/opentelemetry/android/instrumentation/slowrendering/SlowRenderingInstrumentationTest.kt @@ -14,6 +14,7 @@ import io.mockk.just import io.mockk.mockk import io.mockk.slot import io.mockk.verify +import io.opentelemetry.android.instrumentation.InstallationContext import io.opentelemetry.sdk.OpenTelemetrySdk import org.assertj.core.api.Assertions.assertThat import org.junit.Before @@ -63,7 +64,8 @@ class SlowRenderingInstrumentationTest { @Config(sdk = [23]) @Test fun `Not installing instrumentation on devices with API level lower than 24`() { - slowRenderingInstrumentation.install(application, openTelemetry) + val ctx = InstallationContext(application, openTelemetry, mockk()) + slowRenderingInstrumentation.install(ctx) verify { application wasNot Called @@ -79,8 +81,8 @@ class SlowRenderingInstrumentationTest { val capturedListener = slot() every { openTelemetry.getTracer(any()) }.returns(mockk()) every { application.registerActivityLifecycleCallbacks(any()) } just Runs - - slowRenderingInstrumentation.install(application, openTelemetry) + val ctx = InstallationContext(application, openTelemetry, mockk()) + slowRenderingInstrumentation.install(ctx) verify { openTelemetry.getTracer("io.opentelemetry.slow-rendering") } verify { application.registerActivityLifecycleCallbacks(capture(capturedListener)) } diff --git a/instrumentation/startup/src/main/java/io/opentelemetry/android/instrumentation/startup/StartupInstrumentation.kt b/instrumentation/startup/src/main/java/io/opentelemetry/android/instrumentation/startup/StartupInstrumentation.kt index 25a8763f..7ab8ccdf 100644 --- a/instrumentation/startup/src/main/java/io/opentelemetry/android/instrumentation/startup/StartupInstrumentation.kt +++ b/instrumentation/startup/src/main/java/io/opentelemetry/android/instrumentation/startup/StartupInstrumentation.kt @@ -5,21 +5,17 @@ package io.opentelemetry.android.instrumentation.startup -import android.app.Application import com.google.auto.service.AutoService import io.opentelemetry.android.instrumentation.AndroidInstrumentation +import io.opentelemetry.android.instrumentation.InstallationContext import io.opentelemetry.android.internal.initialization.InitializationEvents -import io.opentelemetry.api.OpenTelemetry @AutoService(AndroidInstrumentation::class) class StartupInstrumentation : AndroidInstrumentation { - override fun install( - application: Application, - openTelemetry: OpenTelemetry, - ) { + override fun install(ctx: InstallationContext) { val events = InitializationEvents.get() if (events is SdkInitializationEvents) { - events.finish(openTelemetry) + events.finish(ctx.openTelemetry) } } } diff --git a/instrumentation/startup/src/test/java/io/opentelemetry/android/instrumentation/startup/StartupInstrumentationTest.kt b/instrumentation/startup/src/test/java/io/opentelemetry/android/instrumentation/startup/StartupInstrumentationTest.kt index ab19df69..98f0eb6f 100644 --- a/instrumentation/startup/src/test/java/io/opentelemetry/android/instrumentation/startup/StartupInstrumentationTest.kt +++ b/instrumentation/startup/src/test/java/io/opentelemetry/android/instrumentation/startup/StartupInstrumentationTest.kt @@ -5,14 +5,16 @@ package io.opentelemetry.android.instrumentation.startup +import android.app.Application import io.mockk.Called import io.mockk.Runs import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.verify -import io.opentelemetry.android.OpenTelemetryRum +import io.opentelemetry.android.instrumentation.InstallationContext import io.opentelemetry.android.internal.initialization.InitializationEvents +import io.opentelemetry.android.internal.services.ServiceManager import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach @@ -41,7 +43,7 @@ class StartupInstrumentationTest { every { sdkInitializationEvents.finish(any()) } just Runs InitializationEvents.set(sdkInitializationEvents) - instrumentation.install(mockk(), otelTesting.openTelemetry) + instrumentation.install(makeContext()) verify { sdkInitializationEvents.finish(otelTesting.openTelemetry) @@ -51,11 +53,18 @@ class StartupInstrumentationTest { @Test fun `No action when the InitializationEvents instance is not SdkInitializationEvents`() { val initializationEvents = mockk() - val openTelemetryRum = mockk() InitializationEvents.set(initializationEvents) - instrumentation.install(mockk(), otelTesting.openTelemetry) + instrumentation.install(makeContext()) verify { initializationEvents wasNot Called } } + + private fun makeContext(): InstallationContext { + return InstallationContext( + mockk(), + otelTesting.openTelemetry, + mockk(), + ) + } } From 53e2b1022239f492904491915d4ee35eb4ce6918 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 18:55:32 -0700 Subject: [PATCH 04/14] decouple instrumentations from ServiceManager singleton --- .../io/opentelemetry/android/OpenTelemetryRum.java | 6 ++++-- .../android/OpenTelemetryRumBuilderTest.java | 3 ++- .../activity/ActivityLifecycleInstrumentation.kt | 10 ++++------ .../fragment/FragmentLifecycleInstrumentation.kt | 10 ++++------ .../network/NetworkChangeInstrumentation.java | 5 ++--- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRum.java b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRum.java index 2810f8de..f8662184 100644 --- a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRum.java +++ b/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) { + boolean discoverInstrumentations, + ServiceManager serviceManager) { ServiceManager.initialize(application); return new SdkPreconfiguredRumBuilder( - application, openTelemetrySdk, discoverInstrumentations, ServiceManager.get()); + application, openTelemetrySdk, discoverInstrumentations, serviceManager); } /** Returns a no-op implementation of {@link OpenTelemetryRum}. */ diff --git a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java index ed9868ec..83e4f90b 100644 --- a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java +++ b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java @@ -457,7 +457,8 @@ public void verifyPreconfiguredServicesInitialization() { when(openTelemetrySdk.getLogsBridge()).thenReturn(logsBridge); when(logsBridge.loggerBuilder(any())).thenReturn(loggerBuilder); - OpenTelemetryRum.builder(application, openTelemetrySdk, true).build(); + OpenTelemetryRum.builder(application, openTelemetrySdk, true, createServiceManager()) + .build(); assertThat(ServiceManager.get()).isNotNull(); } diff --git a/instrumentation/activity/src/main/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentation.kt b/instrumentation/activity/src/main/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentation.kt index 3791af4d..3b506116 100644 --- a/instrumentation/activity/src/main/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentation.kt +++ b/instrumentation/activity/src/main/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentation.kt @@ -12,9 +12,7 @@ import io.opentelemetry.android.instrumentation.InstallationContext import io.opentelemetry.android.instrumentation.activity.startup.AppStartupTimer import io.opentelemetry.android.instrumentation.common.Constants.INSTRUMENTATION_SCOPE import io.opentelemetry.android.instrumentation.common.ScreenNameExtractor -import io.opentelemetry.android.internal.services.ServiceManager import io.opentelemetry.android.internal.services.visiblescreen.activities.DefaultingActivityLifecycleCallbacks -import io.opentelemetry.api.OpenTelemetry import io.opentelemetry.api.trace.Tracer @AutoService(AndroidInstrumentation::class) @@ -34,12 +32,12 @@ class ActivityLifecycleInstrumentation : AndroidInstrumentation { override fun install(ctx: InstallationContext) { startupTimer.start(ctx.openTelemetry.getTracer(INSTRUMENTATION_SCOPE)) ctx.application.registerActivityLifecycleCallbacks(startupTimer.createLifecycleCallback()) - ctx.application.registerActivityLifecycleCallbacks(buildActivityLifecycleTracer(ctx.openTelemetry)) + ctx.application.registerActivityLifecycleCallbacks(buildActivityLifecycleTracer(ctx)) } - private fun buildActivityLifecycleTracer(openTelemetry: OpenTelemetry): DefaultingActivityLifecycleCallbacks { - val visibleScreenService = ServiceManager.get().getVisibleScreenService() - val delegateTracer: Tracer = openTelemetry.getTracer(INSTRUMENTATION_SCOPE) + private fun buildActivityLifecycleTracer(ctx: InstallationContext): DefaultingActivityLifecycleCallbacks { + val visibleScreenService = ctx.serviceManager.getVisibleScreenService() + val delegateTracer: Tracer = ctx.openTelemetry.getTracer(INSTRUMENTATION_SCOPE) val tracers = ActivityTracerCache( tracerCustomizer.invoke(delegateTracer), diff --git a/instrumentation/fragment/src/main/java/io/opentelemetry/android/instrumentation/fragment/FragmentLifecycleInstrumentation.kt b/instrumentation/fragment/src/main/java/io/opentelemetry/android/instrumentation/fragment/FragmentLifecycleInstrumentation.kt index ad0439ab..cf4a618f 100644 --- a/instrumentation/fragment/src/main/java/io/opentelemetry/android/instrumentation/fragment/FragmentLifecycleInstrumentation.kt +++ b/instrumentation/fragment/src/main/java/io/opentelemetry/android/instrumentation/fragment/FragmentLifecycleInstrumentation.kt @@ -12,9 +12,7 @@ import io.opentelemetry.android.instrumentation.AndroidInstrumentation import io.opentelemetry.android.instrumentation.InstallationContext import io.opentelemetry.android.instrumentation.common.Constants.INSTRUMENTATION_SCOPE import io.opentelemetry.android.instrumentation.common.ScreenNameExtractor -import io.opentelemetry.android.internal.services.ServiceManager import io.opentelemetry.android.internal.services.visiblescreen.fragments.RumFragmentActivityRegisterer -import io.opentelemetry.api.OpenTelemetry import io.opentelemetry.api.trace.Tracer @AutoService(AndroidInstrumentation::class) @@ -31,12 +29,12 @@ class FragmentLifecycleInstrumentation : AndroidInstrumentation { } override fun install(ctx: InstallationContext) { - ctx.application.registerActivityLifecycleCallbacks(buildFragmentRegisterer(ctx.openTelemetry)) + ctx.application.registerActivityLifecycleCallbacks(buildFragmentRegisterer(ctx)) } - private fun buildFragmentRegisterer(openTelemetry: OpenTelemetry): ActivityLifecycleCallbacks { - val visibleScreenService = ServiceManager.get().getVisibleScreenService() - val delegateTracer: Tracer = openTelemetry.getTracer(INSTRUMENTATION_SCOPE) + private fun buildFragmentRegisterer(ctx: InstallationContext): ActivityLifecycleCallbacks { + val visibleScreenService = ctx.serviceManager.getVisibleScreenService() + val delegateTracer: Tracer = ctx.openTelemetry.getTracer(INSTRUMENTATION_SCOPE) val fragmentLifecycle = RumFragmentLifecycleCallbacks( tracerCustomizer.invoke(delegateTracer), diff --git a/instrumentation/network/src/main/java/io/opentelemetry/android/instrumentation/network/NetworkChangeInstrumentation.java b/instrumentation/network/src/main/java/io/opentelemetry/android/instrumentation/network/NetworkChangeInstrumentation.java index bddb8e4a..d0245db1 100644 --- a/instrumentation/network/src/main/java/io/opentelemetry/android/instrumentation/network/NetworkChangeInstrumentation.java +++ b/instrumentation/network/src/main/java/io/opentelemetry/android/instrumentation/network/NetworkChangeInstrumentation.java @@ -9,7 +9,6 @@ import com.google.auto.service.AutoService; import io.opentelemetry.android.instrumentation.AndroidInstrumentation; import io.opentelemetry.android.instrumentation.InstallationContext; -import io.opentelemetry.android.internal.services.ServiceManager; import io.opentelemetry.android.internal.services.network.data.CurrentNetwork; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import java.util.ArrayList; @@ -34,8 +33,8 @@ public void install(@NonNull InstallationContext ctx) { NetworkChangeMonitor networkChangeMonitor = new NetworkChangeMonitor( ctx.getOpenTelemetry(), - ServiceManager.get().getAppLifecycleService(), - ServiceManager.get().getCurrentNetworkProvider(), + ctx.getServiceManager().getAppLifecycleService(), + ctx.getServiceManager().getCurrentNetworkProvider(), Collections.unmodifiableList(additionalExtractors)); networkChangeMonitor.start(); } From e4865e6cfc7a3b733c2548ef97f077e8246b9619 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 19:07:28 -0700 Subject: [PATCH 05/14] pull static usage up --- .../diskbuffering/scheduler/DefaultExportScheduleHandler.kt | 5 +++-- .../diskbuffering/scheduler/DefaultExportScheduler.kt | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt index 4f4b0bb4..724dd23c 100644 --- a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt +++ b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt @@ -26,10 +26,11 @@ class DefaultExportScheduleHandler( companion object { @JvmStatic fun create(): DefaultExportScheduleHandler { + val serviceManager = ServiceManager.get() return DefaultExportScheduleHandler( - DefaultExportScheduler.create(), + DefaultExportScheduler.create(serviceManager), ) { - ServiceManager.get().getPeriodicWorkService() + serviceManager.getPeriodicWorkService() } } } diff --git a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt index f4beb661..299ae0e9 100644 --- a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt +++ b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt @@ -19,9 +19,9 @@ class DefaultExportScheduler(periodicWorkServiceProvider: () -> PeriodicWorkServ companion object { private val DELAY_BEFORE_NEXT_EXPORT_IN_MILLIS = TimeUnit.SECONDS.toMillis(10) - fun create(): DefaultExportScheduler { + fun create(serviceManager: ServiceManager): DefaultExportScheduler { return DefaultExportScheduler { - ServiceManager.get().getPeriodicWorkService() + serviceManager.getPeriodicWorkService() } } } From 6c1f30712ef1276393807f6f097d967a5ae76e04 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 19:10:37 -0700 Subject: [PATCH 06/14] remove static create method --- .../diskbuffering/scheduler/DefaultExportScheduleHandler.kt | 4 +++- .../diskbuffering/scheduler/DefaultExportScheduler.kt | 6 ------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt index 724dd23c..b22df3aa 100644 --- a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt +++ b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt @@ -28,7 +28,9 @@ class DefaultExportScheduleHandler( fun create(): DefaultExportScheduleHandler { val serviceManager = ServiceManager.get() return DefaultExportScheduleHandler( - DefaultExportScheduler.create(serviceManager), + DefaultExportScheduler { + serviceManager.getPeriodicWorkService() + } ) { serviceManager.getPeriodicWorkService() } diff --git a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt index 299ae0e9..7bbfb28b 100644 --- a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt +++ b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt @@ -18,12 +18,6 @@ class DefaultExportScheduler(periodicWorkServiceProvider: () -> PeriodicWorkServ PeriodicRunnable(periodicWorkServiceProvider) { companion object { private val DELAY_BEFORE_NEXT_EXPORT_IN_MILLIS = TimeUnit.SECONDS.toMillis(10) - - fun create(serviceManager: ServiceManager): DefaultExportScheduler { - return DefaultExportScheduler { - serviceManager.getPeriodicWorkService() - } - } } override fun onRun() { From 530d16f5563c57cd856cee2bd91ae7a9ed670d4f Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 19:42:02 -0700 Subject: [PATCH 07/14] remove static create() in favor of constructor --- .../diskbuffering/DiskBufferingConfiguration.java | 10 +++++++++- .../scheduler/DefaultExportScheduleHandler.kt | 15 --------------- .../scheduler/DefaultExportScheduler.kt | 1 - 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java index ab4d5a66..2c3cf6da 100644 --- a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java +++ b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java @@ -6,10 +6,14 @@ package io.opentelemetry.android.features.diskbuffering; 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.internal.services.ServiceManager; +import io.opentelemetry.android.internal.services.periodicwork.PeriodicWorkService; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; +import kotlin.jvm.functions.Function0; /** Configuration for disk buffering. */ public final class DiskBufferingConfiguration { @@ -71,7 +75,11 @@ public static final class Builder { private long minFileAgeForReadMillis = TimeUnit.SECONDS.toMillis(33); private long maxFileAgeForReadMillis = TimeUnit.HOURS.toMillis(18); - private ExportScheduleHandler exportScheduleHandler = DefaultExportScheduleHandler.create(); + private final Function0 getWorkService = + () -> ServiceManager.get().getPeriodicWorkService(); + private ExportScheduleHandler exportScheduleHandler = + new DefaultExportScheduleHandler( + new DefaultExportScheduler(getWorkService), getWorkService); /** Enables or disables disk buffering. */ public Builder setEnabled(boolean enabled) { diff --git a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt index b22df3aa..e2527778 100644 --- a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduleHandler.kt +++ b/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,18 +21,4 @@ class DefaultExportScheduleHandler( periodicWorkService.enqueue(exportScheduler) } } - - companion object { - @JvmStatic - fun create(): DefaultExportScheduleHandler { - val serviceManager = ServiceManager.get() - return DefaultExportScheduleHandler( - DefaultExportScheduler { - serviceManager.getPeriodicWorkService() - } - ) { - serviceManager.getPeriodicWorkService() - } - } - } } diff --git a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt index 7bbfb28b..a508fdfe 100644 --- a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/scheduler/DefaultExportScheduler.kt +++ b/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 From 927af2371f1fb5a43d7362abfc8b2a4559554ac7 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 19:58:22 -0700 Subject: [PATCH 08/14] pull operational object out of disk buffer config and promote to OTRB setter with same sane defaults. --- .../android/OpenTelemetryRumBuilder.java | 34 +++++++++++++++---- .../DiskBufferingConfiguration.java | 27 --------------- .../android/OpenTelemetryRumBuilderTest.java | 29 ++++++++-------- 3 files changed, 42 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java index 674fe5f6..92afede1 100644 --- a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java +++ b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java @@ -14,6 +14,8 @@ 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; @@ -24,6 +26,7 @@ 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.periodicwork.PeriodicWorkService; import io.opentelemetry.android.session.SessionManager; import io.opentelemetry.android.session.SessionProvider; import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator; @@ -58,6 +61,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 @@ -88,6 +92,7 @@ public final class OpenTelemetryRumBuilder { private Resource resource; @Nullable private ServiceManager serviceManager; + @Nullable private ExportScheduleHandler exportScheduleHandler; private static TextMapPropagator buildDefaultPropagator() { return TextMapPropagator.composite( @@ -312,7 +317,7 @@ public OpenTelemetryRum build() { otelSdkReadyListeners.forEach(listener -> listener.accept(sdk)); - scheduleDiskTelemetryReader(signalFromDiskExporter, diskBufferingConfiguration); + scheduleDiskTelemetryReader(signalFromDiskExporter); SdkPreconfiguredRumBuilder delegate = new SdkPreconfiguredRumBuilder( @@ -340,6 +345,16 @@ public OpenTelemetryRumBuilder setServiceManager(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(); @@ -357,11 +372,18 @@ private StorageConfiguration createStorageConfiguration() throws IOException { .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 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). diff --git a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java index 2c3cf6da..258037b4 100644 --- a/core/src/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java +++ b/core/src/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java @@ -5,15 +5,9 @@ package io.opentelemetry.android.features.diskbuffering; -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.internal.services.ServiceManager; -import io.opentelemetry.android.internal.services.periodicwork.PeriodicWorkService; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; -import kotlin.jvm.functions.Function0; /** Configuration for disk buffering. */ public final class DiskBufferingConfiguration { @@ -22,7 +16,6 @@ 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; @@ -30,7 +23,6 @@ public final class DiskBufferingConfiguration { private DiskBufferingConfiguration(Builder builder) { enabled = builder.enabled; maxCacheSize = builder.maxCacheSize; - exportScheduleHandler = builder.exportScheduleHandler; maxFileAgeForWriteMillis = builder.maxFileAgeForWriteMillis; minFileAgeForReadMillis = builder.minFileAgeForReadMillis; maxFileAgeForReadMillis = builder.maxFileAgeForReadMillis; @@ -52,10 +44,6 @@ public int getMaxCacheFileSize() { return MAX_FILE_SIZE; } - public ExportScheduleHandler getExportScheduleHandler() { - return exportScheduleHandler; - } - public long getMaxFileAgeForWriteMillis() { return maxFileAgeForWriteMillis; } @@ -75,12 +63,6 @@ public static final class Builder { private long minFileAgeForReadMillis = TimeUnit.SECONDS.toMillis(33); private long maxFileAgeForReadMillis = TimeUnit.HOURS.toMillis(18); - private final Function0 getWorkService = - () -> ServiceManager.get().getPeriodicWorkService(); - private ExportScheduleHandler exportScheduleHandler = - new DefaultExportScheduleHandler( - new DefaultExportScheduler(getWorkService), getWorkService); - /** Enables or disables disk buffering. */ public Builder setEnabled(boolean enabled) { this.enabled = enabled; @@ -121,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) { diff --git a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java index 83e4f90b..69de2b3c 100644 --- a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java +++ b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java @@ -327,13 +327,13 @@ public void diskBufferingEnabled() { OtelRumConfig config = buildConfig(); ExportScheduleHandler scheduleHandler = mock(); config.setDiskBufferingConfiguration( - DiskBufferingConfiguration.builder() - .setEnabled(true) - .setExportScheduleHandler(scheduleHandler) - .build()); + DiskBufferingConfiguration.builder().setEnabled(true).build()); ArgumentCaptor exporterCaptor = ArgumentCaptor.forClass(SpanExporter.class); - OpenTelemetryRum.builder(application, config).setServiceManager(serviceManager).build(); + OpenTelemetryRum.builder(application, config) + .setExportScheduleHandler(scheduleHandler) + .setServiceManager(serviceManager) + .build(); assertThat(SignalFromDiskExporter.get()).isNotNull(); verify(scheduleHandler).enable(); @@ -357,12 +357,12 @@ public void diskBufferingEnabled_when_exception_thrown() { ArgumentCaptor exporterCaptor = ArgumentCaptor.forClass(SpanExporter.class); OtelRumConfig config = buildConfig(); config.setDiskBufferingConfiguration( - DiskBufferingConfiguration.builder() - .setEnabled(true) - .setExportScheduleHandler(scheduleHandler) - .build()); + DiskBufferingConfiguration.builder().setEnabled(true).build()); - OpenTelemetryRum.builder(application, config).setServiceManager(serviceManager).build(); + OpenTelemetryRum.builder(application, config) + .setServiceManager(serviceManager) + .setExportScheduleHandler(scheduleHandler) + .build(); verify(initializationEvents).spanExporterInitialized(exporterCaptor.capture()); verify(scheduleHandler, never()).enable(); @@ -389,12 +389,11 @@ public void diskBufferingDisabled() { OtelRumConfig config = buildConfig(); config.setDiskBufferingConfiguration( - DiskBufferingConfiguration.builder() - .setEnabled(false) - .setExportScheduleHandler(scheduleHandler) - .build()); + DiskBufferingConfiguration.builder().setEnabled(false).build()); - OpenTelemetryRum.builder(application, config).build(); + OpenTelemetryRum.builder(application, config) + .setExportScheduleHandler(scheduleHandler) + .build(); verify(initializationEvents).spanExporterInitialized(exporterCaptor.capture()); verify(scheduleHandler, never()).enable(); From 557113101530cca2e9869c216bae548ef8fe1b17 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 20:05:23 -0700 Subject: [PATCH 09/14] move opinionated default list of services out of ServiceManager interface and into impl --- .../android/OpenTelemetryRumBuilder.java | 6 +++--- .../internal/services/ServiceManager.kt | 14 +------------- .../internal/services/ServiceManagerImpl.kt | 19 +++++++++++++++++++ 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java index 92afede1..7e9796b8 100644 --- a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java +++ b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java @@ -346,8 +346,8 @@ public OpenTelemetryRumBuilder setServiceManager(ServiceManager serviceManager) } /** - * 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. + * 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) { @@ -374,7 +374,7 @@ private StorageConfiguration createStorageConfiguration() throws IOException { private void scheduleDiskTelemetryReader(@Nullable SignalFromDiskExporter signalExporter) { - if(exportScheduleHandler == null){ + 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.... diff --git a/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManager.kt b/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManager.kt index 647cd450..005905c0 100644 --- a/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManager.kt +++ b/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManager.kt @@ -35,19 +35,7 @@ interface ServiceManager : Startable { if (instance != null) { return } - instance = - ServiceManagerImpl( - listOf( - Preferences.create(application), - CacheStorage( - application, - ), - PeriodicWorkService(), - CurrentNetworkProvider.create(application), - AppLifecycleService.create(), - VisibleScreenService.create(application), - ), - ) + instance = ServiceManagerImpl.create(application) } @JvmStatic diff --git a/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManagerImpl.kt b/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManagerImpl.kt index c6ffeaf3..63285956 100644 --- a/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManagerImpl.kt +++ b/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManagerImpl.kt @@ -5,6 +5,7 @@ package io.opentelemetry.android.internal.services +import android.app.Application import io.opentelemetry.android.internal.services.applifecycle.AppLifecycleService import io.opentelemetry.android.internal.services.network.CurrentNetworkProvider import io.opentelemetry.android.internal.services.periodicwork.PeriodicWorkService @@ -22,6 +23,24 @@ internal class ServiceManagerImpl(services: List) : ServiceManager { this.services = Collections.unmodifiableMap(map) } + companion object { + @JvmStatic + fun create(application: Application): ServiceManager { + return ServiceManagerImpl( + listOf( + Preferences.create(application), + CacheStorage( + application, + ), + PeriodicWorkService(), + CurrentNetworkProvider.create(application), + AppLifecycleService.create(), + VisibleScreenService.create(application), + ), + ) + } + } + override fun getPreferences(): Preferences { return getService(Preferences::class.java) } From d906f895159625822f8fce9ed70276a590e82477 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 20:09:02 -0700 Subject: [PATCH 10/14] remove static call and just test the impl --- .../android/internal/services/ServiceManagerImplTest.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/io/opentelemetry/android/internal/services/ServiceManagerImplTest.kt b/core/src/test/java/io/opentelemetry/android/internal/services/ServiceManagerImplTest.kt index e76a7dcc..79c61347 100644 --- a/core/src/test/java/io/opentelemetry/android/internal/services/ServiceManagerImplTest.kt +++ b/core/src/test/java/io/opentelemetry/android/internal/services/ServiceManagerImplTest.kt @@ -5,7 +5,6 @@ package io.opentelemetry.android.internal.services -import io.opentelemetry.android.internal.services.ServiceManager.Companion.initialize import io.opentelemetry.android.internal.services.network.CurrentNetworkProvider import io.opentelemetry.android.internal.services.periodicwork.PeriodicWorkService import org.assertj.core.api.Assertions.assertThat @@ -21,10 +20,10 @@ import org.robolectric.RuntimeEnvironment @RunWith(RobolectricTestRunner::class) class ServiceManagerImplTest { @Test - fun verifyAvailableServices() { - initialize(RuntimeEnvironment.getApplication()) + fun verifyAvailableDefaultServices() { + val app = RuntimeEnvironment.getApplication() - val serviceManager = ServiceManager.get() + val serviceManager = ServiceManagerImpl.create(app) assertThat(serviceManager.getPeriodicWorkService()).isInstanceOf(PeriodicWorkService::class.java) assertThat(serviceManager.getCacheStorage()).isInstanceOf(CacheStorage::class.java) From 2bcd1e882d36d38b33523af3078eeebc7e3a3e99 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 20:09:20 -0700 Subject: [PATCH 11/14] cleanup --- .../android/internal/services/ServiceManagerImpl.kt | 4 +--- .../instrumentation/activity/ActivityInstrumentationTest.kt | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManagerImpl.kt b/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManagerImpl.kt index 63285956..9b63727e 100644 --- a/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManagerImpl.kt +++ b/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManagerImpl.kt @@ -29,9 +29,7 @@ internal class ServiceManagerImpl(services: List) : ServiceManager { return ServiceManagerImpl( listOf( Preferences.create(application), - CacheStorage( - application, - ), + CacheStorage(application), PeriodicWorkService(), CurrentNetworkProvider.create(application), AppLifecycleService.create(), diff --git a/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt b/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt index b2b0ae00..4a051899 100644 --- a/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt +++ b/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt @@ -27,7 +27,6 @@ import org.robolectric.RuntimeEnvironment class ActivityInstrumentationTest { private lateinit var activityLifecycleInstrumentation: ActivityLifecycleInstrumentation private lateinit var application: Application - private lateinit var openTelemetryRum: OpenTelemetryRum private lateinit var openTelemetry: OpenTelemetrySdk @Before From 486840fe5dc0cd6fc315475103020e759c277625 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 20:18:38 -0700 Subject: [PATCH 12/14] eliminate test dep on ServiceManager static usage --- ...t.kt => ActivityLifecycleInstrumentationTest.kt} | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) rename instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/{ActivityInstrumentationTest.kt => ActivityLifecycleInstrumentationTest.kt} (87%) diff --git a/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt b/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentationTest.kt similarity index 87% rename from instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt rename to instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentationTest.kt index 4a051899..20e7c195 100644 --- a/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityInstrumentationTest.kt +++ b/instrumentation/activity/src/test/java/io/opentelemetry/android/instrumentation/activity/ActivityLifecycleInstrumentationTest.kt @@ -10,10 +10,10 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import io.mockk.every import io.mockk.mockk import io.mockk.verify -import io.opentelemetry.android.OpenTelemetryRum import io.opentelemetry.android.common.RumConstants import io.opentelemetry.android.instrumentation.InstallationContext -import io.opentelemetry.android.internal.services.ServiceManager.Companion +import io.opentelemetry.android.internal.services.ServiceManager +import io.opentelemetry.android.internal.services.visiblescreen.VisibleScreenService import io.opentelemetry.api.trace.Span import io.opentelemetry.api.trace.SpanBuilder import io.opentelemetry.api.trace.Tracer @@ -24,18 +24,19 @@ import org.junit.runner.RunWith import org.robolectric.RuntimeEnvironment @RunWith(AndroidJUnit4::class) -class ActivityInstrumentationTest { +class ActivityLifecycleInstrumentationTest { private lateinit var activityLifecycleInstrumentation: ActivityLifecycleInstrumentation private lateinit var application: Application private lateinit var openTelemetry: OpenTelemetrySdk + private lateinit var serviceManager: ServiceManager @Before fun setUp() { application = RuntimeEnvironment.getApplication() openTelemetry = mockk() activityLifecycleInstrumentation = ActivityLifecycleInstrumentation() - - Companion.initialize(application) + serviceManager = mockk() + every { serviceManager.getVisibleScreenService() }.returns(mockk()) } @Test @@ -52,7 +53,7 @@ class ActivityInstrumentationTest { ) every { startupSpanBuilder.startSpan() }.returns(startupSpan) - val ctx = InstallationContext(application, openTelemetry, mockk()) + val ctx = InstallationContext(application, openTelemetry, serviceManager) activityLifecycleInstrumentation.install(ctx) verify { From 71b56dc57ddd33d47102c6bc629cd230e3b83d56 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 20:47:57 -0700 Subject: [PATCH 13/14] finally remove singleton nature from ServiceManager. --- .../android/OpenTelemetryRum.java | 2 +- .../android/OpenTelemetryRumBuilder.java | 4 +- .../internal/services/ServiceManager.kt | 24 ----------- .../android/OpenTelemetryRumBuilderTest.java | 42 +++++++------------ 4 files changed, 17 insertions(+), 55 deletions(-) diff --git a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRum.java b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRum.java index f8662184..1d72b07e 100644 --- a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRum.java +++ b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRum.java @@ -68,7 +68,7 @@ static SdkPreconfiguredRumBuilder builder( OpenTelemetrySdk openTelemetrySdk, boolean discoverInstrumentations, ServiceManager serviceManager) { - ServiceManager.initialize(application); + return new SdkPreconfiguredRumBuilder( application, openTelemetrySdk, discoverInstrumentations, serviceManager); } diff --git a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java index 7e9796b8..ef686706 100644 --- a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java +++ b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java @@ -26,6 +26,7 @@ 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; @@ -334,8 +335,7 @@ public OpenTelemetryRum build() { @NonNull private ServiceManager getServiceManager() { if (serviceManager == null) { - ServiceManager.initialize(application); - serviceManager = ServiceManager.get(); + serviceManager = ServiceManagerImpl.Companion.create(application); } return serviceManager; } diff --git a/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManager.kt b/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManager.kt index 005905c0..96034be7 100644 --- a/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManager.kt +++ b/core/src/main/java/io/opentelemetry/android/internal/services/ServiceManager.kt @@ -5,7 +5,6 @@ package io.opentelemetry.android.internal.services -import android.app.Application import io.opentelemetry.android.internal.services.applifecycle.AppLifecycleService import io.opentelemetry.android.internal.services.network.CurrentNetworkProvider import io.opentelemetry.android.internal.services.periodicwork.PeriodicWorkService @@ -26,27 +25,4 @@ interface ServiceManager : Startable { fun getAppLifecycleService(): AppLifecycleService fun getVisibleScreenService(): VisibleScreenService - - companion object { - private var instance: ServiceManager? = null - - @JvmStatic - fun initialize(application: Application) { - if (instance != null) { - return - } - instance = ServiceManagerImpl.create(application) - } - - @JvmStatic - fun get(): ServiceManager { - checkNotNull(instance) { "Services haven't been initialized" } - return instance!! - } - - @JvmStatic - fun resetForTest() { - instance = null - } - } } diff --git a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java index 69de2b3c..7d2e066c 100644 --- a/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java +++ b/core/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java @@ -10,7 +10,6 @@ import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; import static org.awaitility.Awaitility.await; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyCollection; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; @@ -46,8 +45,6 @@ import io.opentelemetry.api.common.Value; import io.opentelemetry.api.incubator.events.EventLogger; import io.opentelemetry.api.logs.Logger; -import io.opentelemetry.api.logs.LoggerBuilder; -import io.opentelemetry.api.logs.LoggerProvider; import io.opentelemetry.api.logs.Severity; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; @@ -112,7 +109,6 @@ public void tearDown() throws Exception { SignalFromDiskExporter.resetForTesting(); InitializationEvents.resetForTest(); AndroidInstrumentationLoader.resetForTest(); - ServiceManager.resetForTest(); mocks.close(); } @@ -431,37 +427,27 @@ public void verifyGlobalAttrsForLogs() { } @Test - public void verifyServicesAreInitialized() { - makeBuilder().build(); - - assertThat(ServiceManager.get()).isNotNull(); + public void verifyDefaultServicesAreCreated() { + AtomicReference serviceManagerHolder = new AtomicReference<>(); + AndroidInstrumentation instrumentationTrap = + ctx -> serviceManagerHolder.set(ctx.getServiceManager()); + makeBuilder().addInstrumentation(instrumentationTrap).build(); + assertThat(serviceManagerHolder.get()).isNotNull(); + assertThat(serviceManagerHolder.get().getAppLifecycleService()).isNotNull(); + assertThat(serviceManagerHolder.get().getCacheStorage()).isNotNull(); + assertThat(serviceManagerHolder.get().getCurrentNetworkProvider()).isNotNull(); + assertThat(serviceManagerHolder.get().getPeriodicWorkService()).isNotNull(); + assertThat(serviceManagerHolder.get().getPreferences()).isNotNull(); + assertThat(serviceManagerHolder.get().getVisibleScreenService()).isNotNull(); } @Test - public void verifyServicesAreStarted() { - ServiceManager serviceManager = mock(); - doReturn(mock(AppLifecycleService.class)).when(serviceManager).getAppLifecycleService(); - + public void verifyServiceManagerIsStarted() { + ServiceManager serviceManager = createServiceManager(); makeBuilder().setServiceManager(serviceManager).build(); - verify(serviceManager).start(); } - @Test - public void verifyPreconfiguredServicesInitialization() { - OpenTelemetrySdk openTelemetrySdk = mock(); - // Work around sdk EventLogger api limitations - LoggerProvider logsBridge = mock(LoggerProvider.class); - LoggerBuilder loggerBuilder = mock(); - when(openTelemetrySdk.getLogsBridge()).thenReturn(logsBridge); - when(logsBridge.loggerBuilder(any())).thenReturn(loggerBuilder); - - OpenTelemetryRum.builder(application, openTelemetrySdk, true, createServiceManager()) - .build(); - - assertThat(ServiceManager.get()).isNotNull(); - } - /** * @noinspection KotlinInternalInJava */ From 3e388ed02143587d77ca7083d04bf4d296744e42 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Tue, 29 Oct 2024 20:56:06 -0700 Subject: [PATCH 14/14] add period --- .../android/instrumentation/AndroidInstrumentation.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentation.kt b/core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentation.kt index c9ac7a47..c3934bfe 100644 --- a/core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentation.kt +++ b/core/src/main/java/io/opentelemetry/android/instrumentation/AndroidInstrumentation.kt @@ -28,7 +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 ctx The InstallationContext under which the instrumentation is being installed + * @param ctx The InstallationContext under which the instrumentation is being installed. */ fun install(ctx: InstallationContext) }