Avoid sending empty profiles (#2232)

* added a check on the sampled profile not to be empty in order to be sent * added unit and ui test to check empty profiles are discarded
getsentry · Sep 7, 2022 · b598e3e · b598e3e
1 parent 8312deb
commit b598e3e
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Avoid sending empty profiles ([#2232](https://github.com/getsentry/sentry-java/pull/2232))
+
 ## 6.4.1
 
 ### Fixes

diff --git a/...ests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/...ests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt
@@ -13,6 +13,7 @@ import io.sentry.SentryEvent
 import io.sentry.SentryOptions
 import io.sentry.protocol.SentryTransaction
 import org.junit.runner.RunWith
+import java.io.File
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertTrue
@@ -65,6 +66,32 @@ class EnvelopeTests : BaseUiTest() {
         }
     }
 
+    @Test
+    fun checkProfileNotSentIfEmpty() {
+
+        initSentry(true) { options: SentryOptions ->
+            options.tracesSampleRate = 1.0
+            options.profilesSampleRate = 1.0
+        }
+        relayIdlingResource.increment()
+        val transaction = Sentry.startTransaction("e2etests", "test empty")
+        transaction.finish()
+        // Let's modify the trace file to be empty, so that the profile will actually be empty.
+        val profilesDirPath = Sentry.getCurrentHub().options.profilingTracesDirPath
+        val origProfileFile = File(profilesDirPath!!).listFiles()?.maxByOrNull { f -> f.lastModified() }
+        origProfileFile?.writeBytes(ByteArray(0))
+
+        relay.assert {
+            assertEnvelope {
+                it.assertItem<SentryTransaction>()
+                // Since the profile is empty, it is discarded and not sent to Sentry
+                it.assertNoOtherItems()
+            }
+            assertNoOtherEnvelopes()
+            assertNoOtherRequests()
+        }
+    }
+
     @Test
     fun sendProfiledTransaction() {
         // This is a dogfooding test

diff --git a/...mples/sentry-samples-android/src/main/java/io/sentry/samples/android/ProfilingActivity.kt b/...mples/sentry-samples-android/src/main/java/io/sentry/samples/android/ProfilingActivity.kt
@@ -21,7 +21,7 @@ class ProfilingActivity : AppCompatActivity() {
 
     private lateinit var binding: ActivityProfilingBinding
     private val executors = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors())
-    private var profileFinished = false
+    private var profileFinished = true
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)

diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java
@@ -226,6 +226,9 @@ public static SentryEnvelopeItem fromAttachment(
               // base64
               byte[] traceFileBytes = readBytesFromFile(traceFile.getPath(), maxTraceFileSize);
               String base64Trace = Base64.encodeToString(traceFileBytes, NO_WRAP | NO_PADDING);
+              if (base64Trace.isEmpty()) {
+                throw new SentryEnvelopeException("Profiling trace file is empty");
+              }
               profilingTraceData.setSampledProfile(base64Trace);
               profilingTraceData.readDeviceCpuFrequencies();
 

diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt
@@ -90,6 +90,7 @@ class SentryClientTest {
 
         fun getSut(optionsCallback: ((SentryOptions) -> Unit)? = null): SentryClient {
             optionsCallback?.invoke(sentryOptions)
+            profilingTraceFile.writeText("sampledProfile")
             return SentryClient(sentryOptions)
         }
     }
@@ -1055,6 +1056,14 @@ class SentryClientTest {
         verifyProfilingTraceInEnvelope(transaction.eventId)
     }
 
+    @Test
+    fun `when captureTransaction with empty trace file, profiling data is not sent`() {
+        val transaction = SentryTransaction(fixture.sentryTracer)
+        fixture.getSut().captureTransaction(transaction, null, null, null, fixture.profilingTraceData)
+        fixture.profilingTraceFile.writeText("")
+        assertFails { verifyProfilingTraceInEnvelope(transaction.eventId) }
+    }
+
     @Test
     fun `when captureTransaction with non existing profiling trace file, profiling trace data is not sent`() {
         val transaction = SentryTransaction(fixture.sentryTracer)