Guard against empty files. (#407)

* guard against creating empty spans files and delete them gracefully on the exporter side. * spotless * remove test code
open-telemetry · Nov 15, 2022 · 7893159 · 7893159
1 parent 1287cea
commit 7893159
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 7 deletions.
diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/FileSender.java b/splunk-otel-android/src/main/java/com/splunk/rum/FileSender.java
@@ -60,6 +60,7 @@ boolean handleFileOnDisk(File file) {
         Log.d(LOG_TAG, "Reading file content for ingest: " + file);
         List<byte[]> encodedSpans = readFileCompletely(file);
         if (encodedSpans.isEmpty()) {
+            fileUtils.safeDelete(file);
             return false;
         }
 

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
@@ -59,6 +59,9 @@ public int messageSizeInBytes(List<byte[]> encodedSpans) {
 
     @Override
     public Call<Void> sendSpans(List<byte[]> encodedSpans) {
+        if (encodedSpans.isEmpty()) {
+            return Call.create(null);
+        }
         if (!storageLimiter.ensureFreeSpace()) {
             Log.e(
                     SplunkRum.LOG_TAG,

diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/FileSenderTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/FileSenderTest.java
@@ -62,26 +62,27 @@ void setup() throws Exception {
     }
 
     @Test
-    void testEmptyFile() throws Exception {
+    void sendEmptyFile() throws Exception {
         Mockito.reset(fileUtils);
         Mockito.reset(delegate);
         File file = new File("/asdflkajsdfoij");
         when(fileUtils.readFileCompletely(file)).thenReturn(emptyList());
         FileSender sender = buildSender();
         boolean result = sender.handleFileOnDisk(file);
         assertFalse(result);
+        verify(fileUtils).safeDelete(file);
     }
 
     @Test
-    void testHappyPathSendSpans() {
+    void happyPathSendSpans() {
         FileSender sender = buildSender();
         boolean result = sender.handleFileOnDisk(file);
         assertTrue(result);
         verify(bandwidthTracker).tick(fileSpans);
     }
 
     @Test
-    void testSendFailsButNotExceeded() throws Exception {
+    void sendFailsButNotExceeded() throws Exception {
         when(httpCall.execute()).thenThrow(new IOException("boom"));
         FileSender sender = buildSender();
         boolean result = sender.handleFileOnDisk(file);
@@ -91,7 +92,7 @@ void testSendFailsButNotExceeded() throws Exception {
     }
 
     @Test
-    void testSenderFailureRetriesExhausted() throws Exception {
+    void senderFailureRetriesExhausted() throws Exception {
         when(httpCall.execute()).thenThrow(new IOException("boom"));
         FileSender sender = buildSender(3);
         boolean result = sender.handleFileOnDisk(file);
@@ -109,7 +110,7 @@ void testSenderFailureRetriesExhausted() throws Exception {
     }
 
     @Test
-    void testReadFileFails() throws IOException {
+    void readFileFails() throws IOException {
         Mockito.reset(fileUtils);
         Mockito.reset(delegate);
         when(fileUtils.readFileCompletely(file)).thenThrow(new IOException("boom"));

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
@@ -16,8 +16,11 @@
 
 package com.splunk.rum;
 
+import static java.util.Collections.emptyList;
 import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
@@ -51,8 +54,8 @@ class ZipkinToDiskSenderTest {
 
     @BeforeEach
     void setup() {
-        when(clock.now()).thenReturn(now);
-        when(limiter.ensureFreeSpace()).thenReturn(true);
+        lenient().when(clock.now()).thenReturn(now);
+        lenient().when(limiter.ensureFreeSpace()).thenReturn(true);
     }
 
     @Test
@@ -70,6 +73,18 @@ void testHappyPath() throws Exception {
         verify(fileUtils).writeAsLines(finalPath, spans);
     }
 
+    @Test
+    void testEmptyListDoesNotWriteFile() {
+        ZipkinToDiskSender sender =
+                ZipkinToDiskSender.builder()
+                        .path(path)
+                        .fileUtils(fileUtils)
+                        .storageLimiter(limiter)
+                        .build();
+        sender.sendSpans(emptyList());
+        verifyNoInteractions(fileUtils);
+    }
+
     @Test
     void testWriteFails() throws Exception {
         doThrow(new IOException("boom")).when(fileUtils).writeAsLines(finalPath, spans);