From b99cd601e62eacc1c72d1d107a671a47f2e223f0 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 16 Dec 2022 19:23:43 +0100 Subject: [PATCH 1/5] SentryTracer sends profile inside transaction envelope AndroidTransactionProfiler now profiles only one transaction at a time Removed @Deprecated annotations to methods in SentryClient to capture a profile with a transaction Added methods in Hub to capture a profile with a transaction --- .../core/AndroidTransactionProfiler.java | 137 +++++++++--------- .../core/ActivityLifecycleIntegrationTest.kt | 15 +- .../core/AndroidTransactionProfilerTest.kt | 96 +++--------- .../android/core/SentryAndroidOptionsTest.kt | 3 +- .../io/sentry/uitest/android/EnvelopeTests.kt | 112 ++------------ .../apollo3/SentryApollo3InterceptorTest.kt | 6 + ...ntryApollo3InterceptorWithVariablesTest.kt | 3 + .../apollo/SentryApolloInterceptorTest.kt | 5 + .../tracing/SentryTracingFilterTest.kt | 6 + .../tracing/SentryTransactionAdviceTest.kt | 5 + .../spring/tracing/SentryTracingFilterTest.kt | 6 + .../tracing/SentryTransactionAdviceTest.kt | 5 + sentry/api/sentry.api | 15 +- sentry/src/main/java/io/sentry/Hub.java | 6 +- .../src/main/java/io/sentry/HubAdapter.java | 32 +--- sentry/src/main/java/io/sentry/IHub.java | 21 ++- .../main/java/io/sentry/ISentryClient.java | 21 ++- .../java/io/sentry/ITransactionProfiler.java | 4 +- sentry/src/main/java/io/sentry/NoOpHub.java | 15 +- .../main/java/io/sentry/NoOpSentryClient.java | 3 +- .../io/sentry/NoOpTransactionProfiler.java | 5 +- .../src/main/java/io/sentry/SentryClient.java | 36 +---- .../src/main/java/io/sentry/SentryTracer.java | 5 +- sentry/src/test/java/io/sentry/HubTest.kt | 10 +- .../io/sentry/NoOpTransactionProfilerTest.kt | 6 +- .../test/java/io/sentry/SentryClientTest.kt | 18 --- .../test/java/io/sentry/SentryTracerTest.kt | 22 ++- 27 files changed, 251 insertions(+), 367 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 065c541db9..ccddddf4ad 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -17,11 +17,9 @@ import io.sentry.ITransactionProfiler; import io.sentry.ProfilingTraceData; import io.sentry.ProfilingTransactionData; -import io.sentry.SentryEnvelope; import io.sentry.SentryLevel; import io.sentry.android.core.internal.util.CpuInfoUtils; import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; -import io.sentry.exception.SentryEnvelopeException; import io.sentry.profilemeasurements.ProfileMeasurement; import io.sentry.profilemeasurements.ProfileMeasurementValue; import io.sentry.util.Objects; @@ -56,6 +54,7 @@ final class AndroidTransactionProfiler implements ITransactionProfiler { private @Nullable File traceFile = null; private @Nullable File traceFilesDir = null; private @Nullable Future scheduledFinish = null; + private volatile @Nullable ProfilingTraceData timedOutProfilingData = null; private final @NotNull Context context; private final @NotNull SentryAndroidOptions options; private final @NotNull IHub hub; @@ -160,20 +159,23 @@ private void onTransactionStartSafe(final @NotNull ITransaction transaction) { // When the first transaction is starting, we can start profiling if (transactionsCounter == 1) { onFirstTransactionStarted(transaction); + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Transaction %s (%s) started and being profiled.", + transaction.getName(), + transaction.getSpanContext().getTraceId().toString()); } else { - ProfilingTransactionData transactionData = - new ProfilingTransactionData( - transaction, SystemClock.elapsedRealtimeNanos(), Process.getElapsedCpuTime()); - transactionMap.put(transaction.getEventId().toString(), transactionData); + transactionsCounter--; + options + .getLogger() + .log( + SentryLevel.WARNING, + "A transaction is already being profiled. Transaction %s (%s) will be ignored.", + transaction.getName(), + transaction.getSpanContext().getTraceId().toString()); } - options - .getLogger() - .log( - SentryLevel.DEBUG, - "Transaction %s (%s) started. Transactions being profiled: %d", - transaction.getName(), - transaction.getSpanContext().getTraceId().toString(), - transactionsCounter); } @SuppressLint("NewApi") @@ -229,7 +231,9 @@ public void onFrameMetricCollected( scheduledFinish = options .getExecutorService() - .schedule(() -> onTransactionFinish(transaction, true), PROFILING_TIMEOUT_MILLIS); + .schedule( + () -> timedOutProfilingData = onTransactionFinish(transaction, true), + PROFILING_TIMEOUT_MILLIS); transactionStartNanos = SystemClock.elapsedRealtimeNanos(); profileStartCpuMillis = Process.getElapsedCpuTime(); @@ -242,20 +246,42 @@ public void onFrameMetricCollected( } @Override - public synchronized void onTransactionFinish(final @NotNull ITransaction transaction) { - options.getExecutorService().submit(() -> onTransactionFinish(transaction, false)); + public @Nullable synchronized ProfilingTraceData onTransactionFinish( + final @NotNull ITransaction transaction) { + return onTransactionFinish(transaction, false); } @SuppressLint("NewApi") - private void onTransactionFinish( + private @Nullable ProfilingTraceData onTransactionFinish( final @NotNull ITransaction transaction, final boolean isTimeout) { // onTransactionStart() is only available since Lollipop // and SystemClock.elapsedRealtimeNanos() since Jelly Bean - if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) return; + if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) return null; - // Transaction finished, but it's not in the current profile. We can skip it + final ProfilingTraceData profilingData = timedOutProfilingData; + + // Transaction finished, but it's not in the current profile if (!transactionMap.containsKey(transaction.getEventId().toString())) { + // We check if we cached a profiling data due to a timeout with this profile in it + // If so, we return it back, otherwise we would simply lose it + if (profilingData != null) { + if (profilingData.getTransactionId().equals(transaction.getEventId().toString())) { + timedOutProfilingData = null; + return profilingData; + } else { + // Another transaction is finishing before the timed out one + options + .getLogger() + .log( + SentryLevel.INFO, + "A timed out profiling data exists, but the finishing transaction %s (%s) is not part of it", + transaction.getName(), + transaction.getSpanContext().getTraceId().toString()); + return null; + } + } + // A transaction is finishing, but it's not profiled. We can skip it options .getLogger() .log( @@ -263,7 +289,7 @@ private void onTransactionFinish( "Transaction %s (%s) finished, but was not currently being profiled. Skipping", transaction.getName(), transaction.getSpanContext().getTraceId().toString()); - return; + return null; } if (transactionsCounter > 0) { @@ -274,10 +300,9 @@ private void onTransactionFinish( .getLogger() .log( SentryLevel.DEBUG, - "Transaction %s (%s) finished. Transactions to be profiled: %d", + "Transaction %s (%s) finished.", transaction.getName(), - transaction.getSpanContext().getTraceId().toString(), - transactionsCounter); + transaction.getSpanContext().getTraceId().toString()); if (transactionsCounter != 0 && !isTimeout) { // We notify the data referring to this transaction that it finished @@ -290,13 +315,9 @@ private void onTransactionFinish( Process.getElapsedCpuTime(), profileStartCpuMillis); } - return; + return null; } - onLastTransactionFinished(transaction, isTimeout); - } - @SuppressLint("NewApi") - private void onLastTransactionFinished(final ITransaction transaction, final boolean isTimeout) { Debug.stopMethodTracing(); frameMetricsCollector.stopCollection(frameMetricsCollectorId); @@ -316,7 +337,7 @@ private void onLastTransactionFinished(final ITransaction transaction, final boo if (traceFile == null) { options.getLogger().log(SentryLevel.ERROR, "Trace file does not exists"); - return; + return null; } String totalMem = "0"; @@ -355,42 +376,26 @@ private void onLastTransactionFinished(final ITransaction transaction, final boo // cpu max frequencies are read with a lambda because reading files is involved, so it will be // done in the background when the trace file is read - ProfilingTraceData profilingTraceData = - new ProfilingTraceData( - traceFile, - transactionList, - transaction, - Long.toString(transactionDurationNanos), - buildInfoProvider.getSdkInfoVersion(), - abis != null && abis.length > 0 ? abis[0] : "", - () -> CpuInfoUtils.getInstance().readMaxFrequencies(), - buildInfoProvider.getManufacturer(), - buildInfoProvider.getModel(), - buildInfoProvider.getVersionRelease(), - buildInfoProvider.isEmulator(), - totalMem, - options.getProguardUuid(), - options.getRelease(), - options.getEnvironment(), - isTimeout - ? ProfilingTraceData.TRUNCATION_REASON_TIMEOUT - : ProfilingTraceData.TRUNCATION_REASON_NORMAL, - measurementsMap); - - SentryEnvelope envelope; - try { - envelope = - SentryEnvelope.from( - options.getSerializer(), - profilingTraceData, - options.getMaxTraceFileSize(), - options.getSdkVersion()); - } catch (SentryEnvelopeException e) { - options.getLogger().log(SentryLevel.ERROR, "Failed to capture profile.", e); - return; - } - - hub.captureEnvelope(envelope); + return new ProfilingTraceData( + traceFile, + transactionList, + transaction, + Long.toString(transactionDurationNanos), + buildInfoProvider.getSdkInfoVersion(), + abis != null && abis.length > 0 ? abis[0] : "", + () -> CpuInfoUtils.getInstance().readMaxFrequencies(), + buildInfoProvider.getManufacturer(), + buildInfoProvider.getModel(), + buildInfoProvider.getVersionRelease(), + buildInfoProvider.isEmulator(), + totalMem, + options.getProguardUuid(), + options.getRelease(), + options.getEnvironment(), + isTimeout + ? ProfilingTraceData.TRUNCATION_REASON_TIMEOUT + : ProfilingTraceData.TRUNCATION_REASON_NORMAL, + measurementsMap); } /** diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 89e7399f80..0ad252fc8d 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -378,6 +378,7 @@ class ActivityLifecycleIntegrationTest { assertEquals(SpanStatus.OK, it.status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -397,6 +398,7 @@ class ActivityLifecycleIntegrationTest { assertEquals(SpanStatus.OK, it.status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -420,6 +422,7 @@ class ActivityLifecycleIntegrationTest { assertEquals(SpanStatus.UNKNOWN_ERROR, it.status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -435,7 +438,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, fixture.bundle) sut.onActivityPostResumed(activity) - verify(fixture.hub, never()).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test @@ -446,7 +449,7 @@ class ActivityLifecycleIntegrationTest { val activity = mock() sut.onActivityPostResumed(activity) - verify(fixture.hub, never()).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test @@ -459,7 +462,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, fixture.bundle) sut.onActivityDestroyed(activity) - verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test @@ -561,7 +564,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(mock(), mock()) sut.onActivityCreated(mock(), fixture.bundle) - verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test @@ -574,7 +577,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, mock()) sut.onActivityResumed(activity) - verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull(), anyOrNull()) } @Test @@ -602,7 +605,7 @@ class ActivityLifecycleIntegrationTest { sut.ttidSpanMap.values.first().finish() sut.onActivityResumed(activity) - verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 1a3eec8588..96f32dc8f5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -12,11 +12,9 @@ import io.sentry.SentryLevel import io.sentry.SentryTracer import io.sentry.TransactionContext import io.sentry.android.core.internal.util.SentryFrameMetricsCollector -import io.sentry.assertEnvelopeItem import io.sentry.test.getCtor import org.junit.runner.RunWith import org.mockito.kotlin.any -import org.mockito.kotlin.check import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.spy @@ -32,7 +30,7 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertNotNull -import kotlin.test.assertTrue +import kotlin.test.assertNull @RunWith(AndroidJUnit4::class) class AndroidTransactionProfilerTest { @@ -135,16 +133,10 @@ class AndroidTransactionProfilerTest { fun `profiler profiles current transaction`() { val profiler = fixture.getSut(context) profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionFinish(fixture.transaction1) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub).captureEnvelope( - check { - assertEquals(1, it.items.count()) - assertEnvelopeItem(it.items.toList()) { _, item -> - assertEquals(item.transactionId, fixture.transaction1.eventId.toString()) - } - } - ) + assertNotNull(profilingTraceData) + assertEquals(profilingTraceData.transactionId, fixture.transaction1.eventId.toString()) } @Test @@ -302,84 +294,32 @@ class AndroidTransactionProfilerTest { fixture.lastScheduledRunnable?.run() // First transaction finishes: timed out data is returned - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub).captureEnvelope( - check { - assertEquals(1, it.items.count()) - assertEnvelopeItem(it.items.toList()) { _, item -> - assertEquals(item.transactionId, fixture.transaction1.eventId.toString()) - assertEquals(ProfilingTraceData.TRUNCATION_REASON_TIMEOUT, item.truncationReason) - } - } - ) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) + assertEquals(profilingTraceData!!.transactionId, fixture.transaction1.eventId.toString()) + assertEquals(ProfilingTraceData.TRUNCATION_REASON_TIMEOUT, profilingTraceData.truncationReason) } @Test - fun `profiling stops and returns data only when the last transaction finishes`() { + fun `profiling stops and returns data only when the first transaction finishes`() { val profiler = fixture.getSut(context) profiler.onTransactionStart(fixture.transaction1) profiler.onTransactionStart(fixture.transaction2) - profiler.onTransactionFinish(fixture.transaction2) - verify(fixture.hub, never()).captureEnvelope(any()) + var profilingTraceData = profiler.onTransactionFinish(fixture.transaction2) + assertNull(profilingTraceData) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub).captureEnvelope( - check { - assertEquals(1, it.items.count()) - assertEnvelopeItem(it.items.toList()) { _, item -> - assertEquals(item.transactionId, fixture.transaction1.eventId.toString()) - } - } - ) - } - - @Test - fun `profiling records multiple concurrent transactions`() { - val profiler = fixture.getSut(context) - profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionStart(fixture.transaction2) - - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub, never()).captureEnvelope(any()) - - profiler.onTransactionStart(fixture.transaction3) - profiler.onTransactionFinish(fixture.transaction3) - verify(fixture.hub, never()).captureEnvelope(any()) - - profiler.onTransactionFinish(fixture.transaction2) - verify(fixture.hub).captureEnvelope( - check { - val expectedTransactions = listOf( - fixture.transaction1.eventId.toString(), - fixture.transaction3.eventId.toString(), - fixture.transaction2.eventId.toString() - ) - - assertEquals(1, it.items.count()) - assertEnvelopeItem(it.items.toList()) { _, item -> - assertEquals(item.transactionId, fixture.transaction2.eventId.toString()) - - assertTrue(item.transactions.map { it.id }.containsAll(expectedTransactions)) - assertTrue(expectedTransactions.containsAll(item.transactions.map { it.id })) - } - } - ) + profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) + assertNotNull(profilingTraceData) + assertEquals(profilingTraceData.transactionId, fixture.transaction1.eventId.toString()) } @Test fun `profiling trace data contains release field`() { val profiler = fixture.getSut(context) profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub).captureEnvelope( - check { - assertEnvelopeItem(it.items.toList()) { _, item -> - assertEquals(fixture.options.release, item.release) - assertNotNull(item.release) - } - } - ) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) + assertNotNull(profilingTraceData!!.release) + assertEquals(fixture.options.release, profilingTraceData.release) } fun `profiler starts collecting frame metrics when the first transaction starts`() { @@ -391,15 +331,13 @@ class AndroidTransactionProfilerTest { } @Test - fun `profiler stops collecting frame metrics when the last transaction finishes`() { + fun `profiler stops collecting frame metrics when the first transaction finishes`() { val profiler = fixture.getSut(context) val frameMetricsCollectorId = "id" whenever(fixture.frameMetricsCollector.startCollection(any())).thenReturn(frameMetricsCollectorId) profiler.onTransactionStart(fixture.transaction1) profiler.onTransactionStart(fixture.transaction2) profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.frameMetricsCollector, never()).stopCollection(frameMetricsCollectorId) - profiler.onTransactionFinish(fixture.transaction2) verify(fixture.frameMetricsCollector).stopCollection(frameMetricsCollectorId) } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt index 520f9be700..7c44407ce0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt @@ -3,6 +3,7 @@ package io.sentry.android.core import io.sentry.ITransaction import io.sentry.ITransactionProfiler import io.sentry.NoOpTransactionProfiler +import io.sentry.ProfilingTraceData import io.sentry.protocol.DebugImage import kotlin.test.Test import kotlin.test.assertEquals @@ -109,6 +110,6 @@ class SentryAndroidOptionsTest { private class CustomTransactionProfiler : ITransactionProfiler { override fun onTransactionStart(transaction: ITransaction) {} - override fun onTransactionFinish(transaction: ITransaction) {} + override fun onTransactionFinish(transaction: ITransaction): ProfilingTraceData? = null } } 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 39acc1b26a..062558fda3 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 @@ -20,8 +20,6 @@ import java.io.File import java.util.concurrent.TimeUnit import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFailsWith -import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertTrue @@ -60,7 +58,6 @@ class EnvelopeTests : BaseUiTest() { sampleScenario.moveToState(Lifecycle.State.DESTROYED) IdlingRegistry.getInstance().unregister(ProfilingSampleActivity.scrollingIdlingResource) relayIdlingResource.increment() - relayIdlingResource.increment() transaction.finish() relay.assert { @@ -76,15 +73,10 @@ class EnvelopeTests : BaseUiTest() { assertEnvelopeItem(it.items.toList()).transaction == "profiledTransaction" }.assert { val transactionItem: SentryTransaction = it.assertItem() - it.assertNoOtherItems() - assertEquals("profiledTransaction", transactionItem.transaction) - } - - findEnvelope { - assertEnvelopeItem(it.items.toList()).transactionName == "profiledTransaction" - }.assert { val profilingTraceData: ProfilingTraceData = it.assertItem() it.assertNoOtherItems() + + assertEquals("profiledTransaction", transactionItem.transaction) assertEquals(profilingTraceData.transactionId, transaction.eventId.toString()) assertEquals("profiledTransaction", profilingTraceData.transactionName) assertTrue(profilingTraceData.environment.isNotEmpty()) @@ -117,87 +109,6 @@ class EnvelopeTests : BaseUiTest() { } } - @Test - fun checkEnvelopeConcurrentTransactions() { - initSentry(true) { options: SentryAndroidOptions -> - options.tracesSampleRate = 1.0 - options.profilesSampleRate = 1.0 - } - relayIdlingResource.increment() - relayIdlingResource.increment() - relayIdlingResource.increment() - relayIdlingResource.increment() - - val transaction = Sentry.startTransaction("e2etests", "test1") - val transaction2 = Sentry.startTransaction("e2etests1", "test2") - val transaction3 = Sentry.startTransaction("e2etests2", "test3") - transaction.finish() - transaction2.finish() - transaction3.finish() - - relay.assert { - findEnvelope { - assertEnvelopeItem(it.items.toList()).transaction == "e2etests" - }.assert { - val transactionItem: SentryTransaction = it.assertItem() - it.assertNoOtherItems() - assertEquals(transaction.eventId.toString(), transactionItem.eventId.toString()) - } - findEnvelope { - assertEnvelopeItem(it.items.toList()).transaction == "e2etests1" - }.assert { - val transactionItem: SentryTransaction = it.assertItem() - it.assertNoOtherItems() - assertEquals(transaction2.eventId.toString(), transactionItem.eventId.toString()) - } - findEnvelope { - assertEnvelopeItem(it.items.toList()).transaction == "e2etests2" - }.assert { - val transactionItem: SentryTransaction = it.assertItem() - it.assertNoOtherItems() - assertEquals(transaction3.eventId.toString(), transactionItem.eventId.toString()) - } - // The profile is sent in its own envelope - findEnvelope { - assertEnvelopeItem(it.items.toList()).transactionName == "e2etests2" - }.assert { - val profilingTraceData: ProfilingTraceData = it.assertItem() - it.assertNoOtherItems() - assertEquals("e2etests2", profilingTraceData.transactionName) - assertEquals("normal", profilingTraceData.truncationReason) - - // The transaction list is not ordered, since it's stored using a map to be able to quickly check the - // existence of a certain id. So we order the list to make more meaningful checks on timestamps. - val transactions = profilingTraceData.transactions.sortedBy { it.relativeStartNs } - assertEquals(transactions.last().id, transaction3.eventId.toString()) - val startTimes = transactions.map { t -> t.relativeStartNs } - val endTimes = transactions.mapNotNull { t -> t.relativeEndNs } - val startCpuTimes = transactions.map { t -> t.relativeStartCpuMs } - val endCpuTimes = transactions.mapNotNull { t -> t.relativeEndCpuMs } - - // Transaction timestamps should be all different from each other - assertTrue(startTimes[0] < startTimes[1]) - assertTrue(startTimes[1] < startTimes[2]) - assertTrue(endTimes[0] < endTimes[1]) - assertTrue(endTimes[1] < endTimes[2]) - - // The cpu timestamps use milliseconds precision, so few of them could have the same timestamps - // However, it's basically impossible that all of them have the same timestamps - assertFalse(startCpuTimes[0] == startCpuTimes[1] && startCpuTimes[1] == startCpuTimes[2]) - assertTrue(startCpuTimes[0] <= startCpuTimes[1]) - assertTrue(startCpuTimes[1] <= startCpuTimes[2]) - assertFalse(endCpuTimes[0] == endCpuTimes[1] && endCpuTimes[1] == endCpuTimes[2]) - assertTrue(endCpuTimes[0] <= endCpuTimes[1]) - assertTrue(endCpuTimes[1] <= endCpuTimes[2]) - - // The first and last transactions should be aligned to the start/stop of profile - assertEquals(endTimes.last() - startTimes.first(), profilingTraceData.durationNs.toLong()) - } - assertNoOtherEnvelopes() - assertNoOtherRequests() - } - } - @Test fun checkProfileNotSentIfEmpty() { initSentry(true) { options: SentryAndroidOptions -> @@ -205,7 +116,7 @@ class EnvelopeTests : BaseUiTest() { options.profilesSampleRate = 1.0 } relayIdlingResource.increment() - relayIdlingResource.increment() + // relayIdlingResource.increment() val profilesDirPath = Sentry.getCurrentHub().options.profilingTracesDirPath val transaction = Sentry.startTransaction("emptyProfileTransaction", "test empty") @@ -224,6 +135,7 @@ class EnvelopeTests : BaseUiTest() { finished = true } + Thread.sleep(5000) relay.assert { findEnvelope { assertEnvelopeItem(it.items.toList()).transaction == "emptyProfileTransaction" @@ -233,7 +145,7 @@ class EnvelopeTests : BaseUiTest() { assertEquals("emptyProfileTransaction", transactionItem.transaction) } // The profile failed to be sent. Trying to read the envelope from the data transmitted throws an exception - assertFailsWith { assertFirstEnvelope {} } + // assertFailsWith { assertFirstEnvelope {} } assertNoOtherEnvelopes() assertNoOtherRequests() } @@ -248,16 +160,24 @@ class EnvelopeTests : BaseUiTest() { options.profilesSampleRate = 1.0 } relayIdlingResource.increment() - Sentry.startTransaction("timedOutProfile", "testTimeout") - // We don't call transaction.finish() and let the timeout do its job + val transaction = Sentry.startTransaction("timedOutProfile", "testTimeout") + Thread { + Thread.sleep(35_000) + transaction.finish() + }.start() + // We call transaction.finish() after 35 seconds, and check profile times out after 30 seconds relay.assert { findEnvelope { - assertEnvelopeItem(it.items.toList()).transactionName == "timedOutProfile" + assertEnvelopeItem(it.items.toList()).transaction == "timedOutProfile" }.assert { + val transactionItem: SentryTransaction = it.assertItem() val profilingTraceData: ProfilingTraceData = it.assertItem() it.assertNoOtherItems() + assertEquals("timedOutProfile", transactionItem.transaction) assertEquals("timedOutProfile", profilingTraceData.transactionName) + // The profile should timeout after 30 seconds + assertTrue(profilingTraceData.durationNs.toLong() < TimeUnit.SECONDS.toNanos(31)) assertEquals(ProfilingTraceData.TRUNCATION_REASON_TIMEOUT, profilingTraceData.truncationReason) } assertNoOtherEnvelopes() diff --git a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt index 2cd4e57a05..b5cd806299 100644 --- a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt +++ b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt @@ -102,6 +102,7 @@ class SentryApollo3InterceptorTest { assertEquals(SpanStatus.OK, it.spans.first().status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -116,6 +117,7 @@ class SentryApollo3InterceptorTest { assertEquals(SpanStatus.PERMISSION_DENIED, it.spans.first().status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -130,6 +132,7 @@ class SentryApollo3InterceptorTest { assertEquals(SpanStatus.INTERNAL_ERROR, it.spans.first().status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -185,6 +188,7 @@ class SentryApollo3InterceptorTest { assertEquals("overwritten description", httpClientSpan.description) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -202,6 +206,7 @@ class SentryApollo3InterceptorTest { assertEquals(0, it.spans.size) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -221,6 +226,7 @@ class SentryApollo3InterceptorTest { assertEquals(1, it.spans.size) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } diff --git a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorWithVariablesTest.kt b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorWithVariablesTest.kt index df0ce7f348..3de27196b6 100644 --- a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorWithVariablesTest.kt +++ b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorWithVariablesTest.kt @@ -85,6 +85,7 @@ class SentryApollo3InterceptorWithVariablesTest { assertEquals(SpanStatus.OK, it.spans.first().status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -99,6 +100,7 @@ class SentryApollo3InterceptorWithVariablesTest { assertEquals(SpanStatus.PERMISSION_DENIED, it.spans.first().status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -113,6 +115,7 @@ class SentryApollo3InterceptorWithVariablesTest { assertEquals(SpanStatus.INTERNAL_ERROR, it.spans.first().status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } diff --git a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt index 2fe4755ead..cd80f5a871 100644 --- a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt +++ b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt @@ -91,6 +91,7 @@ class SentryApolloInterceptorTest { assertEquals(SpanStatus.OK, it.spans.first().status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -105,6 +106,7 @@ class SentryApolloInterceptorTest { assertEquals(SpanStatus.PERMISSION_DENIED, it.spans.first().status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -119,6 +121,7 @@ class SentryApolloInterceptorTest { assertEquals(SpanStatus.INTERNAL_ERROR, it.spans.first().status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -154,6 +157,7 @@ class SentryApolloInterceptorTest { assertEquals("overwritten description", httpClientSpan.description) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -169,6 +173,7 @@ class SentryApolloInterceptorTest { assertEquals(1, it.spans.size) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt index 5578bafe44..63f0ca2dea 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt @@ -91,6 +91,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.operation).isEqualTo("http.server") }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -106,6 +107,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -121,6 +123,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.status).isNull() }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -136,6 +139,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.parentSpanId).isNull() }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -152,6 +156,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.parentSpanId).isEqualTo(parentSpanId) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -184,6 +189,7 @@ class SentryTracingFilterTest { assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTransactionAdviceTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTransactionAdviceTest.kt index 16c8bb3618..cfe3ab5e47 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTransactionAdviceTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTransactionAdviceTest.kt @@ -66,6 +66,7 @@ class SentryTransactionAdviceTest { assertThat(it.status).isEqualTo(SpanStatus.OK) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -78,6 +79,7 @@ class SentryTransactionAdviceTest { assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -91,6 +93,7 @@ class SentryTransactionAdviceTest { assertThat(it.contexts.trace!!.operation).isEqualTo("op") }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -114,6 +117,7 @@ class SentryTransactionAdviceTest { assertThat(it.contexts.trace!!.operation).isEqualTo("op") }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -127,6 +131,7 @@ class SentryTransactionAdviceTest { assertThat(it.contexts.trace!!.operation).isEqualTo("my-op") }, anyOrNull(), + anyOrNull(), anyOrNull() ) } diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt index e02e04cb86..e64959158f 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt @@ -91,6 +91,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.operation).isEqualTo("http.server") }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -106,6 +107,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -121,6 +123,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.status).isNull() }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -136,6 +139,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.parentSpanId).isNull() }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -152,6 +156,7 @@ class SentryTracingFilterTest { assertThat(it.contexts.trace!!.parentSpanId).isEqualTo(parentSpanId) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -184,6 +189,7 @@ class SentryTracingFilterTest { assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTransactionAdviceTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTransactionAdviceTest.kt index c154f5aaed..ae9ae28816 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTransactionAdviceTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTransactionAdviceTest.kt @@ -66,6 +66,7 @@ class SentryTransactionAdviceTest { assertThat(it.status).isEqualTo(SpanStatus.OK) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -78,6 +79,7 @@ class SentryTransactionAdviceTest { assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -91,6 +93,7 @@ class SentryTransactionAdviceTest { assertThat(it.contexts.trace!!.operation).isEqualTo("op") }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -114,6 +117,7 @@ class SentryTransactionAdviceTest { assertThat(it.contexts.trace!!.operation).isEqualTo("op") }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -127,6 +131,7 @@ class SentryTransactionAdviceTest { assertThat(it.contexts.trace!!.operation).isEqualTo("my-op") }, anyOrNull(), + anyOrNull(), anyOrNull() ) } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index db6c9eeb6f..6f1f3836f1 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -289,7 +289,7 @@ public final class io/sentry/Hub : io/sentry/IHub { public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; - public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Hint;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public fun captureUserFeedback (Lio/sentry/UserFeedback;)V public fun clearBreadcrumbs ()V public fun clone ()Lio/sentry/IHub; @@ -331,7 +331,6 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; - public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Hint;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public fun captureUserFeedback (Lio/sentry/UserFeedback;)V public fun clearBreadcrumbs ()V @@ -396,7 +395,8 @@ public abstract interface class io/sentry/IHub { public abstract fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;)Lio/sentry/protocol/SentryId; - public abstract fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public abstract fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Hint;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public abstract fun captureUserFeedback (Lio/sentry/UserFeedback;)V public abstract fun clearBreadcrumbs ()V public abstract fun clone ()Lio/sentry/IHub; @@ -468,7 +468,8 @@ public abstract interface class io/sentry/ISentryClient { public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/Scope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;)Lio/sentry/protocol/SentryId; - public abstract fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Scope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Scope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public abstract fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Scope;Lio/sentry/Hint;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public abstract fun captureUserFeedback (Lio/sentry/UserFeedback;)V public abstract fun close ()V public abstract fun flush (J)V @@ -535,7 +536,7 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan { } public abstract interface class io/sentry/ITransactionProfiler { - public abstract fun onTransactionFinish (Lio/sentry/ITransaction;)V + public abstract fun onTransactionFinish (Lio/sentry/ITransaction;)Lio/sentry/ProfilingTraceData; public abstract fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -694,7 +695,6 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; - public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Hint;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public fun captureUserFeedback (Lio/sentry/UserFeedback;)V public fun clearBreadcrumbs ()V @@ -812,7 +812,7 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { public final class io/sentry/NoOpTransactionProfiler : io/sentry/ITransactionProfiler { public static fun getInstance ()Lio/sentry/NoOpTransactionProfiler; - public fun onTransactionFinish (Lio/sentry/ITransaction;)V + public fun onTransactionFinish (Lio/sentry/ITransaction;)Lio/sentry/ProfilingTraceData; public fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -1196,7 +1196,6 @@ public final class io/sentry/SentryClient : io/sentry/ISentryClient { public fun captureEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Scope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureSession (Lio/sentry/Session;Lio/sentry/Hint;)V - public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Scope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureTransaction (Lio/sentry/protocol/SentryTransaction;Lio/sentry/TraceContext;Lio/sentry/Scope;Lio/sentry/Hint;Lio/sentry/ProfilingTraceData;)Lio/sentry/protocol/SentryId; public fun captureUserFeedback (Lio/sentry/UserFeedback;)V public fun close ()V diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 172b81dcad..e93b39845d 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -605,7 +605,8 @@ public void flush(long timeoutMillis) { public @NotNull SentryId captureTransaction( final @NotNull SentryTransaction transaction, final @Nullable TraceContext traceContext, - final @Nullable Hint hint) { + final @Nullable Hint hint, + final @Nullable ProfilingTraceData profilingTraceData) { Objects.requireNonNull(transaction, "transaction is required"); SentryId sentryId = SentryId.EMPTY_ID; @@ -640,7 +641,8 @@ public void flush(long timeoutMillis) { item = stack.peek(); sentryId = item.getClient() - .captureTransaction(transaction, traceContext, item.getScope(), hint); + .captureTransaction( + transaction, traceContext, item.getScope(), hint, profilingTraceData); } catch (Throwable e) { options .getLogger() diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index 746d9c0b71..0a7756863a 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -1,6 +1,5 @@ package io.sentry; -import io.sentry.exception.SentryEnvelopeException; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; @@ -173,39 +172,14 @@ public void flush(long timeoutMillis) { return Sentry.getCurrentHub().clone(); } - /** - * @deprecated please use {{@link Hub#captureTransaction(SentryTransaction, TraceContext, Hint)}} - * and {{@link Hub#captureEnvelope(SentryEnvelope)}} instead. - */ - @Deprecated + @Override public @NotNull SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceContext traceContext, @Nullable Hint hint, @Nullable ProfilingTraceData profilingTraceData) { - if (profilingTraceData != null) { - SentryEnvelope envelope; - try { - envelope = - SentryEnvelope.from( - getOptions().getSerializer(), - profilingTraceData, - getOptions().getMaxTraceFileSize(), - getOptions().getSdkVersion()); - captureEnvelope(envelope); - } catch (SentryEnvelopeException e) { - getOptions().getLogger().log(SentryLevel.ERROR, "Failed to capture profile.", e); - } - } - return captureTransaction(transaction, traceContext, hint); - } - - @Override - public @NotNull SentryId captureTransaction( - @NotNull SentryTransaction transaction, - @Nullable TraceContext traceContext, - @Nullable Hint hint) { - return Sentry.getCurrentHub().captureTransaction(transaction, traceContext, hint); + return Sentry.getCurrentHub() + .captureTransaction(transaction, traceContext, hint, profilingTraceData); } @Override diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index 3b0779826c..01d8546d84 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -347,6 +347,7 @@ default void addBreadcrumb(@NotNull String message, @NotNull String category) { * @param transaction the transaction * @param traceContext the trace context * @param hint the hints + * @param profilingTraceData the profiling trace data * @return transaction's id */ @ApiStatus.Internal @@ -354,7 +355,25 @@ default void addBreadcrumb(@NotNull String message, @NotNull String category) { SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceContext traceContext, - @Nullable Hint hint); + @Nullable Hint hint, + @Nullable ProfilingTraceData profilingTraceData); + + /** + * Captures the transaction and enqueues it for sending to Sentry server. + * + * @param transaction the transaction + * @param traceContext the trace context + * @param hint the hints + * @return transaction's id + */ + @ApiStatus.Internal + @NotNull + default SentryId captureTransaction( + @NotNull SentryTransaction transaction, + @Nullable TraceContext traceContext, + @Nullable Hint hint) { + return captureTransaction(transaction, traceContext, hint, null); + } @ApiStatus.Internal @NotNull diff --git a/sentry/src/main/java/io/sentry/ISentryClient.java b/sentry/src/main/java/io/sentry/ISentryClient.java index bac9a7b3c1..4789eb3dec 100644 --- a/sentry/src/main/java/io/sentry/ISentryClient.java +++ b/sentry/src/main/java/io/sentry/ISentryClient.java @@ -206,6 +206,23 @@ default SentryId captureTransaction( return captureTransaction(transaction, null, scope, hint); } + /** + * Captures a transaction. + * + * @param transaction the {@link ITransaction} to send + * @param scope An optional scope to be applied to the event. + * @param hint SDK specific but provides high level information about the origin of the event + * @return The Id (SentryId object) of the event + */ + @NotNull + default SentryId captureTransaction( + @NotNull SentryTransaction transaction, + @Nullable TraceContext traceContext, + @Nullable Scope scope, + @Nullable Hint hint) { + return captureTransaction(transaction, traceContext, scope, hint, null); + } + /** * Captures a transaction. * @@ -213,6 +230,7 @@ default SentryId captureTransaction( * @param traceContext the trace context * @param scope An optional scope to be applied to the event. * @param hint SDK specific but provides high level information about the origin of the event + * @param profilingTraceData An optional profiling trace data captured during the transaction * @return The Id (SentryId object) of the event */ @NotNull @@ -221,7 +239,8 @@ SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceContext traceContext, @Nullable Scope scope, - @Nullable Hint hint); + @Nullable Hint hint, + @Nullable ProfilingTraceData profilingTraceData); /** * Captures a transaction without scope nor hint. diff --git a/sentry/src/main/java/io/sentry/ITransactionProfiler.java b/sentry/src/main/java/io/sentry/ITransactionProfiler.java index 6bfc435b36..6e6e9cf582 100644 --- a/sentry/src/main/java/io/sentry/ITransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/ITransactionProfiler.java @@ -2,11 +2,13 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** Used for performing operations when a transaction is started or ended. */ @ApiStatus.Internal public interface ITransactionProfiler { void onTransactionStart(@NotNull ITransaction transaction); - void onTransactionFinish(@NotNull ITransaction transaction); + @Nullable + ProfilingTraceData onTransactionFinish(@NotNull ITransaction transaction); } diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index d94a1236ac..52bef0ff43 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -132,12 +132,7 @@ public void flush(long timeoutMillis) {} return instance; } - /** - * @deprecated please use {{@link Hub#captureTransaction(SentryTransaction, TraceContext, Hint)}} - * and {{@link Hub#captureEnvelope(SentryEnvelope)}} instead. - */ - @Deprecated - @SuppressWarnings("InlineMeSuggester") + @Override public @NotNull SentryId captureTransaction( final @NotNull SentryTransaction transaction, final @Nullable TraceContext traceContext, @@ -146,14 +141,6 @@ public void flush(long timeoutMillis) {} return SentryId.EMPTY_ID; } - @Override - public @NotNull SentryId captureTransaction( - final @NotNull SentryTransaction transaction, - final @Nullable TraceContext traceContext, - final @Nullable Hint hint) { - return SentryId.EMPTY_ID; - } - @Override public @NotNull ITransaction startTransaction(@NotNull TransactionContext transactionContexts) { return NoOpTransaction.getInstance(); diff --git a/sentry/src/main/java/io/sentry/NoOpSentryClient.java b/sentry/src/main/java/io/sentry/NoOpSentryClient.java index 88bf38d017..4afbdbb8c8 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryClient.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryClient.java @@ -48,7 +48,8 @@ public SentryId captureEnvelope(@NotNull SentryEnvelope envelope, @Nullable Hint @NotNull SentryTransaction transaction, @Nullable TraceContext traceContext, @Nullable Scope scope, - @Nullable Hint hint) { + @Nullable Hint hint, + @Nullable ProfilingTraceData profilingTraceData) { return SentryId.EMPTY_ID; } } diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java index d4fd1ff34c..2580ded221 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java @@ -1,6 +1,7 @@ package io.sentry; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; public final class NoOpTransactionProfiler implements ITransactionProfiler { @@ -16,5 +17,7 @@ public static NoOpTransactionProfiler getInstance() { public void onTransactionStart(@NotNull ITransaction transaction) {} @Override - public void onTransactionFinish(@NotNull ITransaction transaction) {} + public @Nullable ProfilingTraceData onTransactionFinish(@NotNull ITransaction transaction) { + return null; + } } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 7fa67ae63e..a6feae65e7 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -498,41 +498,13 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint } } - /** - * @deprecated please use {{@link ISentryClient#captureTransaction(SentryTransaction, - * TraceContext, Scope, Hint)}} and {{@link ISentryClient#captureEnvelope(SentryEnvelope)}} - * instead. - */ - @Deprecated + @Override public @NotNull SentryId captureTransaction( @NotNull SentryTransaction transaction, @Nullable TraceContext traceContext, final @Nullable Scope scope, @Nullable Hint hint, final @Nullable ProfilingTraceData profilingTraceData) { - if (profilingTraceData != null) { - SentryEnvelope envelope; - try { - envelope = - SentryEnvelope.from( - options.getSerializer(), - profilingTraceData, - options.getMaxTraceFileSize(), - options.getSdkVersion()); - captureEnvelope(envelope); - } catch (SentryEnvelopeException e) { - options.getLogger().log(SentryLevel.ERROR, "Failed to capture profile.", e); - } - } - return captureTransaction(transaction, traceContext, scope, hint); - } - - @Override - public @NotNull SentryId captureTransaction( - @NotNull SentryTransaction transaction, - @Nullable TraceContext traceContext, - final @Nullable Scope scope, - @Nullable Hint hint) { Objects.requireNonNull(transaction, "Transaction is required."); if (hint == null) { @@ -588,7 +560,11 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint try { final SentryEnvelope envelope = buildEnvelope( - transaction, filterForTransaction(getAttachments(hint)), null, traceContext, null); + transaction, + filterForTransaction(getAttachments(hint)), + null, + traceContext, + profilingTraceData); hint.clear(); if (envelope != null) { diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 451dd5a5a1..8deea72d2f 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -327,8 +327,9 @@ public void finish(@Nullable SpanStatus status) { public void finish(@Nullable SpanStatus status, @Nullable Date finishDate) { this.finishStatus = FinishStatus.finishing(status); if (!root.isFinished() && (!waitForChildren || hasAllChildrenFinished())) { + ProfilingTraceData profilingTraceData = null; if (Boolean.TRUE.equals(isSampled()) && Boolean.TRUE.equals(isProfileSampled())) { - hub.getOptions().getTransactionProfiler().onTransactionFinish(this); + profilingTraceData = hub.getOptions().getTransactionProfiler().onTransactionFinish(this); } // try to get the high precision timestamp from the root span @@ -396,7 +397,7 @@ public void finish(@Nullable SpanStatus status, @Nullable Date finishDate) { } transaction.getMeasurements().putAll(measurements); - hub.captureTransaction(transaction, traceContext(), null); + hub.captureTransaction(transaction, traceContext(), null, profilingTraceData); } } diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index dfeccb12c0..74751371b8 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -1315,7 +1315,7 @@ class HubTest { sentryTracer.finish() sut.captureTransaction(SentryTransaction(sentryTracer), null as TraceContext?) verify(mockClient, never()).captureTransaction(any(), any(), any()) - verify(mockClient, never()).captureTransaction(any(), any(), any(), anyOrNull()) + verify(mockClient, never()).captureTransaction(any(), any(), any(), anyOrNull(), anyOrNull()) } @Test @@ -1331,7 +1331,7 @@ class HubTest { val sentryTracer = SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(true)), sut) sentryTracer.finish() val traceContext = sentryTracer.traceContext() - verify(mockClient).captureTransaction(any(), equalTraceContext(traceContext), any(), eq(null)) + verify(mockClient).captureTransaction(any(), equalTraceContext(traceContext), any(), eq(null), eq(null)) } @Test @@ -1343,7 +1343,7 @@ class HubTest { val sut = Hub(options) val mockClient = mock() sut.bindClient(mockClient) - whenever(mockClient.captureTransaction(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull())).thenReturn(SentryId()) + whenever(mockClient.captureTransaction(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull())).thenReturn(SentryId()) val sentryTracer = SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(true)), sut) sentryTracer.finish() @@ -1362,7 +1362,7 @@ class HubTest { val sentryTracer = SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(true)), sut) sut.captureTransaction(SentryTransaction(sentryTracer), null as TraceContext?) - verify(mockClient, never()).captureTransaction(any(), any(), any(), eq(null)) + verify(mockClient, never()).captureTransaction(any(), any(), any(), eq(null), anyOrNull()) } @Test @@ -1378,7 +1378,7 @@ class HubTest { val sentryTracer = SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(false)), sut) sentryTracer.finish() val traceContext = sentryTracer.traceContext() - verify(mockClient, never()).captureTransaction(any(), equalTraceContext(traceContext), any(), eq(null)) + verify(mockClient, never()).captureTransaction(any(), equalTraceContext(traceContext), any(), eq(null), anyOrNull()) } @Test diff --git a/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt b/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt index aaf30190ed..f09e24f106 100644 --- a/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt @@ -1,6 +1,7 @@ package io.sentry import kotlin.test.Test +import kotlin.test.assertNull class NoOpTransactionProfilerTest { private var profiler = NoOpTransactionProfiler.getInstance() @@ -10,6 +11,7 @@ class NoOpTransactionProfilerTest { profiler.onTransactionStart(NoOpTransaction.getInstance()) @Test - fun `onTransactionFinish does not throw`() = - profiler.onTransactionFinish(NoOpTransaction.getInstance()) + fun `onTransactionFinish does returns null`() { + assertNull(profiler.onTransactionFinish(NoOpTransaction.getInstance())) + } } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 72b3a8210c..e1f5a88f1b 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -30,7 +30,6 @@ import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.mockingDetails import org.mockito.kotlin.never -import org.mockito.kotlin.spy import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions @@ -1137,23 +1136,6 @@ class SentryClientTest { verifyProfilingTraceInEnvelope(SentryId(fixture.profilingTraceData.profileId)) } - @Test - fun `captureTransaction with profile calls captureEnvelope and captureTransaction`() { - val transaction = SentryTransaction(fixture.sentryTracer) - val client = spy(fixture.getSut()) - client.captureTransaction(transaction, null, null, null, fixture.profilingTraceData) - verify(client).captureTransaction(transaction, null, null, null) - verify(client).captureEnvelope( - check { - assertEquals(1, it.items.count()) - assertEnvelopeItem(it.items.toList()) { _, item -> - assertEquals(item.profileId, fixture.profilingTraceData.profileId) - } - }, - anyOrNull() - ) - } - @Test fun `when capture profile with empty trace file, profile is not sent`() { val client = fixture.getSut() diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index fed5bcadc6..efffc30c7a 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -139,6 +139,7 @@ class SentryTracerTest { assertEquals(it.transaction, tracer.name) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -197,6 +198,7 @@ class SentryTracerTest { } }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -214,6 +216,7 @@ class SentryTracerTest { assertEquals("op1", it.spans.first().op) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -239,6 +242,7 @@ class SentryTracerTest { assertEquals(otelContext, it.contexts["otel"]) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -371,6 +375,7 @@ class SentryTracerTest { } }, anyOrNull(), + anyOrNull(), anyOrNull() ) @@ -448,7 +453,7 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true) transaction.startChild("op") transaction.finish() - verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull(), anyOrNull()) } @Test @@ -457,7 +462,7 @@ class SentryTracerTest { val child = transaction.startChild("op") child.finish() transaction.finish() - verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull()) + verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test @@ -478,7 +483,7 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true) val child = transaction.startChild("op") child.finish() - verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull(), anyOrNull()) } @Test @@ -486,13 +491,14 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true) val child = transaction.startChild("op") transaction.finish(SpanStatus.INVALID_ARGUMENT) - verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull(), anyOrNull()) child.finish() verify(fixture.hub, times(1)).captureTransaction( check { assertEquals(SpanStatus.INVALID_ARGUMENT, it.status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -512,6 +518,7 @@ class SentryTracerTest { assertEquals(SpanStatus.DEADLINE_EXCEEDED, it.spans[1].status) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -687,6 +694,7 @@ class SentryTracerTest { assertEquals("val", it.getExtra("key")) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -705,6 +713,7 @@ class SentryTracerTest { } }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -730,6 +739,7 @@ class SentryTracerTest { await.untilFalse(transaction.isFinishTimerRunning) verify(fixture.hub, never()).captureTransaction( + anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull() @@ -746,6 +756,7 @@ class SentryTracerTest { await.untilFalse(transaction.isFinishTimerRunning) verify(fixture.hub).captureTransaction( + anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull() @@ -809,6 +820,7 @@ class SentryTracerTest { assertEquals(transaction.root.endNanos, span2.endNanos) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -849,6 +861,7 @@ class SentryTracerTest { assertEquals("day", it.measurements["days"]!!.unit) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } @@ -866,6 +879,7 @@ class SentryTracerTest { assertEquals("day", it.measurements["metric1"]!!.unit) }, anyOrNull(), + anyOrNull(), anyOrNull() ) } From 9fdeddd7f3ca0a0cab8b43f2c92ded62852556ae Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 16 Dec 2022 22:00:39 +0100 Subject: [PATCH 2/5] updated changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96be8c6563..718cb82df6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +### Features + +- Disable Android concurrent profiling ([#2434](https://github.com/getsentry/sentry-java/pull/2434)) + + ## 6.10.0 ### Features From eaf74c665ecbcc58c9324f94b5864b4449e59b2f Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 20 Dec 2022 13:46:05 +0100 Subject: [PATCH 3/5] removed transaction map from profiler, as now only one transaction at a time can be profiled AndroidTransactionProfiler.onTransactionFinish now waits for executor service thread to finish and returns its data instead of finishing profile in another thread added ISentryExecutorService method Future submit(Callable) --- .../core/AndroidTransactionProfiler.java | 32 ++++++++----- .../core/AndroidTransactionProfilerTest.kt | 46 +++++++++++++------ .../io/sentry/uitest/android/EnvelopeTests.kt | 4 -- sentry/api/sentry.api | 1 + .../io/sentry/ISentryExecutorService.java | 10 ++++ .../io/sentry/NoOpSentryExecutorService.java | 6 +++ .../java/io/sentry/SentryExecutorService.java | 6 +++ 7 files changed, 75 insertions(+), 30 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index ccddddf4ad..93b6f644e1 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.NotNull; @@ -65,7 +66,7 @@ final class AndroidTransactionProfiler implements ITransactionProfiler { private int transactionsCounter = 0; private @Nullable String frameMetricsCollectorId; private final @NotNull SentryFrameMetricsCollector frameMetricsCollector; - private final @NotNull Map transactionMap = new HashMap<>(); + private @Nullable ProfilingTransactionData currentTransaction; private final @NotNull ArrayDeque screenFrameRateMeasurements = new ArrayDeque<>(); private final @NotNull ArrayDeque slowFrameRenderMeasurements = @@ -238,9 +239,8 @@ public void onFrameMetricCollected( transactionStartNanos = SystemClock.elapsedRealtimeNanos(); profileStartCpuMillis = Process.getElapsedCpuTime(); - ProfilingTransactionData transactionData = + currentTransaction = new ProfilingTransactionData(transaction, transactionStartNanos, profileStartCpuMillis); - transactionMap.put(transaction.getEventId().toString(), transactionData); Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs); } @@ -248,7 +248,17 @@ public void onFrameMetricCollected( @Override public @Nullable synchronized ProfilingTraceData onTransactionFinish( final @NotNull ITransaction transaction) { - return onTransactionFinish(transaction, false); + try { + return options + .getExecutorService() + .submit(() -> onTransactionFinish(transaction, false)) + .get(); + } catch (ExecutionException e) { + e.printStackTrace(); + } catch (InterruptedException e) { + e.printStackTrace(); + } + return null; } @SuppressLint("NewApi") @@ -262,7 +272,8 @@ public void onFrameMetricCollected( final ProfilingTraceData profilingData = timedOutProfilingData; // Transaction finished, but it's not in the current profile - if (!transactionMap.containsKey(transaction.getEventId().toString())) { + if (currentTransaction == null + || !currentTransaction.getId().equals(transaction.getEventId().toString())) { // We check if we cached a profiling data due to a timeout with this profile in it // If so, we return it back, otherwise we would simply lose it if (profilingData != null) { @@ -306,10 +317,8 @@ public void onFrameMetricCollected( if (transactionsCounter != 0 && !isTimeout) { // We notify the data referring to this transaction that it finished - ProfilingTransactionData transactionData = - transactionMap.get(transaction.getEventId().toString()); - if (transactionData != null) { - transactionData.notifyFinish( + if (currentTransaction != null) { + currentTransaction.notifyFinish( SystemClock.elapsedRealtimeNanos(), transactionStartNanos, Process.getElapsedCpuTime(), @@ -325,8 +334,9 @@ public void onFrameMetricCollected( long transactionEndCpuMillis = Process.getElapsedCpuTime(); long transactionDurationNanos = transactionEndNanos - transactionStartNanos; - List transactionList = new ArrayList<>(transactionMap.values()); - transactionMap.clear(); + List transactionList = new ArrayList<>(1); + transactionList.add(currentTransaction); + currentTransaction = null; // We clear the counter in case of a timeout transactionsCounter = 0; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 96f32dc8f5..05b5be98b5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -22,6 +22,7 @@ import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import java.io.File +import java.util.concurrent.Callable import java.util.concurrent.Future import java.util.concurrent.FutureTask import kotlin.test.AfterTest @@ -52,6 +53,17 @@ class AndroidTransactionProfilerTest { runnable.run() return FutureTask {} } + override fun submit(callable: Callable): Future { + val futureTask = mock>() + whenever(futureTask.get()).thenAnswer { + return@thenAnswer try { + callable.call() + } catch (e: Exception) { + null + } + } + return futureTask + } override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { lastScheduledRunnable = runnable return FutureTask {} @@ -146,8 +158,8 @@ class AndroidTransactionProfilerTest { } val profiler = fixture.getSut(context, buildInfo) profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub, never()).captureEnvelope(any()) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) + assertNull(profilingTraceData) } @Test @@ -157,8 +169,8 @@ class AndroidTransactionProfilerTest { } val profiler = fixture.getSut(context) profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub, never()).captureEnvelope(any()) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) + assertNull(profilingTraceData) } @Test @@ -230,8 +242,8 @@ class AndroidTransactionProfilerTest { } val profiler = fixture.getSut(context) profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub, never()).captureEnvelope(any()) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) + assertNull(profilingTraceData) } @Test @@ -241,8 +253,8 @@ class AndroidTransactionProfilerTest { } val profiler = fixture.getSut(context) profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub, never()).captureEnvelope(any()) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) + assertNull(profilingTraceData) } @Test @@ -252,8 +264,8 @@ class AndroidTransactionProfilerTest { } val profiler = fixture.getSut(context) profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub, never()).captureEnvelope(any()) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) + assertNull(profilingTraceData) } @Test @@ -270,17 +282,21 @@ class AndroidTransactionProfilerTest { @Test fun `profiler uses background threads`() { val profiler = fixture.getSut(context) - fixture.options.executorService = mock() + val mockExecutorService: ISentryExecutorService = mock() + fixture.options.executorService = mockExecutorService + whenever(mockExecutorService.submit(any>())).thenReturn(mock()) profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub, never()).captureEnvelope(any()) + verify(mockExecutorService).submit(any()) + val profilingTraceData: ProfilingTraceData? = profiler.onTransactionFinish(fixture.transaction1) + assertNull(profilingTraceData) + verify(mockExecutorService).submit(any>()) } @Test fun `onTransactionFinish works only if previously started`() { val profiler = fixture.getSut(context) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub, never()).captureEnvelope(any()) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1) + assertNull(profilingTraceData) } @Test 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 062558fda3..d0eb2690ed 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 @@ -116,7 +116,6 @@ class EnvelopeTests : BaseUiTest() { options.profilesSampleRate = 1.0 } relayIdlingResource.increment() - // relayIdlingResource.increment() val profilesDirPath = Sentry.getCurrentHub().options.profilingTracesDirPath val transaction = Sentry.startTransaction("emptyProfileTransaction", "test empty") @@ -135,7 +134,6 @@ class EnvelopeTests : BaseUiTest() { finished = true } - Thread.sleep(5000) relay.assert { findEnvelope { assertEnvelopeItem(it.items.toList()).transaction == "emptyProfileTransaction" @@ -144,8 +142,6 @@ class EnvelopeTests : BaseUiTest() { it.assertNoOtherItems() assertEquals("emptyProfileTransaction", transactionItem.transaction) } - // The profile failed to be sent. Trying to read the envelope from the data transmitted throws an exception - // assertFailsWith { assertFirstEnvelope {} } assertNoOtherEnvelopes() assertNoOtherRequests() } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 6f1f3836f1..d63572987d 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -480,6 +480,7 @@ public abstract interface class io/sentry/ISentryExecutorService { public abstract fun close (J)V public abstract fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; public abstract fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; + public abstract fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future; } public abstract interface class io/sentry/ISerializer { diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index aeaa9a3bc8..7f6c81cedc 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.concurrent.Callable; import java.util.concurrent.Future; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -17,6 +18,15 @@ public interface ISentryExecutorService { @NotNull Future submit(final @NotNull Runnable runnable); + /** + * Submits a Callable to the ThreadExecutor + * + * @param callable the Callable + * @return a Future of the Callable + */ + @NotNull + Future submit(final @NotNull Callable callable); + @NotNull Future schedule(final @NotNull Runnable runnable, final long delayMillis); diff --git a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java index dfb323ec4e..232dfaf112 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.concurrent.Callable; import java.util.concurrent.Future; import java.util.concurrent.FutureTask; import org.jetbrains.annotations.NotNull; @@ -18,6 +19,11 @@ private NoOpSentryExecutorService() {} return new FutureTask<>(() -> null); } + @Override + public @NotNull Future submit(final @NotNull Callable callable) { + return new FutureTask<>(() -> null); + } + @Override public @NotNull Future schedule(@NotNull Runnable runnable, long delayMillis) { return new FutureTask<>(() -> null); diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index d388dfd726..89e343d475 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.concurrent.Callable; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; @@ -25,6 +26,11 @@ final class SentryExecutorService implements ISentryExecutorService { return executorService.submit(runnable); } + @Override + public @NotNull Future submit(final @NotNull Callable callable) { + return executorService.submit(callable); + } + @Override public @NotNull Future schedule(final @NotNull Runnable runnable, final long delayMillis) { return executorService.schedule(runnable, delayMillis, TimeUnit.MILLISECONDS); From 907cf94897ce279089d5b03dce71458658226970 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 20 Dec 2022 18:32:45 +0100 Subject: [PATCH 4/5] renamed internal field in AndroidTransactionProfiler currentTransaction -> currentProfilingTransactionData --- .../android/core/AndroidTransactionProfiler.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 93b6f644e1..18f2288f06 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -66,7 +66,7 @@ final class AndroidTransactionProfiler implements ITransactionProfiler { private int transactionsCounter = 0; private @Nullable String frameMetricsCollectorId; private final @NotNull SentryFrameMetricsCollector frameMetricsCollector; - private @Nullable ProfilingTransactionData currentTransaction; + private @Nullable ProfilingTransactionData currentProfilingTransactionData; private final @NotNull ArrayDeque screenFrameRateMeasurements = new ArrayDeque<>(); private final @NotNull ArrayDeque slowFrameRenderMeasurements = @@ -239,7 +239,7 @@ public void onFrameMetricCollected( transactionStartNanos = SystemClock.elapsedRealtimeNanos(); profileStartCpuMillis = Process.getElapsedCpuTime(); - currentTransaction = + currentProfilingTransactionData = new ProfilingTransactionData(transaction, transactionStartNanos, profileStartCpuMillis); Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs); @@ -272,8 +272,8 @@ public void onFrameMetricCollected( final ProfilingTraceData profilingData = timedOutProfilingData; // Transaction finished, but it's not in the current profile - if (currentTransaction == null - || !currentTransaction.getId().equals(transaction.getEventId().toString())) { + if (currentProfilingTransactionData == null + || !currentProfilingTransactionData.getId().equals(transaction.getEventId().toString())) { // We check if we cached a profiling data due to a timeout with this profile in it // If so, we return it back, otherwise we would simply lose it if (profilingData != null) { @@ -317,8 +317,8 @@ public void onFrameMetricCollected( if (transactionsCounter != 0 && !isTimeout) { // We notify the data referring to this transaction that it finished - if (currentTransaction != null) { - currentTransaction.notifyFinish( + if (currentProfilingTransactionData != null) { + currentProfilingTransactionData.notifyFinish( SystemClock.elapsedRealtimeNanos(), transactionStartNanos, Process.getElapsedCpuTime(), @@ -335,8 +335,8 @@ public void onFrameMetricCollected( long transactionDurationNanos = transactionEndNanos - transactionStartNanos; List transactionList = new ArrayList<>(1); - transactionList.add(currentTransaction); - currentTransaction = null; + transactionList.add(currentProfilingTransactionData); + currentProfilingTransactionData = null; // We clear the counter in case of a timeout transactionsCounter = 0; From c255e2e4867f90025730238bb793576e0af187e2 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 21 Dec 2022 18:31:20 +0100 Subject: [PATCH 5/5] used logger to log exceptions --- .../io/sentry/android/core/AndroidTransactionProfiler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 18f2288f06..6054aec885 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -254,9 +254,9 @@ public void onFrameMetricCollected( .submit(() -> onTransactionFinish(transaction, false)) .get(); } catch (ExecutionException e) { - e.printStackTrace(); + options.getLogger().log(SentryLevel.ERROR, "Error finishing profiling: ", e); } catch (InterruptedException e) { - e.printStackTrace(); + options.getLogger().log(SentryLevel.ERROR, "Error finishing profiling: ", e); } return null; }