From 2dc3386fd64e82ff5de5daf65b0ea8be02f3f86b Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Sat, 25 Nov 2023 08:40:27 +0100 Subject: [PATCH 1/9] Using disk span exporter --- .../android/OpenTelemetryRumBuilder.java | 35 ++++++++- .../features/persistence/DiskManager.java | 5 +- .../SimpleTemporaryFileProvider.java | 25 +++++++ .../android/OpenTelemetryRumBuilderTest.java | 73 +++++++++++++++++++ .../features/persistence/DiskManagerTest.java | 5 +- 5 files changed, 135 insertions(+), 8 deletions(-) create mode 100644 instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java diff --git a/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java index 0421bdc5b..3b47f2011 100644 --- a/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java +++ b/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java @@ -8,6 +8,7 @@ import static java.util.Objects.requireNonNull; import android.app.Application; +import io.opentelemetry.android.config.DiskBufferingConfiguration; import io.opentelemetry.android.config.OtelRumConfig; import io.opentelemetry.android.instrumentation.InstrumentedApplication; import io.opentelemetry.android.instrumentation.activity.VisibleScreenTracker; @@ -15,10 +16,14 @@ import io.opentelemetry.android.instrumentation.network.NetworkAttributesSpanAppender; import io.opentelemetry.android.instrumentation.startup.InitializationEvents; import io.opentelemetry.android.instrumentation.startup.SdkInitializationEvents; +import io.opentelemetry.android.internal.features.persistence.DiskManager; +import io.opentelemetry.android.internal.features.persistence.SimpleTemporaryFileProvider; import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator; import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.context.propagation.TextMapPropagator; +import io.opentelemetry.contrib.disk.buffering.SpanDiskExporter; +import io.opentelemetry.contrib.disk.buffering.internal.StorageConfiguration; import io.opentelemetry.exporter.logging.LoggingSpanExporter; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.logs.SdkLoggerProvider; @@ -31,6 +36,7 @@ import io.opentelemetry.sdk.trace.SpanProcessor; import io.opentelemetry.sdk.trace.export.BatchSpanProcessor; import io.opentelemetry.sdk.trace.export.SpanExporter; +import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.function.BiFunction; @@ -308,7 +314,34 @@ private SdkTracerProvider buildTracerProvider(SessionId sessionId, Application a private SpanExporter buildSpanExporter() { // TODO: Default to otlp...but how can we make endpoint and auth mandatory? SpanExporter defaultExporter = LoggingSpanExporter.create(); - return spanExporterCustomizer.apply(defaultExporter); + SpanExporter spanExporter; + DiskBufferingConfiguration diskBufferingConfiguration = + config.getDiskBufferingConfiguration(); + if (diskBufferingConfiguration.isEnabled()) { + try { + spanExporter = createDiskExporter(defaultExporter, diskBufferingConfiguration); + } catch (IOException e) { + spanExporter = defaultExporter; + } + } else { + spanExporter = defaultExporter; + } + return spanExporterCustomizer.apply(spanExporter); + } + + private static SpanExporter createDiskExporter( + SpanExporter defaultExporter, DiskBufferingConfiguration diskBufferingConfiguration) + throws IOException { + DiskManager diskManager = DiskManager.create(diskBufferingConfiguration); + StorageConfiguration storageConfiguration = + StorageConfiguration.builder() + .setMaxFileSize(diskManager.getMaxCacheFileSize()) + .setMaxFolderSize(diskManager.getMaxFolderSize()) + .setTemporaryFileProvider( + new SimpleTemporaryFileProvider(diskManager.getTemporaryDir())) + .build(); + return SpanDiskExporter.create( + defaultExporter, diskManager.getSignalsBufferDir(), storageConfiguration); } private SdkMeterProvider buildMeterProvider(Application application) { diff --git a/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.java b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.java index 5cc0c5020..7b87fedaf 100644 --- a/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.java +++ b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.java @@ -6,7 +6,6 @@ package io.opentelemetry.android.internal.features.persistence; import io.opentelemetry.android.config.DiskBufferingConfiguration; -import io.opentelemetry.android.config.OtelRumConfig; import io.opentelemetry.android.internal.services.CacheStorageService; import io.opentelemetry.android.internal.services.PreferencesService; import io.opentelemetry.android.internal.services.ServiceManager; @@ -25,12 +24,12 @@ public final class DiskManager { private final PreferencesService preferencesService; private final DiskBufferingConfiguration diskBufferingConfiguration; - public static DiskManager create(OtelRumConfig config) { + public static DiskManager create(DiskBufferingConfiguration config) { ServiceManager serviceManager = ServiceManager.get(); return new DiskManager( serviceManager.getService(CacheStorageService.class), serviceManager.getService(PreferencesService.class), - config.getDiskBufferingConfiguration()); + config); } private DiskManager( diff --git a/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java new file mode 100644 index 000000000..63380e25c --- /dev/null +++ b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java @@ -0,0 +1,25 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.android.internal.features.persistence; + +import io.opentelemetry.contrib.disk.buffering.internal.files.TemporaryFileProvider; +import java.io.File; + +/** + * This class is internal and not for public use. Its APIs are unstable and can change at any time. + */ +public final class SimpleTemporaryFileProvider implements TemporaryFileProvider { + private final File tempDir; + + public SimpleTemporaryFileProvider(File tempDir) { + this.tempDir = tempDir; + } + + @Override + public File createTemporaryFile(String prefix) { + return new File(tempDir, prefix + "_" + System.currentTimeMillis() + ".tmp"); + } +} diff --git a/instrumentation/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java b/instrumentation/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java index 7324b5927..a0a874a1b 100644 --- a/instrumentation/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java +++ b/instrumentation/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java @@ -11,7 +11,10 @@ import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; import static org.awaitility.Awaitility.await; import static org.mockito.ArgumentMatchers.anyCollection; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -19,18 +22,25 @@ import android.app.Activity; import android.app.Application; import androidx.annotation.NonNull; +import io.opentelemetry.android.config.DiskBufferingConfiguration; import io.opentelemetry.android.config.OtelRumConfig; import io.opentelemetry.android.instrumentation.ApplicationStateListener; +import io.opentelemetry.android.internal.services.CacheStorageService; +import io.opentelemetry.android.internal.services.PreferencesService; +import io.opentelemetry.android.internal.services.Service; +import io.opentelemetry.android.internal.services.ServiceManager; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.context.propagation.TextMapPropagator; +import io.opentelemetry.contrib.disk.buffering.SpanDiskExporter; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter; import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor; import io.opentelemetry.sdk.trace.export.SpanExporter; +import java.io.IOException; import java.time.Duration; import java.util.List; import java.util.function.Function; @@ -158,6 +168,69 @@ void setSpanExporterCustomizer() { .untilAsserted(() -> verify(exporter).export(anyCollection())); } + @Test + void diskBufferingEnabled() { + PreferencesService preferences = mock(); + CacheStorageService cacheStorage = mock(); + doReturn(60 * 1024 * 1024L).when(cacheStorage).ensureCacheSpaceAvailable(anyLong()); + setUpServiceManager(preferences, cacheStorage); + OtelRumConfig config = buildConfig(); + config.setDiskBufferingConfiguration( + DiskBufferingConfiguration.builder().setEnabled(true).build()); + + OpenTelemetryRum.builder(application, config) + .addSpanExporterCustomizer( + spanExporter -> { + assertThat(spanExporter).isInstanceOf(SpanDiskExporter.class); + return spanExporter; + }) + .build(); + } + + @Test + void diskBufferingEnabled_when_exception_thrown() { + PreferencesService preferences = mock(); + CacheStorageService cacheStorage = mock(); + doReturn(60 * 1024 * 1024L).when(cacheStorage).ensureCacheSpaceAvailable(anyLong()); + doAnswer( + invocation -> { + throw new IOException(); + }) + .when(cacheStorage) + .getCacheDir(); + setUpServiceManager(preferences, cacheStorage); + OtelRumConfig config = buildConfig(); + config.setDiskBufferingConfiguration( + DiskBufferingConfiguration.builder().setEnabled(true).build()); + + OpenTelemetryRum.builder(application, config) + .addSpanExporterCustomizer( + spanExporter -> { + assertThat(spanExporter).isNotInstanceOf(SpanDiskExporter.class); + return spanExporter; + }) + .build(); + } + + @Test + void diskBufferingDisabled() { + makeBuilder() + .addSpanExporterCustomizer( + spanExporter -> { + assertThat(spanExporter).isNotInstanceOf(SpanDiskExporter.class); + return spanExporter; + }) + .build(); + } + + private static void setUpServiceManager(Service... services) { + ServiceManager serviceManager = mock(); + for (Service service : services) { + doReturn(service).when(serviceManager).getService(service.getClass()); + } + ServiceManager.setForTest(serviceManager); + } + @NonNull private OpenTelemetryRumBuilder makeBuilder() { return OpenTelemetryRum.builder(application, buildConfig()); diff --git a/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/DiskManagerTest.java b/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/DiskManagerTest.java index 7566a18bc..70af58045 100644 --- a/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/DiskManagerTest.java +++ b/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/DiskManagerTest.java @@ -14,7 +14,6 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import io.opentelemetry.android.config.DiskBufferingConfiguration; -import io.opentelemetry.android.config.OtelRumConfig; import io.opentelemetry.android.internal.services.CacheStorageService; import io.opentelemetry.android.internal.services.PreferencesService; import io.opentelemetry.android.internal.services.ServiceManager; @@ -41,13 +40,11 @@ class DiskManagerTest { @BeforeEach void setUp() { - OtelRumConfig config = mock(); ServiceManager serviceManager = mock(); - doReturn(diskBufferingConfiguration).when(config).getDiskBufferingConfiguration(); doReturn(cacheStorageService).when(serviceManager).getService(CacheStorageService.class); doReturn(preferencesService).when(serviceManager).getService(PreferencesService.class); ServiceManager.setForTest(serviceManager); - diskManager = DiskManager.create(config); + diskManager = DiskManager.create(diskBufferingConfiguration); } @Test From 4b5fcce162b6977c2b40671479d0da6f33f7c8d4 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 4 Dec 2023 12:09:40 +0100 Subject: [PATCH 2/9] Moving test assertions out of nested callbacks --- .../android/OpenTelemetryRumBuilderTest.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/instrumentation/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java b/instrumentation/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java index a0a874a1b..379f18da7 100644 --- a/instrumentation/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java +++ b/instrumentation/src/test/java/io/opentelemetry/android/OpenTelemetryRumBuilderTest.java @@ -43,6 +43,7 @@ import java.io.IOException; import java.time.Duration; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -177,14 +178,17 @@ void diskBufferingEnabled() { OtelRumConfig config = buildConfig(); config.setDiskBufferingConfiguration( DiskBufferingConfiguration.builder().setEnabled(true).build()); + AtomicReference capturedExporter = new AtomicReference<>(); OpenTelemetryRum.builder(application, config) .addSpanExporterCustomizer( spanExporter -> { - assertThat(spanExporter).isInstanceOf(SpanDiskExporter.class); + capturedExporter.set(spanExporter); return spanExporter; }) .build(); + + assertThat(capturedExporter.get()).isInstanceOf(SpanDiskExporter.class); } @Test @@ -202,25 +206,32 @@ void diskBufferingEnabled_when_exception_thrown() { OtelRumConfig config = buildConfig(); config.setDiskBufferingConfiguration( DiskBufferingConfiguration.builder().setEnabled(true).build()); + AtomicReference capturedExporter = new AtomicReference<>(); OpenTelemetryRum.builder(application, config) .addSpanExporterCustomizer( spanExporter -> { - assertThat(spanExporter).isNotInstanceOf(SpanDiskExporter.class); + capturedExporter.set(spanExporter); return spanExporter; }) .build(); + + assertThat(capturedExporter.get()).isNotInstanceOf(SpanDiskExporter.class); } @Test void diskBufferingDisabled() { + AtomicReference capturedExporter = new AtomicReference<>(); + makeBuilder() .addSpanExporterCustomizer( spanExporter -> { - assertThat(spanExporter).isNotInstanceOf(SpanDiskExporter.class); + capturedExporter.set(spanExporter); return spanExporter; }) .build(); + + assertThat(capturedExporter.get()).isNotInstanceOf(SpanDiskExporter.class); } private static void setUpServiceManager(Service... services) { From d9f682fa77db26dbcd8449b711c89af8dd84bfdc Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 4 Dec 2023 13:59:46 +0100 Subject: [PATCH 3/9] Logging when disk span exporter init fails --- .../opentelemetry/android/OpenTelemetryRumBuilder.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java index 3b47f2011..14181bb62 100644 --- a/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java +++ b/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java @@ -42,6 +42,8 @@ import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; +import java.util.logging.Level; +import java.util.logging.Logger; /** * A builder of {@link OpenTelemetryRum}. It enabled configuring the OpenTelemetry SDK and disabling @@ -62,6 +64,7 @@ public final class OpenTelemetryRumBuilder { loggerProviderCustomizers = new ArrayList<>(); private final OtelRumConfig config; private final VisibleScreenTracker visibleScreenTracker = new VisibleScreenTracker(); + private static final Logger LOGGER = Logger.getLogger("OpenTelemetryRumBuilder"); private Function spanExporterCustomizer = a -> a; private final List> instrumentationInstallers = @@ -314,17 +317,15 @@ private SdkTracerProvider buildTracerProvider(SessionId sessionId, Application a private SpanExporter buildSpanExporter() { // TODO: Default to otlp...but how can we make endpoint and auth mandatory? SpanExporter defaultExporter = LoggingSpanExporter.create(); - SpanExporter spanExporter; + SpanExporter spanExporter = defaultExporter; DiskBufferingConfiguration diskBufferingConfiguration = config.getDiskBufferingConfiguration(); if (diskBufferingConfiguration.isEnabled()) { try { spanExporter = createDiskExporter(defaultExporter, diskBufferingConfiguration); } catch (IOException e) { - spanExporter = defaultExporter; + LOGGER.log(Level.WARNING, "Could not create span disk exporter", e); } - } else { - spanExporter = defaultExporter; } return spanExporterCustomizer.apply(spanExporter); } From 3dbd9a8b526001b7009e0630328d258717f3952f Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 4 Dec 2023 14:01:01 +0100 Subject: [PATCH 4/9] Updating log message --- .../java/io/opentelemetry/android/OpenTelemetryRumBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java index 14181bb62..5888547e9 100644 --- a/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java +++ b/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java @@ -324,7 +324,7 @@ private SpanExporter buildSpanExporter() { try { spanExporter = createDiskExporter(defaultExporter, diskBufferingConfiguration); } catch (IOException e) { - LOGGER.log(Level.WARNING, "Could not create span disk exporter", e); + LOGGER.log(Level.WARNING, "Could not create span disk exporter.", e); } } return spanExporterCustomizer.apply(spanExporter); From 5917780700ec4b81fd128543fdb46a18336ea649 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 4 Dec 2023 14:10:30 +0100 Subject: [PATCH 5/9] Created SimpleTemporaryFileProviderTest --- .../SimpleTemporaryFileProviderTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProviderTest.java diff --git a/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProviderTest.java b/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProviderTest.java new file mode 100644 index 000000000..dedbb5c97 --- /dev/null +++ b/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProviderTest.java @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.android.internal.features.persistence; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class SimpleTemporaryFileProviderTest { + @TempDir File tempDir; + + @Test + void createUniqueFilesBasedOnCurrentTimeAndPrefix() throws InterruptedException { + SimpleTemporaryFileProvider provider = new SimpleTemporaryFileProvider(tempDir); + File first = provider.createTemporaryFile("a"); + File second = provider.createTemporaryFile("b"); + Thread.sleep(1); + File third = provider.createTemporaryFile("a"); + + assertThat(first.getName()).startsWith("a").endsWith(".tmp"); + assertThat(second.getName()).startsWith("b").endsWith(".tmp"); + assertThat(third.getName()).startsWith("a").endsWith(".tmp"); + assertThat(first).isNotEqualTo(third); + } +} From 9a3c9e184ea3b2a56d9134b239edbd5d4faf775f Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Tue, 5 Dec 2023 09:47:46 +0100 Subject: [PATCH 6/9] Adding javadoc to SimpleTemporaryFileProvider.createTemporaryFile --- .../features/persistence/SimpleTemporaryFileProvider.java | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java index 63380e25c..17592d1e9 100644 --- a/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java +++ b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java @@ -18,6 +18,7 @@ public SimpleTemporaryFileProvider(File tempDir) { this.tempDir = tempDir; } + /** Creates a unique file instance using the provided prefix and the current time in millis. */ @Override public File createTemporaryFile(String prefix) { return new File(tempDir, prefix + "_" + System.currentTimeMillis() + ".tmp"); From b089905c2b7f27c4490128de36ca76005c3ad420 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Tue, 5 Dec 2023 09:53:08 +0100 Subject: [PATCH 7/9] Replacing java Logger by android Log --- .../android/OpenTelemetryRumBuilder.java | 6 ++---- .../features/persistence/DiskManager.java | 17 ++++++++--------- .../internal/services/CacheStorageService.java | 15 +++++++-------- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java index 5888547e9..3ccd9041c 100644 --- a/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java +++ b/instrumentation/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java @@ -8,6 +8,7 @@ import static java.util.Objects.requireNonNull; import android.app.Application; +import android.util.Log; import io.opentelemetry.android.config.DiskBufferingConfiguration; import io.opentelemetry.android.config.OtelRumConfig; import io.opentelemetry.android.instrumentation.InstrumentedApplication; @@ -42,8 +43,6 @@ import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A builder of {@link OpenTelemetryRum}. It enabled configuring the OpenTelemetry SDK and disabling @@ -64,7 +63,6 @@ public final class OpenTelemetryRumBuilder { loggerProviderCustomizers = new ArrayList<>(); private final OtelRumConfig config; private final VisibleScreenTracker visibleScreenTracker = new VisibleScreenTracker(); - private static final Logger LOGGER = Logger.getLogger("OpenTelemetryRumBuilder"); private Function spanExporterCustomizer = a -> a; private final List> instrumentationInstallers = @@ -324,7 +322,7 @@ private SpanExporter buildSpanExporter() { try { spanExporter = createDiskExporter(defaultExporter, diskBufferingConfiguration); } catch (IOException e) { - LOGGER.log(Level.WARNING, "Could not create span disk exporter.", e); + Log.w(RumConstants.OTEL_RUM_LOG_TAG, "Could not create span disk exporter.", e); } } return spanExporterCustomizer.apply(spanExporter); diff --git a/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.java b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.java index 7b87fedaf..f6739c673 100644 --- a/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.java +++ b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.java @@ -5,21 +5,20 @@ package io.opentelemetry.android.internal.features.persistence; +import android.util.Log; +import io.opentelemetry.android.RumConstants; import io.opentelemetry.android.config.DiskBufferingConfiguration; import io.opentelemetry.android.internal.services.CacheStorageService; import io.opentelemetry.android.internal.services.PreferencesService; import io.opentelemetry.android.internal.services.ServiceManager; import java.io.File; import java.io.IOException; -import java.util.logging.Level; -import java.util.logging.Logger; /** * This class is internal and not for public use. Its APIs are unstable and can change at any time. */ public final class DiskManager { private static final String MAX_FOLDER_SIZE_KEY = "max_signal_folder_size"; - private static final Logger logger = Logger.getLogger("DiskManager"); private final CacheStorageService cacheStorageService; private final PreferencesService preferencesService; private final DiskBufferingConfiguration diskBufferingConfiguration; @@ -75,8 +74,8 @@ public File getTemporaryDir() throws IOException { public int getMaxFolderSize() { int storedSize = preferencesService.retrieveInt(MAX_FOLDER_SIZE_KEY, -1); if (storedSize > 0) { - logger.log( - Level.FINER, + Log.d( + RumConstants.OTEL_RUM_LOG_TAG, String.format("Returning max folder size from preferences: %s", storedSize)); return storedSize; } @@ -88,8 +87,8 @@ public int getMaxFolderSize() { int maxCacheFileSize = getMaxCacheFileSize(); int calculatedSize = (availableCacheSize / 3) - maxCacheFileSize; if (calculatedSize < maxCacheFileSize) { - logger.log( - Level.WARNING, + Log.w( + RumConstants.OTEL_RUM_LOG_TAG, String.format( "Insufficient folder cache size: %s, it must be at least: %s", calculatedSize, maxCacheFileSize)); @@ -97,8 +96,8 @@ public int getMaxFolderSize() { } preferencesService.store(MAX_FOLDER_SIZE_KEY, calculatedSize); - logger.log( - Level.FINER, + Log.d( + RumConstants.OTEL_RUM_LOG_TAG, String.format( "Requested cache size: %s, available cache size: %s, folder size: %s", requestedSize, availableCacheSize, calculatedSize)); diff --git a/instrumentation/src/main/java/io/opentelemetry/android/internal/services/CacheStorageService.java b/instrumentation/src/main/java/io/opentelemetry/android/internal/services/CacheStorageService.java index 79767ab00..1dfc8b042 100644 --- a/instrumentation/src/main/java/io/opentelemetry/android/internal/services/CacheStorageService.java +++ b/instrumentation/src/main/java/io/opentelemetry/android/internal/services/CacheStorageService.java @@ -8,13 +8,13 @@ import android.content.Context; import android.os.Build; import android.os.storage.StorageManager; +import android.util.Log; import androidx.annotation.RequiresApi; import androidx.annotation.WorkerThread; +import io.opentelemetry.android.RumConstants; import java.io.File; import java.io.IOException; import java.util.UUID; -import java.util.logging.Level; -import java.util.logging.Logger; /** * Utility to get information about the host app. @@ -24,7 +24,6 @@ */ public class CacheStorageService implements Service { private final Context appContext; - private static final Logger logger = Logger.getLogger("CacheStorageService"); public CacheStorageService(Context appContext) { this.appContext = appContext; @@ -53,8 +52,8 @@ public long ensureCacheSpaceAvailable(long maxSpaceNeeded) { @RequiresApi(api = Build.VERSION_CODES.O) private long getAvailableSpace(File directory, long maxSpaceNeeded) { - logger.log( - Level.FINER, + Log.d( + RumConstants.OTEL_RUM_LOG_TAG, String.format( "Getting available space for %s, max needed is: %s", directory, maxSpaceNeeded)); @@ -70,14 +69,14 @@ private long getAvailableSpace(File directory, long maxSpaceNeeded) { storageManager.allocateBytes(appSpecificInternalDirUuid, spaceToAllocate); return spaceToAllocate; } catch (IOException e) { - logger.log(Level.WARNING, "Failed to get available space", e); + Log.w(RumConstants.OTEL_RUM_LOG_TAG, "Failed to get available space", e); return getLegacyAvailableSpace(directory, maxSpaceNeeded); } } private long getLegacyAvailableSpace(File directory, long maxSpaceNeeded) { - logger.log( - Level.FINER, + Log.d( + RumConstants.OTEL_RUM_LOG_TAG, String.format( "Getting legacy available space for %s max needed is: %s", directory, maxSpaceNeeded)); From 62063462923e7ef26cd936681e81a56338f9d637 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Tue, 12 Dec 2023 09:56:19 +0100 Subject: [PATCH 8/9] Improving tests for SimpleTemporaryFileProvider --- .../features/persistence/SimpleTemporaryFileProviderTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProviderTest.java b/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProviderTest.java index dedbb5c97..2de5741e1 100644 --- a/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProviderTest.java +++ b/instrumentation/src/test/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProviderTest.java @@ -26,5 +26,9 @@ void createUniqueFilesBasedOnCurrentTimeAndPrefix() throws InterruptedException assertThat(second.getName()).startsWith("b").endsWith(".tmp"); assertThat(third.getName()).startsWith("a").endsWith(".tmp"); assertThat(first).isNotEqualTo(third); + + assertThat(first.getParentFile()).isEqualTo(tempDir); + assertThat(second.getParentFile()).isEqualTo(tempDir); + assertThat(third.getParentFile()).isEqualTo(tempDir); } } From db4901198d533f657b49a705a78c5717277a5de0 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Tue, 12 Dec 2023 10:05:15 +0100 Subject: [PATCH 9/9] Using UUID to reduce the changes of creating duplicate temp files --- .../features/persistence/SimpleTemporaryFileProvider.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java index 17592d1e9..4e36494dc 100644 --- a/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java +++ b/instrumentation/src/main/java/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java @@ -7,6 +7,7 @@ import io.opentelemetry.contrib.disk.buffering.internal.files.TemporaryFileProvider; import java.io.File; +import java.util.UUID; /** * This class is internal and not for public use. Its APIs are unstable and can change at any time. @@ -21,6 +22,6 @@ public SimpleTemporaryFileProvider(File tempDir) { /** Creates a unique file instance using the provided prefix and the current time in millis. */ @Override public File createTemporaryFile(String prefix) { - return new File(tempDir, prefix + "_" + System.currentTimeMillis() + ".tmp"); + return new File(tempDir, prefix + "_" + UUID.randomUUID() + ".tmp"); } }