diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/DefaultSpanStorage.java b/splunk-otel-android/src/main/java/com/splunk/rum/DefaultSpanStorage.java index 2644e91f..5fec880d 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/DefaultSpanStorage.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/DefaultSpanStorage.java @@ -31,7 +31,7 @@ public DefaultSpanStorage(FileUtils fileUtils, File rootDir) { } @Override - public File provideSpanFile() { + public File provideSpansDirectory() { File spansPath = fileUtils.getSpansDirectory(rootDir); if (spansPath.exists() || spansPath.mkdirs()) { return spansPath; @@ -45,16 +45,16 @@ public File provideSpanFile() { @Override public Stream getAllSpanFiles() { - return fileUtils.listSpanFiles(provideSpanFile()); + return fileUtils.listSpanFiles(provideSpansDirectory()); } @Override public long getTotalFileSizeInBytes() { - return fileUtils.getTotalFileSizeInBytes(provideSpanFile()); + return fileUtils.getTotalFileSizeInBytes(provideSpansDirectory()); } @Override public Stream getPendingFiles() { - return fileUtils.listSpanFiles(provideSpanFile()); + return fileUtils.listSpanFiles(provideSpansDirectory()); } } diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/DeviceSpanStorageLimiter.java b/splunk-otel-android/src/main/java/com/splunk/rum/DeviceSpanStorageLimiter.java index 1f04dbf1..d71f2c15 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/DeviceSpanStorageLimiter.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/DeviceSpanStorageLimiter.java @@ -52,7 +52,7 @@ private DeviceSpanStorageLimiter(Builder builder) { boolean ensureFreeSpace() { tryFreeingSpace(); // play nice if disk is getting full - return fileProvider.provideSpanFile().getFreeSpace() > limitInBytes(); + return fileProvider.provideSpansDirectory().getFreeSpace() > limitInBytes(); } private void tryFreeingSpace() { diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/SpanStorage.java b/splunk-otel-android/src/main/java/com/splunk/rum/SpanStorage.java index a0295ce2..a41dde85 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/SpanStorage.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/SpanStorage.java @@ -26,10 +26,9 @@ interface SpanStorage { /*** - * Provide location of storing spans - * @return + * Returns the location where spans are buffered. */ - File provideSpanFile(); + File provideSpansDirectory(); /*** * @return all spans including those that can be sent or not diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/StartTypeAwareSpanStorage.java b/splunk-otel-android/src/main/java/com/splunk/rum/StartTypeAwareSpanStorage.java index eb33a385..a3c98f6c 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/StartTypeAwareSpanStorage.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/StartTypeAwareSpanStorage.java @@ -19,6 +19,7 @@ import static com.splunk.rum.SplunkRum.LOG_TAG; import android.util.Log; +import androidx.annotation.VisibleForTesting; import io.opentelemetry.android.instrumentation.activity.VisibleScreenTracker; import java.io.File; import java.util.UUID; @@ -29,7 +30,7 @@ * If the app is brought to foreground and the same session ID still in use, the background spans are moved to /span for eventual sending. * If the app still in the background until process-kill, the background span files will eventually be deleted by the @DeviceSpanStorageLimiter. */ -public class StartTypeAwareSpanStorage implements SpanStorage { +class StartTypeAwareSpanStorage implements SpanStorage { private final VisibleScreenTracker visibleScreenTracker; private final FileUtils fileUtils; @@ -39,19 +40,37 @@ public class StartTypeAwareSpanStorage implements SpanStorage { static StartTypeAwareSpanStorage create( VisibleScreenTracker visibleScreenTracker, FileUtils fileUtils, File rootDir) { + File spansDir = fileUtils.getSpansDirectory(rootDir); + String uniqueId = UUID.randomUUID().toString(); + return create(visibleScreenTracker, fileUtils, rootDir, spansDir, uniqueId); + } + + @VisibleForTesting + static StartTypeAwareSpanStorage create( + VisibleScreenTracker visibleScreenTracker, + FileUtils fileUtils, + File rootDir, + File spansDir, + String uniqueId) { StartTypeAwareSpanStorage startTypeAwareSpanStorage = - new StartTypeAwareSpanStorage(visibleScreenTracker, fileUtils, rootDir); + new StartTypeAwareSpanStorage( + visibleScreenTracker, fileUtils, rootDir, spansDir, uniqueId); startTypeAwareSpanStorage.cleanupUnsentBackgroundSpans(); return startTypeAwareSpanStorage; } - private StartTypeAwareSpanStorage( - VisibleScreenTracker visibleScreenTracker, FileUtils fileUtils, File rootDir) { + @VisibleForTesting + StartTypeAwareSpanStorage( + VisibleScreenTracker visibleScreenTracker, + FileUtils fileUtils, + File rootDir, + File spansDir, + String uniqueId) { this.visibleScreenTracker = visibleScreenTracker; this.fileUtils = fileUtils; this.rootDir = rootDir; - this.spanDir = fileUtils.getSpansDirectory(rootDir); - this.uniqueId = UUID.randomUUID().toString(); + this.spanDir = spansDir; + this.uniqueId = uniqueId; } @Override @@ -80,7 +99,7 @@ private boolean isAppForeground() { private void moveBackgroundSpanToPendingSpan() { fileUtils - .listSpanFiles(getCurrentSessionBackgroundFile()) + .listSpanFiles(getCurrentSessionBackgroundDirectory()) .forEach( file -> { File destinationFile = new File(spanDir, file.getName()); @@ -96,19 +115,19 @@ private void moveBackgroundSpanToPendingSpan() { cleanupUnsentBackgroundSpans(); } - private File getCurrentSessionBackgroundFile() { + private File getCurrentSessionBackgroundDirectory() { return new File(spanDir, "background/" + uniqueId); } @Override - public File provideSpanFile() { - return ensureDirExist(getSpanFile()); + public File provideSpansDirectory() { + return ensureDirExist(getSpansDirectory()); } - private File getSpanFile() { + private File getSpansDirectory() { if (!isAppForeground()) { Log.d(LOG_TAG, "Creating background span " + uniqueId); - return getCurrentSessionBackgroundFile(); + return getCurrentSessionBackgroundDirectory(); } Log.d(LOG_TAG, "Creating foreground span " + uniqueId); return spanDir; @@ -139,5 +158,9 @@ private void cleanupUnsentBackgroundSpans() { fileUtils.listFilesRecursively(dir).forEach(fileUtils::safeDelete); fileUtils.safeDelete(dir); }); + File backgroundDir = getCurrentSessionBackgroundDirectory(); + if (!fileUtils.listFilesRecursively(backgroundDir).findAny().isPresent()) { + fileUtils.safeDelete(backgroundDir); + } } } diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/ZipkinToDiskSender.java b/splunk-otel-android/src/main/java/com/splunk/rum/ZipkinToDiskSender.java index 54e74da3..bd9200db 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/ZipkinToDiskSender.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/ZipkinToDiskSender.java @@ -81,7 +81,7 @@ public Call sendSpans(List encodedSpans) { } private File createFilename(long now) { - return new File(spanStorage.provideSpanFile(), now + ".spans"); + return new File(spanStorage.provideSpansDirectory(), now + ".spans"); } static Builder builder() { diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/DeviceSpanStorageLimiterTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/DeviceSpanStorageLimiterTest.java index ab0e72aa..dd68f216 100644 --- a/splunk-otel-android/src/test/java/com/splunk/rum/DeviceSpanStorageLimiterTest.java +++ b/splunk-otel-android/src/test/java/com/splunk/rum/DeviceSpanStorageLimiterTest.java @@ -56,7 +56,7 @@ void setup() { void ensureFreeSpace_littleUsageEnoughFreeSpace() { File mockFile = mock(File.class); when(spanStorage.getTotalFileSizeInBytes()).thenReturn(10 * 1024L); - when(spanStorage.provideSpanFile()).thenReturn(mockFile); + when(spanStorage.provideSpansDirectory()).thenReturn(mockFile); when(mockFile.getFreeSpace()).thenReturn(99L); // Disk is very full assertFalse(limiter.ensureFreeSpace()); verify(fileUtils, never()).safeDelete(any()); @@ -66,7 +66,7 @@ void ensureFreeSpace_littleUsageEnoughFreeSpace() { void ensureFreeSpace_littleUsageButNotEnoughFreeSpace() { File mockFile = mock(File.class); when(spanStorage.getTotalFileSizeInBytes()).thenReturn(10 * 1024L); - when(spanStorage.provideSpanFile()).thenReturn(mockFile); + when(spanStorage.provideSpansDirectory()).thenReturn(mockFile); when(mockFile.getFreeSpace()).thenReturn(MAX_STORAGE_USE_BYTES * 99); // lots of room assertTrue(limiter.ensureFreeSpace()); verify(fileUtils, never()).safeDelete(any()); @@ -75,7 +75,7 @@ void ensureFreeSpace_littleUsageButNotEnoughFreeSpace() { @Test void ensureFreeSpace_underLimit() { File mockFile = mock(File.class); - when(spanStorage.provideSpanFile()).thenReturn(mockFile); + when(spanStorage.provideSpansDirectory()).thenReturn(mockFile); when(spanStorage.getTotalFileSizeInBytes()).thenReturn(MAX_STORAGE_USE_BYTES - 1); when(mockFile.getFreeSpace()).thenReturn(MAX_STORAGE_USE_BYTES + 1); @@ -91,7 +91,7 @@ void ensureFreeSpace_overLimitHappyDeletion() { File file3 = new File("newest"); File mockFile = mock(File.class); - when(spanStorage.provideSpanFile()).thenReturn(mockFile); + when(spanStorage.provideSpansDirectory()).thenReturn(mockFile); when(spanStorage.getTotalFileSizeInBytes()).thenReturn(MAX_STORAGE_USE_BYTES + 1); when(fileUtils.getModificationTime(file1)).thenReturn(1000L); when(fileUtils.getModificationTime(file2)).thenReturn(1001L); diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/DiskToZipkinExporterTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/DiskToZipkinExporterTest.java index 028e6f78..1dda3cba 100644 --- a/splunk-otel-android/src/test/java/com/splunk/rum/DiskToZipkinExporterTest.java +++ b/splunk-otel-android/src/test/java/com/splunk/rum/DiskToZipkinExporterTest.java @@ -54,11 +54,14 @@ class DiskToZipkinExporterTest { @BeforeEach void setup() throws Exception { Mockito.reset(SPAN_STORAGE); - when(SPAN_STORAGE.provideSpanFile()).thenReturn(spanFilesPath); - file1 = new File(SPAN_STORAGE.provideSpanFile() + File.separator + "file1.spans"); - file2 = new File(SPAN_STORAGE.provideSpanFile() + File.separator + "file2.spans"); + when(SPAN_STORAGE.provideSpansDirectory()).thenReturn(spanFilesPath); + file1 = new File(SPAN_STORAGE.provideSpansDirectory() + File.separator + "file1.spans"); + file2 = new File(SPAN_STORAGE.provideSpansDirectory() + File.separator + "file2.spans"); imposter = - new File(SPAN_STORAGE.provideSpanFile() + File.separator + "someImposterFile.dll"); + new File( + SPAN_STORAGE.provideSpansDirectory() + + File.separator + + "someImposterFile.dll"); when(currentNetworkProvider.refreshNetworkStatus()).thenReturn(currentNetwork); when(currentNetwork.isOnline()).thenReturn(true); diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/StartTypeAwareSpanStorageTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/StartTypeAwareSpanStorageTest.java index 5cb43674..638013ef 100644 --- a/splunk-otel-android/src/test/java/com/splunk/rum/StartTypeAwareSpanStorageTest.java +++ b/splunk-otel-android/src/test/java/com/splunk/rum/StartTypeAwareSpanStorageTest.java @@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -28,23 +29,25 @@ import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; -public class StartTypeAwareSpanStorageTest { +class StartTypeAwareSpanStorageTest { - private final FileUtils fileUtils = mock(); - - private final File rootDir = new File("files/"); - private final VisibleScreenTracker visibleScreenTracker = mock(); + final File rootDir = new File("files/"); + VisibleScreenTracker visibleScreenTracker; + FileUtils fileUtils; private StartTypeAwareSpanStorage fileProvider; @BeforeEach void setup() { + visibleScreenTracker = mock(); + fileUtils = mock(); when(fileUtils.getSpansDirectory(rootDir)).thenReturn(new File(rootDir, "spans")); fileProvider = StartTypeAwareSpanStorage.create(visibleScreenTracker, fileUtils, rootDir); } @@ -52,18 +55,48 @@ void setup() { @Test void create_onNewId_shouldCleanOldBackgroundFiles() { File file = mock(); + ArgumentCaptor fileArgumentCaptor = ArgumentCaptor.forClass(File.class); when(file.getPath()).thenReturn("files/spans/background/123"); + + fileProvider = StartTypeAwareSpanStorage.create(visibleScreenTracker, fileUtils, rootDir); + fileUtils = mock(FileUtils.class); when(fileUtils.listDirectories(any())).thenReturn(Stream.of(file)); - ArgumentCaptor fileArgumentCaptor = ArgumentCaptor.forClass(File.class); + when(fileUtils.listFiles(file)).thenReturn(Stream.of(file)); fileProvider = StartTypeAwareSpanStorage.create(visibleScreenTracker, fileUtils, rootDir); + verify(fileUtils, times(2)).safeDelete(fileArgumentCaptor.capture()); + assertEquals(file, fileArgumentCaptor.getAllValues().get(0)); + assertEquals( + fileProvider.provideSpansDirectory(), fileArgumentCaptor.getAllValues().get(1)); + } + + @Test + void create_cleanDoesntRemoveDirIfNotEmpty() { + String uniqueId = UUID.randomUUID().toString(); + File spansDir = new File("files/spans"); + File file = mock(); + File file2 = mock(); + fileUtils = mock(FileUtils.class); + + ArgumentCaptor fileArgumentCaptor = ArgumentCaptor.forClass(File.class); + when(file.getPath()).thenReturn("files/spans/background/123"); + + when(fileUtils.listDirectories(any())).thenReturn(Stream.of(file)); + when(fileUtils.listFiles(file)).thenReturn(Stream.of(file)); + when(fileUtils.listFilesRecursively(new File(spansDir, "background/" + uniqueId))) + .thenReturn(Stream.of(file2)); + + fileProvider = + StartTypeAwareSpanStorage.create( + visibleScreenTracker, fileUtils, rootDir, spansDir, uniqueId); + verify(fileUtils).safeDelete(fileArgumentCaptor.capture()); - assertEquals(file.getPath(), fileArgumentCaptor.getValue().getPath()); + assertEquals(file, fileArgumentCaptor.getAllValues().get(0)); } @Test - void getPendingFiles_givenInBackground_shouldReturnForegoundOnlySpan() { + void getPendingFiles_givenInBackground_shouldReturnForegroundOnlySpan() { when(visibleScreenTracker.getPreviouslyVisibleScreen()).thenReturn(null); when(visibleScreenTracker.getCurrentlyVisibleScreen()).thenReturn("unknown"); List spans = fileProvider.getPendingFiles().collect(Collectors.toList()); @@ -72,7 +105,7 @@ void getPendingFiles_givenInBackground_shouldReturnForegoundOnlySpan() { @Test void - getPendingFiles_givenPrevouslyInBackground_shouldMoveBackgroundSpanToForegroundSpanForSending() { + getPendingFiles_givenPreviouslyInBackground_shouldMoveBackgroundSpanToForegroundSpanForSending() { when(visibleScreenTracker.getPreviouslyVisibleScreen()).thenReturn("LauncherActivity"); when(visibleScreenTracker.getCurrentlyVisibleScreen()).thenReturn("MainActivity"); @@ -104,7 +137,7 @@ void getSpanPath_givenInBackground_shouldReturnBackgroundSpanPath() { when(visibleScreenTracker.getPreviouslyVisibleScreen()).thenReturn(null); when(visibleScreenTracker.getCurrentlyVisibleScreen()).thenReturn("unknown"); - File path = fileProvider.provideSpanFile(); + File path = fileProvider.provideSpansDirectory(); assertTrue(path.getPath().startsWith("files/spans/background/")); } @@ -114,7 +147,7 @@ void getSpanPath_givenInForeground_shouldReturnForegroundSpanPath() { when(visibleScreenTracker.getPreviouslyVisibleScreen()).thenReturn("LauncherActivity"); when(visibleScreenTracker.getCurrentlyVisibleScreen()).thenReturn("MainActivity"); - File path = fileProvider.provideSpanFile(); + File path = fileProvider.provideSpansDirectory(); assertFalse(path.getPath().startsWith("files/spans/background/")); } diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/ZipkinToDiskSenderTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/ZipkinToDiskSenderTest.java index 37f765d9..a21d6f0c 100644 --- a/splunk-otel-android/src/test/java/com/splunk/rum/ZipkinToDiskSenderTest.java +++ b/splunk-otel-android/src/test/java/com/splunk/rum/ZipkinToDiskSenderTest.java @@ -63,7 +63,7 @@ void setup() { @Test void testHappyPath() throws Exception { - when(spanStorage.provideSpanFile()).thenReturn(path); + when(spanStorage.provideSpansDirectory()).thenReturn(path); ZipkinToDiskSender sender = ZipkinToDiskSender.builder() @@ -91,7 +91,7 @@ void testEmptyListDoesNotWriteFile() { @Test void testWriteFails() throws Exception { - when(spanStorage.provideSpanFile()).thenReturn(path); + when(spanStorage.provideSpansDirectory()).thenReturn(path); doThrow(new IOException("boom")).when(fileUtils).writeAsLines(finalPath, spans); ZipkinToDiskSender sender =