From cfa10f85d3fbc2b25552bfe9ce9c07c57ba8e059 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 5 Sep 2022 16:16:59 +0200 Subject: [PATCH 1/2] 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 profiling sample activity default flag switched --- .../io/sentry/uitest/android/EnvelopeTests.kt | 27 +++++++++++++++++++ .../samples/android/ProfilingActivity.kt | 2 +- .../java/io/sentry/SentryEnvelopeItem.java | 3 +++ .../test/java/io/sentry/SentryClientTest.kt | 9 +++++++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt index 82618499be7..575c47666ab 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt +++ b/sentry-android-integration-tests/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() + // 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/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/ProfilingActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/ProfilingActivity.kt index 74fc79e431a..1731c0966f8 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/ProfilingActivity.kt +++ b/sentry-samples/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 index f75fbf60e64..56fe30ebdf5 100644 --- 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 index 47769b24815..749b7546275 100644 --- 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) From 695ead7e8e3cafeb6aa262e8cdf5bfe660e14f83 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 5 Sep 2022 16:23:34 +0200 Subject: [PATCH 2/2] 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 profiling sample activity default flag switched --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a09ec9c6f74..60a51110739 100644 --- 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