From 061af134c21dfefb8e347cd76cac9b23edc11f2d Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 26 Jan 2023 18:28:18 +0100 Subject: [PATCH 1/4] moved synchronized() bit later in stop() method in DefaultTransactionPerformanceCollector --- .../sentry/DefaultTransactionPerformanceCollector.java | 10 +++++----- sentry/src/main/java/io/sentry/SentryOptions.java | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java index a9284e4f214..472d7d289ce 100644 --- a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java @@ -98,16 +98,16 @@ public void run() { @Override public @Nullable PerformanceCollectionData stop(final @NotNull ITransaction transaction) { - synchronized (timerLock) { - PerformanceCollectionData data = - performanceDataMap.remove(transaction.getEventId().toString()); - if (performanceDataMap.isEmpty() && isStarted.getAndSet(false)) { + PerformanceCollectionData data = + performanceDataMap.remove(transaction.getEventId().toString()); + if (performanceDataMap.isEmpty() && isStarted.getAndSet(false)) { + synchronized (timerLock) { if (timer != null) { timer.cancel(); timer = null; } } - return data; } + return data; } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 5427f0749be..b7f88ab1962 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -397,7 +397,7 @@ public class SentryOptions { /** Performance collector that collect performance stats while transactions run. */ private final @NotNull TransactionPerformanceCollector transactionPerformanceCollector = - NoOpTransactionPerformanceCollector.getInstance(); + new DefaultTransactionPerformanceCollector(this); /** * Adds an event processor From 4ce947c7a6c6f63d26e9d7c1d9e675071feef5c9 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 31 Jan 2023 17:23:28 +0100 Subject: [PATCH 2/4] PerformanceCollectionData now contains single values instead of lists ICollectors now work on a single instance of PerformanceCollectionData instead than a list, to improve performance on lots of concurrent transactions TransactionPerformanceCollector: is now set in Android options a NoOpTransactionPerformanceCollector is set in Java options deletes data after 30 seconds collect data only if a profile is sampled ITransactionProfiler now works on a list of PerformanceCollectionData --- .../api/sentry-android-core.api | 4 +- .../android/core/AndroidCpuCollector.java | 7 +-- .../android/core/AndroidMemoryCollector.java | 6 +-- .../core/AndroidOptionsInitializer.java | 2 + .../core/AndroidTransactionProfiler.java | 34 ++++++------ .../android/core/AndroidCpuCollectorTest.kt | 20 ++++--- .../core/AndroidMemoryCollectorTest.kt | 7 ++- .../core/AndroidOptionsInitializerTest.kt | 9 ++++ .../core/AndroidTransactionProfilerTest.kt | 20 ++++--- .../android/core/SentryAndroidOptionsTest.kt | 2 +- sentry/api/sentry.api | 19 +++---- ...efaultTransactionPerformanceCollector.java | 54 ++++++++++--------- .../src/main/java/io/sentry/ICollector.java | 2 +- .../java/io/sentry/ITransactionProfiler.java | 4 +- .../java/io/sentry/JavaMemoryCollector.java | 6 +-- .../NoOpTransactionPerformanceCollector.java | 3 +- .../io/sentry/NoOpTransactionProfiler.java | 4 +- .../io/sentry/PerformanceCollectionData.java | 15 +++--- .../main/java/io/sentry/SentryOptions.java | 15 +++++- .../src/main/java/io/sentry/SentryTracer.java | 9 +++- .../TransactionPerformanceCollector.java | 3 +- ...ultTransactionPerformanceCollectorTest.kt} | 39 ++++++++------ .../java/io/sentry/JavaMemoryCollectorTest.kt | 17 +++--- .../sentry/PerformanceCollectionDataTest.kt | 34 ++++-------- .../test/java/io/sentry/SentryOptionsTest.kt | 6 +++ sentry/src/test/java/io/sentry/SentryTest.kt | 2 +- .../test/java/io/sentry/SentryTracerTest.kt | 10 +++- 27 files changed, 198 insertions(+), 155 deletions(-) rename sentry/src/test/java/io/sentry/{TransactionPerformanceCollectorTest.kt => DefaultTransactionPerformanceCollectorTest.kt} (88%) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index fcd2c080e96..ac614cb0ee5 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -25,7 +25,7 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android public final class io/sentry/android/core/AndroidCpuCollector : io/sentry/ICollector { public fun (Lio/sentry/ILogger;Lio/sentry/android/core/BuildInfoProvider;)V - public fun collect (Ljava/lang/Iterable;)V + public fun collect (Lio/sentry/PerformanceCollectionData;)V public fun setup ()V } @@ -45,7 +45,7 @@ public final class io/sentry/android/core/AndroidLogger : io/sentry/ILogger { public class io/sentry/android/core/AndroidMemoryCollector : io/sentry/ICollector { public fun ()V - public fun collect (Ljava/lang/Iterable;)V + public fun collect (Lio/sentry/PerformanceCollectionData;)V public fun setup ()V } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidCpuCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidCpuCollector.java index c3d41f0aac5..0992fec96d0 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidCpuCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidCpuCollector.java @@ -66,8 +66,7 @@ public void setup() { @SuppressLint("NewApi") @Override - public void collect( - @NotNull final Iterable performanceCollectionData) { + public void collect(final @NotNull PerformanceCollectionData performanceCollectionData) { if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP || !isEnabled) { return; } @@ -86,9 +85,7 @@ public void collect( new CpuCollectionData( System.currentTimeMillis(), (cpuUsagePercentage / (double) numCores) * 100.0); - for (PerformanceCollectionData data : performanceCollectionData) { - data.addCpuData(cpuData); - } + performanceCollectionData.addCpuData(cpuData); } /** Read the /proc/self/stat file and parses the result. */ diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMemoryCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMemoryCollector.java index 2b03ef8b87f..e43823b8d0d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMemoryCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMemoryCollector.java @@ -14,13 +14,11 @@ public class AndroidMemoryCollector implements ICollector { public void setup() {} @Override - public void collect(@NotNull Iterable performanceCollectionData) { + public void collect(final @NotNull PerformanceCollectionData performanceCollectionData) { long now = System.currentTimeMillis(); long usedMemory = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); long usedNativeMemory = Debug.getNativeHeapSize() - Debug.getNativeHeapFreeSize(); MemoryCollectionData memoryData = new MemoryCollectionData(now, usedMemory, usedNativeMemory); - for (PerformanceCollectionData data : performanceCollectionData) { - data.addMemoryData(memoryData); - } + performanceCollectionData.addMemoryData(memoryData); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 358c6fb9048..16465c8b783 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -7,6 +7,7 @@ import android.content.pm.PackageInfo; import android.content.res.AssetManager; import android.os.Build; +import io.sentry.DefaultTransactionPerformanceCollector; import io.sentry.ILogger; import io.sentry.SendFireAndForgetEnvelopeSender; import io.sentry.SendFireAndForgetOutboxSender; @@ -168,6 +169,7 @@ static void initializeIntegrationsAndProcessors( options.addCollector(new AndroidMemoryCollector()); options.addCollector(new AndroidCpuCollector(options.getLogger(), buildInfoProvider)); } + options.setTransactionPerformanceCollector(new DefaultTransactionPerformanceCollector(options)); } private static void installDefaultIntegrations( 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 57cd6dd3291..7ec5297975d 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 @@ -251,7 +251,7 @@ public void onFrameMetricCollected( @Override public @Nullable synchronized ProfilingTraceData onTransactionFinish( final @NotNull ITransaction transaction, - final @Nullable PerformanceCollectionData performanceCollectionData) { + final @Nullable List performanceCollectionData) { try { return options .getExecutorService() @@ -269,7 +269,7 @@ public void onFrameMetricCollected( private @Nullable ProfilingTraceData onTransactionFinish( final @NotNull ITransaction transaction, final boolean isTimeout, - final @Nullable PerformanceCollectionData performanceCollectionData) { + final @Nullable List performanceCollectionData) { // onTransactionStart() is only available since Lollipop // and SystemClock.elapsedRealtimeNanos() since Jelly Bean @@ -416,28 +416,32 @@ public void onFrameMetricCollected( } private void putPerformanceCollectionDataInMeasurements( - final @Nullable PerformanceCollectionData performanceCollectionData) { + final @Nullable List performanceCollectionData) { if (performanceCollectionData != null) { final @NotNull ArrayDeque memoryUsageMeasurements = - new ArrayDeque<>(); + new ArrayDeque<>(performanceCollectionData.size()); final @NotNull ArrayDeque nativeMemoryUsageMeasurements = - new ArrayDeque<>(); - final @NotNull ArrayDeque cpuUsageMeasurements = new ArrayDeque<>(); - for (CpuCollectionData cpuData : performanceCollectionData.getCpuData()) { - cpuUsageMeasurements.add( - new ProfileMeasurementValue( - TimeUnit.MILLISECONDS.toNanos(cpuData.getTimestampMillis()) - transactionStartNanos, - cpuData.getCpuUsagePercentage())); - } - for (MemoryCollectionData memoryData : performanceCollectionData.getMemoryData()) { - if (memoryData.getUsedHeapMemory() > -1) { + new ArrayDeque<>(performanceCollectionData.size()); + final @NotNull ArrayDeque cpuUsageMeasurements = + new ArrayDeque<>(performanceCollectionData.size()); + for (PerformanceCollectionData performanceData : performanceCollectionData) { + CpuCollectionData cpuData = performanceData.getCpuData(); + MemoryCollectionData memoryData = performanceData.getMemoryData(); + if (cpuData != null) { + cpuUsageMeasurements.add( + new ProfileMeasurementValue( + TimeUnit.MILLISECONDS.toNanos(cpuData.getTimestampMillis()) + - transactionStartNanos, + cpuData.getCpuUsagePercentage())); + } + if (memoryData != null && memoryData.getUsedHeapMemory() > -1) { memoryUsageMeasurements.add( new ProfileMeasurementValue( TimeUnit.MILLISECONDS.toNanos(memoryData.getTimestampMillis()) - transactionStartNanos, memoryData.getUsedHeapMemory())); } - if (memoryData.getUsedNativeMemory() > -1) { + if (memoryData != null && memoryData.getUsedNativeMemory() > -1) { nativeMemoryUsageMeasurements.add( new ProfileMeasurementValue( TimeUnit.MILLISECONDS.toNanos(memoryData.getTimestampMillis()) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidCpuCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidCpuCollectorTest.kt index 671093ddf01..9ead4422ba1 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidCpuCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidCpuCollectorTest.kt @@ -8,10 +8,9 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import kotlin.test.Test import kotlin.test.assertFailsWith -import kotlin.test.assertFalse import kotlin.test.assertNotEquals import kotlin.test.assertNotNull -import kotlin.test.assertTrue +import kotlin.test.assertNull class AndroidCpuCollectorTest { @@ -43,9 +42,9 @@ class AndroidCpuCollectorTest { @Test fun `collect works only after setup`() { val data = PerformanceCollectionData() - fixture.getSut().collect(listOf(data)) + fixture.getSut().collect(data) data.commitData() - assertTrue(data.cpuData.isEmpty()) + assertNull(data.cpuData) } @Test @@ -53,13 +52,12 @@ class AndroidCpuCollectorTest { val data = PerformanceCollectionData() val collector = fixture.getSut() collector.setup() - collector.collect(listOf(data)) + collector.collect(data) data.commitData() val cpuData = data.cpuData - assertNotNull(data.cpuData) - assertFalse(data.cpuData.isEmpty()) - assertNotEquals(0.0, cpuData[0].cpuUsagePercentage) - assertNotEquals(0, cpuData[0].timestampMillis) + assertNotNull(cpuData) + assertNotEquals(0.0, cpuData.cpuUsagePercentage) + assertNotEquals(0, cpuData.timestampMillis) } @Test @@ -69,8 +67,8 @@ class AndroidCpuCollectorTest { whenever(mockBuildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.KITKAT) val collector = fixture.getSut(mockBuildInfoProvider) collector.setup() - collector.collect(listOf(data)) + collector.collect(data) data.commitData() - assertTrue(data.cpuData.isEmpty()) + assertNull(data.cpuData) } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMemoryCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMemoryCollectorTest.kt index dad91d1206a..431e94c2a9d 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMemoryCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMemoryCollectorTest.kt @@ -18,13 +18,12 @@ class AndroidMemoryCollectorTest { @Test fun `when collect, both native and heap memory are collected`() { - val performanceCollectionData = PerformanceCollectionData() - val data = listOf(performanceCollectionData) + val data = PerformanceCollectionData() val usedNativeMemory = Debug.getNativeHeapSize() - Debug.getNativeHeapFreeSize() val usedMemory = fixture.runtime.totalMemory() - fixture.runtime.freeMemory() fixture.collector.collect(data) - performanceCollectionData.commitData() - val memoryData = performanceCollectionData.memoryData.firstOrNull() + data.commitData() + val memoryData = data.memoryData assertNotNull(memoryData) assertNotEquals(-1, memoryData.usedNativeMemory) assertEquals(usedNativeMemory, memoryData.usedNativeMemory) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index adc2e88b042..114646985ad 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -4,6 +4,7 @@ import android.content.Context import android.os.Bundle import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.DefaultTransactionPerformanceCollector import io.sentry.ILogger import io.sentry.MainEventProcessor import io.sentry.SentryOptions @@ -24,6 +25,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertIs import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -525,4 +527,11 @@ class AndroidOptionsInitializerTest { assertTrue { fixture.sentryOptions.collectors.any { it is AndroidCpuCollector } } } + + @Test + fun `DefaultTransactionPerformanceCollector is set to options`() { + fixture.initSut() + + assertIs(fixture.sentryOptions.transactionPerformanceCollector) + } } 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 52cc697f183..1fc1b854597 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 @@ -376,14 +376,20 @@ class AndroidTransactionProfilerTest { @Test fun `profiler includes performance measurements when passed on transaction finish`() { val profiler = fixture.getSut(context) - val memoryCollectionData = PerformanceCollectionData() - memoryCollectionData.addMemoryData(MemoryCollectionData(1, 2, 3)) - memoryCollectionData.addCpuData(CpuCollectionData(1, 1.4)) - memoryCollectionData.commitData() - memoryCollectionData.addMemoryData(MemoryCollectionData(2, 3, 4)) - memoryCollectionData.commitData() + val performanceCollectionData = ArrayList() + var singleData = PerformanceCollectionData() + singleData.addMemoryData(MemoryCollectionData(1, 2, 3)) + singleData.addCpuData(CpuCollectionData(1, 1.4)) + singleData.commitData() + performanceCollectionData.add(singleData) + + singleData = PerformanceCollectionData() + singleData.addMemoryData(MemoryCollectionData(2, 3, 4)) + singleData.commitData() + performanceCollectionData.add(singleData) + profiler.onTransactionStart(fixture.transaction1) - val data = profiler.onTransactionFinish(fixture.transaction1, memoryCollectionData) + val data = profiler.onTransactionFinish(fixture.transaction1, performanceCollectionData) assertContentEquals( listOf(1.4), data!!.measurementsMap[ProfileMeasurement.ID_CPU_USAGE]!!.values.map { it.value } 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 be11c4ca418..f5068c7762b 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 @@ -113,7 +113,7 @@ class SentryAndroidOptionsTest { override fun onTransactionStart(transaction: ITransaction) {} override fun onTransactionFinish( transaction: ITransaction, - memoryCollectionData: PerformanceCollectionData? + performanceCollectionData: List? ): ProfilingTraceData? = null } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 81bbb428ae8..8359433343e 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -186,7 +186,7 @@ public final class io/sentry/DateUtils { public final class io/sentry/DefaultTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector { public fun (Lio/sentry/SentryOptions;)V public fun start (Lio/sentry/ITransaction;)V - public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData; + public fun stop (Lio/sentry/ITransaction;)Ljava/util/List; } public final class io/sentry/DiagnosticLogger : io/sentry/ILogger { @@ -391,7 +391,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { } public abstract interface class io/sentry/ICollector { - public abstract fun collect (Ljava/lang/Iterable;)V + public abstract fun collect (Lio/sentry/PerformanceCollectionData;)V public abstract fun setup ()V } @@ -571,7 +571,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;Lio/sentry/PerformanceCollectionData;)Lio/sentry/ProfilingTraceData; + public abstract fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;)Lio/sentry/ProfilingTraceData; public abstract fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -596,7 +596,7 @@ public final class io/sentry/IpAddressUtils { public final class io/sentry/JavaMemoryCollector : io/sentry/ICollector { public fun ()V - public fun collect (Ljava/lang/Iterable;)V + public fun collect (Lio/sentry/PerformanceCollectionData;)V public fun setup ()V } @@ -862,12 +862,12 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { public final class io/sentry/NoOpTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector { public static fun getInstance ()Lio/sentry/NoOpTransactionPerformanceCollector; public fun start (Lio/sentry/ITransaction;)V - public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData; + public fun stop (Lio/sentry/ITransaction;)Ljava/util/List; } public final class io/sentry/NoOpTransactionProfiler : io/sentry/ITransactionProfiler { public static fun getInstance ()Lio/sentry/NoOpTransactionProfiler; - public fun onTransactionFinish (Lio/sentry/ITransaction;Lio/sentry/PerformanceCollectionData;)Lio/sentry/ProfilingTraceData; + public fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;)Lio/sentry/ProfilingTraceData; public fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -892,8 +892,8 @@ public final class io/sentry/PerformanceCollectionData { public fun addCpuData (Lio/sentry/CpuCollectionData;)V public fun addMemoryData (Lio/sentry/MemoryCollectionData;)V public fun commitData ()V - public fun getCpuData ()Ljava/util/List; - public fun getMemoryData ()Ljava/util/List; + public fun getCpuData ()Lio/sentry/CpuCollectionData; + public fun getMemoryData ()Lio/sentry/MemoryCollectionData; } public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io/sentry/JsonUnknown { @@ -1646,6 +1646,7 @@ public class io/sentry/SentryOptions { public fun setTracesSampleRate (Ljava/lang/Double;)V public fun setTracesSampler (Lio/sentry/SentryOptions$TracesSamplerCallback;)V public fun setTracingOrigins (Ljava/util/List;)V + public fun setTransactionPerformanceCollector (Lio/sentry/TransactionPerformanceCollector;)V public fun setTransactionProfiler (Lio/sentry/ITransactionProfiler;)V public fun setTransportFactory (Lio/sentry/ITransportFactory;)V public fun setTransportGate (Lio/sentry/transport/ITransportGate;)V @@ -2069,7 +2070,7 @@ public final class io/sentry/TransactionOptions { public abstract interface class io/sentry/TransactionPerformanceCollector { public abstract fun start (Lio/sentry/ITransaction;)V - public abstract fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData; + public abstract fun stop (Lio/sentry/ITransaction;)Ljava/util/List; } public final class io/sentry/TypeCheckHint { diff --git a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java index 472d7d289ce..159d28be87b 100644 --- a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java @@ -1,6 +1,7 @@ package io.sentry; import io.sentry.util.Objects; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Timer; @@ -18,7 +19,7 @@ public final class DefaultTransactionPerformanceCollector private static final long TRANSACTION_COLLECTION_TIMEOUT_MILLIS = 30000; private final @NotNull Object timerLock = new Object(); private volatile @Nullable Timer timer = null; - private final @NotNull Map performanceDataMap = + private final @NotNull Map> performanceDataMap = new ConcurrentHashMap<>(); private final @NotNull List collectors; private final @NotNull SentryOptions options; @@ -42,17 +43,11 @@ public void start(final @NotNull ITransaction transaction) { } if (!performanceDataMap.containsKey(transaction.getEventId().toString())) { - performanceDataMap.put(transaction.getEventId().toString(), new PerformanceCollectionData()); + performanceDataMap.put(transaction.getEventId().toString(), new ArrayList<>()); + // We schedule deletion of collected performance data after a timeout options .getExecutorService() - .schedule( - () -> { - PerformanceCollectionData data = stop(transaction); - if (data != null) { - performanceDataMap.put(transaction.getEventId().toString(), data); - } - }, - TRANSACTION_COLLECTION_TIMEOUT_MILLIS); + .schedule(() -> stop(transaction), TRANSACTION_COLLECTION_TIMEOUT_MILLIS); } if (!isStarted.getAndSet(true)) { synchronized (timerLock) { @@ -74,22 +69,26 @@ public void run() { // and collect() calls. // This way ICollectors that collect average stats based on time intervals, like // AndroidCpuCollector, can have an actual time interval to evaluate. - timer.scheduleAtFixedRate( + TimerTask timerTask = new TimerTask() { @Override public void run() { - synchronized (timerLock) { - for (ICollector collector : collectors) { - collector.collect(performanceDataMap.values()); - } - // We commit data after calling all collectors. - // This way we avoid issues caused by having multiple cpu or memory collectors. - for (PerformanceCollectionData data : performanceDataMap.values()) { - data.commitData(); - } + final @NotNull PerformanceCollectionData tempData = new PerformanceCollectionData(); + + for (ICollector collector : collectors) { + collector.collect(tempData); + } + // We commit data after calling all collectors. + // This way we avoid issues caused by having multiple cpu or memory collectors. + tempData.commitData(); + + for (List data : performanceDataMap.values()) { + data.add(tempData); } } - }, + }; + timer.scheduleAtFixedRate( + timerTask, TRANSACTION_COLLECTION_INTERVAL_MILLIS, TRANSACTION_COLLECTION_INTERVAL_MILLIS); } @@ -97,9 +96,16 @@ public void run() { } @Override - public @Nullable PerformanceCollectionData stop(final @NotNull ITransaction transaction) { - PerformanceCollectionData data = - performanceDataMap.remove(transaction.getEventId().toString()); + public @Nullable List stop(final @NotNull ITransaction transaction) { + List data = + performanceDataMap.remove(transaction.getEventId().toString()); + options + .getLogger() + .log( + SentryLevel.DEBUG, + "stop collecting performance info for transactions %s (%s)", + transaction.getName(), + transaction.getSpanContext().getTraceId().toString()); if (performanceDataMap.isEmpty() && isStarted.getAndSet(false)) { synchronized (timerLock) { if (timer != null) { diff --git a/sentry/src/main/java/io/sentry/ICollector.java b/sentry/src/main/java/io/sentry/ICollector.java index cae520e063c..945c113d697 100644 --- a/sentry/src/main/java/io/sentry/ICollector.java +++ b/sentry/src/main/java/io/sentry/ICollector.java @@ -9,5 +9,5 @@ public interface ICollector { void setup(); - void collect(@NotNull final Iterable performanceCollectionData); + void collect(final @NotNull PerformanceCollectionData performanceCollectionData); } diff --git a/sentry/src/main/java/io/sentry/ITransactionProfiler.java b/sentry/src/main/java/io/sentry/ITransactionProfiler.java index a53cba859ed..44676e6f727 100644 --- a/sentry/src/main/java/io/sentry/ITransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/ITransactionProfiler.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -11,5 +12,6 @@ public interface ITransactionProfiler { @Nullable ProfilingTraceData onTransactionFinish( - @NotNull ITransaction transaction, @Nullable PerformanceCollectionData memoryCollectionData); + @NotNull ITransaction transaction, + @Nullable List performanceCollectionData); } diff --git a/sentry/src/main/java/io/sentry/JavaMemoryCollector.java b/sentry/src/main/java/io/sentry/JavaMemoryCollector.java index b25117ad9a3..36e6f075dec 100644 --- a/sentry/src/main/java/io/sentry/JavaMemoryCollector.java +++ b/sentry/src/main/java/io/sentry/JavaMemoryCollector.java @@ -12,12 +12,10 @@ public final class JavaMemoryCollector implements ICollector { public void setup() {} @Override - public void collect(@NotNull Iterable performanceCollectionData) { + public void collect(final @NotNull PerformanceCollectionData performanceCollectionData) { final long now = System.currentTimeMillis(); final long usedMemory = runtime.totalMemory() - runtime.freeMemory(); MemoryCollectionData memoryData = new MemoryCollectionData(now, usedMemory); - for (PerformanceCollectionData data : performanceCollectionData) { - data.addMemoryData(memoryData); - } + performanceCollectionData.addMemoryData(memoryData); } } diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java index 8a0bd40b59e..02edeb10944 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.List; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -18,7 +19,7 @@ private NoOpTransactionPerformanceCollector() {} public void start(@NotNull ITransaction transaction) {} @Override - public @Nullable PerformanceCollectionData stop(@NotNull ITransaction transaction) { + public @Nullable List stop(@NotNull ITransaction transaction) { return null; } } diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java index c0dffb42f56..9f51ea33a97 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.List; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -18,7 +19,8 @@ public void onTransactionStart(@NotNull ITransaction transaction) {} @Override public @Nullable ProfilingTraceData onTransactionFinish( - @NotNull ITransaction transaction, @Nullable PerformanceCollectionData memoryCollectionData) { + @NotNull ITransaction transaction, + @Nullable List performanceCollectionData) { return null; } } diff --git a/sentry/src/main/java/io/sentry/PerformanceCollectionData.java b/sentry/src/main/java/io/sentry/PerformanceCollectionData.java index 1c5b79656e2..f6248830757 100644 --- a/sentry/src/main/java/io/sentry/PerformanceCollectionData.java +++ b/sentry/src/main/java/io/sentry/PerformanceCollectionData.java @@ -1,15 +1,12 @@ package io.sentry; -import java.util.ArrayList; -import java.util.List; import org.jetbrains.annotations.ApiStatus; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @ApiStatus.Internal public final class PerformanceCollectionData { - private final @NotNull List memoryData = new ArrayList<>(); - private final @NotNull List cpuData = new ArrayList<>(); + private @Nullable MemoryCollectionData memoryData = null; + private @Nullable CpuCollectionData cpuData = null; private @Nullable MemoryCollectionData uncommittedMemoryData = null; private @Nullable CpuCollectionData uncommittedCpuData = null; @@ -36,20 +33,20 @@ public void addCpuData(final @Nullable CpuCollectionData cpuCollectionData) { /** Save any uncommitted data. */ public void commitData() { if (uncommittedMemoryData != null) { - memoryData.add(uncommittedMemoryData); + memoryData = uncommittedMemoryData; uncommittedMemoryData = null; } if (uncommittedCpuData != null) { - cpuData.add(uncommittedCpuData); + cpuData = uncommittedCpuData; uncommittedCpuData = null; } } - public @NotNull List getCpuData() { + public @Nullable CpuCollectionData getCpuData() { return cpuData; } - public @NotNull List getMemoryData() { + public @Nullable MemoryCollectionData getMemoryData() { return memoryData; } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index b7f88ab1962..2ce67f06b39 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -396,8 +396,8 @@ public class SentryOptions { private final @NotNull List collectors = new ArrayList<>(); /** Performance collector that collect performance stats while transactions run. */ - private final @NotNull TransactionPerformanceCollector transactionPerformanceCollector = - new DefaultTransactionPerformanceCollector(this); + private @NotNull TransactionPerformanceCollector transactionPerformanceCollector = + NoOpTransactionPerformanceCollector.getInstance(); /** * Adds an event processor @@ -1900,6 +1900,17 @@ public void setMainThreadChecker(final @NotNull IMainThreadChecker mainThreadChe return transactionPerformanceCollector; } + /** + * Sets the performance collector used to collect performance stats while transactions run. + * + * @param transactionPerformanceCollector the performance collector. + */ + @ApiStatus.Internal + public void setTransactionPerformanceCollector( + final @NotNull TransactionPerformanceCollector transactionPerformanceCollector) { + this.transactionPerformanceCollector = transactionPerformanceCollector; + } + /** * Whether OPTIONS requests should be traced. * diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 10c774a839b..08574ce9422 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -141,7 +141,9 @@ public SentryTracer( this.baggage = new Baggage(hub.getOptions().getLogger()); } - if (transactionPerformanceCollector != null) { + // We are currently sending the performance data only in profiles, so there's no point in + // collecting them if a profile is not sampled + if (transactionPerformanceCollector != null && Boolean.TRUE.equals(isProfileSampled())) { transactionPerformanceCollector.start(this); } @@ -345,7 +347,7 @@ public void finish(@Nullable SpanStatus status) { public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) { this.finishStatus = FinishStatus.finishing(status); if (!root.isFinished() && (!waitForChildren || hasAllChildrenFinished())) { - PerformanceCollectionData performanceCollectionData = null; + List performanceCollectionData = null; if (transactionPerformanceCollector != null) { performanceCollectionData = transactionPerformanceCollector.stop(this); } @@ -357,6 +359,9 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) .getTransactionProfiler() .onTransactionFinish(this, performanceCollectionData); } + if (performanceCollectionData != null) { + performanceCollectionData.clear(); + } // try to get the high precision timestamp from the root span SentryDate finishTimestamp = root.getFinishDate(); diff --git a/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java index 81a6803be8d..013667d2cfd 100644 --- a/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.List; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -8,5 +9,5 @@ public interface TransactionPerformanceCollector { void start(@NotNull ITransaction transaction); @Nullable - PerformanceCollectionData stop(@NotNull ITransaction transaction); + List stop(@NotNull ITransaction transaction); } diff --git a/sentry/src/test/java/io/sentry/TransactionPerformanceCollectorTest.kt b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt similarity index 88% rename from sentry/src/test/java/io/sentry/TransactionPerformanceCollectorTest.kt rename to sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt index 5d7b5b7718f..b87de67f68f 100644 --- a/sentry/src/test/java/io/sentry/TransactionPerformanceCollectorTest.kt +++ b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt @@ -24,7 +24,7 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue -class TransactionPerformanceCollectorTest { +class DefaultTransactionPerformanceCollectorTest { private val className = "io.sentry.DefaultTransactionPerformanceCollector" private val ctorTypes: Array> = arrayOf(SentryOptions::class.java) @@ -51,10 +51,8 @@ class TransactionPerformanceCollectorTest { val mockCpuCollector: ICollector = object : ICollector { override fun setup() {} - override fun collect(performanceCollectionData: Iterable) { - performanceCollectionData.forEach { - it.addCpuData(mock()) - } + override fun collect(performanceCollectionData: PerformanceCollectionData) { + performanceCollectionData.addCpuData(mock()) } } @@ -150,11 +148,18 @@ class TransactionPerformanceCollectorTest { // There are no more transactions running: the time should stop now verify(fixture.mockTimer)!!.cancel() + assertNotNull(data1) + assertNotNull(data2) + val memoryData1 = data1.map { it.memoryData } + val cpuData1 = data1.map { it.cpuData } + val memoryData2 = data2.map { it.memoryData } + val cpuData2 = data2.map { it.cpuData } + // The data returned by the collector is not empty - assertFalse(data1!!.memoryData.isEmpty()) - assertFalse(data1.cpuData.isEmpty()) - assertFalse(data2!!.memoryData.isEmpty()) - assertFalse(data2.cpuData.isEmpty()) + assertFalse(memoryData1.isEmpty()) + assertFalse(cpuData1.isEmpty()) + assertFalse(memoryData2.isEmpty()) + assertFalse(cpuData2.isEmpty()) } @Test @@ -169,10 +174,9 @@ class TransactionPerformanceCollectorTest { fixture.lastScheduledRunnable?.run() verify(fixture.mockTimer)!!.cancel() - // Data is returned even after the collector times out + // Data is deleted after the collector times out val data1 = collector.stop(fixture.transaction1) - assertFalse(data1!!.memoryData.isEmpty()) - assertFalse(data1.cpuData.isEmpty()) + assertNull(data1) } @Test @@ -193,13 +197,16 @@ class TransactionPerformanceCollectorTest { // Let's sleep to make the collector get values Thread.sleep(300) val data1 = collector.stop(fixture.transaction1) + assertNotNull(data1) + val memoryData = data1.map { it.memoryData } + val cpuData = data1.map { it.cpuData } // The data returned by the collector is not empty - assertFalse(data1!!.memoryData.isEmpty()) - assertFalse(data1.cpuData.isEmpty()) + assertFalse(memoryData.isEmpty()) + assertFalse(cpuData.isEmpty()) // We have the same number of memory and cpu data, even if we have 2 memory collectors and 1 cpu collector - assertEquals(data1.memoryData.size, data1.cpuData.size) + assertEquals(memoryData.size, cpuData.size) } @Test @@ -226,7 +233,7 @@ class TransactionPerformanceCollectorTest { } } - override fun collect(performanceCollectionData: MutableIterable) { + override fun collect(performanceCollectionData: PerformanceCollectionData) { if (mainThreadChecker.isMainThread) { throw AssertionError("collect() was called in the main thread") } diff --git a/sentry/src/test/java/io/sentry/JavaMemoryCollectorTest.kt b/sentry/src/test/java/io/sentry/JavaMemoryCollectorTest.kt index 8af6b2ab4d0..7e95c0e283f 100644 --- a/sentry/src/test/java/io/sentry/JavaMemoryCollectorTest.kt +++ b/sentry/src/test/java/io/sentry/JavaMemoryCollectorTest.kt @@ -2,8 +2,8 @@ package io.sentry import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull class JavaMemoryCollectorTest { @@ -16,15 +16,14 @@ class JavaMemoryCollectorTest { @Test fun `when collect, only heap memory is collected`() { - val performanceCollectionData = PerformanceCollectionData() - val data = listOf(performanceCollectionData) + val data = PerformanceCollectionData() val usedMemory = fixture.runtime.totalMemory() - fixture.runtime.freeMemory() fixture.collector.collect(data) - performanceCollectionData.commitData() - val memoryData = performanceCollectionData.memoryData - assertFalse(memoryData.isEmpty()) - assertEquals(-1, memoryData.first().usedNativeMemory) - assertEquals(usedMemory, memoryData.first().usedHeapMemory) - assertNotEquals(0, memoryData.first().timestampMillis) + data.commitData() + val memoryData = data.memoryData + assertNotNull(memoryData) + assertEquals(-1, memoryData.usedNativeMemory) + assertEquals(usedMemory, memoryData.usedHeapMemory) + assertNotEquals(0, memoryData.timestampMillis) } } diff --git a/sentry/src/test/java/io/sentry/PerformanceCollectionDataTest.kt b/sentry/src/test/java/io/sentry/PerformanceCollectionDataTest.kt index d0db2807bd7..0f7f8247215 100644 --- a/sentry/src/test/java/io/sentry/PerformanceCollectionDataTest.kt +++ b/sentry/src/test/java/io/sentry/PerformanceCollectionDataTest.kt @@ -3,9 +3,9 @@ package io.sentry import org.mockito.kotlin.mock import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertNotEquals -import kotlin.test.assertTrue +import kotlin.test.assertNotNull +import kotlin.test.assertNull class PerformanceCollectionDataTest { @@ -19,18 +19,18 @@ class PerformanceCollectionDataTest { fun `memory data is saved only after commitData`() { val data = fixture.getSut() data.addMemoryData(mock()) - assertTrue(data.memoryData.isEmpty()) + assertNull(data.memoryData) data.commitData() - assertFalse(data.memoryData.isEmpty()) + assertNotNull(data.memoryData) } @Test fun `cpu data is saved only after commitData`() { val data = fixture.getSut() data.addCpuData(mock()) - assertTrue(data.cpuData.isEmpty()) + assertNull(data.cpuData) data.commitData() - assertFalse(data.cpuData.isEmpty()) + assertNotNull(data.cpuData) } @Test @@ -41,7 +41,7 @@ class PerformanceCollectionDataTest { data.addMemoryData(memData1) data.addMemoryData(memData2) data.commitData() - val savedMemoryData = data.memoryData.first() + val savedMemoryData = data.memoryData assertNotEquals(memData1, savedMemoryData) assertEquals(memData2, savedMemoryData) } @@ -54,7 +54,7 @@ class PerformanceCollectionDataTest { data.addCpuData(cpuData1) data.addCpuData(cpuData2) data.commitData() - val savedCpuData = data.cpuData.first() + val savedCpuData = data.cpuData assertNotEquals(cpuData1, savedCpuData) assertEquals(cpuData2, savedCpuData) } @@ -67,22 +67,8 @@ class PerformanceCollectionDataTest { data.addCpuData(null) data.addMemoryData(null) data.commitData() - assertEquals(1, data.cpuData.size) - assertTrue(data.memoryData.isEmpty()) - val savedCpuData = data.cpuData.first() + assertNull(data.memoryData) + val savedCpuData = data.cpuData assertEquals(cpuData1, savedCpuData) } - - @Test - fun `committing multiple times does not duplicate values`() { - val data = fixture.getSut() - data.addCpuData(mock()) - data.addMemoryData(mock()) - data.commitData() - assertEquals(1, data.cpuData.size) - assertEquals(1, data.memoryData.size) - data.commitData() - assertEquals(1, data.cpuData.size) - assertEquals(1, data.memoryData.size) - } } diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 1906bb679ab..06e193aa17d 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -388,7 +388,13 @@ class SentryOptionsTest { assertEquals("${File.separator}test${File.separator}${hash}${File.separator}profiling_traces", options.profilingTracesDirPath) } + @Test fun `when options are initialized, idleTimeout is 3000`() { assertEquals(3000L, SentryOptions().idleTimeout) } + + @Test + fun `when options are initialized, TransactionPerformanceCollector is a NoOp`() { + assertEquals(SentryOptions().transactionPerformanceCollector, NoOpTransactionPerformanceCollector.getInstance()) + } } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index d0c01aad40d..f6258fcf2b1 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -381,7 +381,7 @@ class SentryTest { private class CustomMemoryCollector : ICollector { override fun setup() {} - override fun collect(performanceCollectionData: MutableIterable) {} + override fun collect(performanceCollectionData: PerformanceCollectionData) {} } private class CustomModulesLoader : IModulesLoader { diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index d25631a5a3d..6d5d4651629 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -888,8 +888,16 @@ class SentryTracerTest { } @Test - fun `when transaction is created, transactionPerformanceCollector is started`() { + fun `when transaction is created, but not profiled, transactionPerformanceCollector is not started`() { val transaction = fixture.getSut() + verify(fixture.transactionPerformanceCollector, never()).start(anyOrNull()) + } + + @Test + fun `when transaction is created and profiled transactionPerformanceCollector is started`() { + val transaction = fixture.getSut(optionsConfiguration = { + it.profilesSampleRate = 1.0 + }, samplingDecision = TracesSamplingDecision(true, null, true, null)) verify(fixture.transactionPerformanceCollector).start(check { assertEquals(transaction, it) }) } From 6e687bc978d9af9a0c9098dc36cc0173e69fe4b4 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 31 Jan 2023 17:41:26 +0100 Subject: [PATCH 3/4] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18d48c036cc..739bd707b21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Fix transaction performance collector oom ([#2505](https://github.com/getsentry/sentry-java/pull/2505)) - Remove authority from URLs sent to Sentry ([#2366](https://github.com/getsentry/sentry-java/pull/2366)) - Fix `sentry-bom` containing incorrect artifacts ([#2504](https://github.com/getsentry/sentry-java/pull/2504)) From 0656c210a919ff511cb3cd456936079d155179ec Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 31 Jan 2023 19:12:40 +0100 Subject: [PATCH 4/4] added 2 unit tests --- .../test/java/io/sentry/SentryOptionsTest.kt | 8 ++++++++ .../test/java/io/sentry/SentryTracerTest.kt | 19 +++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 06e193aa17d..32a2420a78c 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -397,4 +397,12 @@ class SentryOptionsTest { fun `when options are initialized, TransactionPerformanceCollector is a NoOp`() { assertEquals(SentryOptions().transactionPerformanceCollector, NoOpTransactionPerformanceCollector.getInstance()) } + + @Test + fun `when setTransactionPerformanceCollector is called, overrides default`() { + val performanceCollector = mock() + val options = SentryOptions() + options.transactionPerformanceCollector = performanceCollector + assertEquals(performanceCollector, options.transactionPerformanceCollector) + } } diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 6d5d4651629..a3cf806e2ee 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -45,10 +45,11 @@ class SentryTracerTest { idleTimeout: Long? = null, trimEnd: Boolean = false, transactionFinishedCallback: TransactionFinishedCallback? = null, - samplingDecision: TracesSamplingDecision? = null + samplingDecision: TracesSamplingDecision? = null, + performanceCollector: TransactionPerformanceCollector? = transactionPerformanceCollector ): SentryTracer { optionsConfiguration.configure(options) - return SentryTracer(TransactionContext("name", "op", samplingDecision), hub, startTimestamp, waitForChildren, idleTimeout, trimEnd, transactionFinishedCallback, transactionPerformanceCollector) + return SentryTracer(TransactionContext("name", "op", samplingDecision), hub, startTimestamp, waitForChildren, idleTimeout, trimEnd, transactionFinishedCallback, performanceCollector) } } @@ -917,4 +918,18 @@ class SentryTracerTest { assertEquals("new-name-2", transaction.name) assertEquals(TransactionNameSource.CUSTOM, transaction.transactionNameSource) } + + @Test + fun `when transaction is finished, collected performance data is cleared`() { + val data = mutableListOf(mock(), mock()) + val mockPerformanceCollector = object : TransactionPerformanceCollector { + override fun start(transaction: ITransaction) {} + override fun stop(transaction: ITransaction): MutableList = data + } + val transaction = fixture.getSut(optionsConfiguration = { + it.profilesSampleRate = 1.0 + }, performanceCollector = mockPerformanceCollector) + transaction.finish() + assertTrue(data.isEmpty()) + } }