Fix transaction performance collector oom (#2505)

* moved synchronized() bit later in stop() method in DefaultTransactionPerformanceCollector
* PerformanceCollectionData now contains single values instead of lists and removed the commit() method
* 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

Author: stefanosiano

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))
 

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 <init> (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 <init> ()V
-	public fun collect (Ljava/lang/Iterable;)V
+	public fun collect (Lio/sentry/PerformanceCollectionData;)V
 	public fun setup ()V
 }
 

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> 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. */

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> 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);
   }
 }

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(

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> 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> 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> performanceCollectionData) {
     if (performanceCollectionData != null) {
       final @NotNull ArrayDeque<ProfileMeasurementValue> memoryUsageMeasurements =
-          new ArrayDeque<>();
+          new ArrayDeque<>(performanceCollectionData.size());
       final @NotNull ArrayDeque<ProfileMeasurementValue> nativeMemoryUsageMeasurements =
-          new ArrayDeque<>();
-      final @NotNull ArrayDeque<ProfileMeasurementValue> 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<ProfileMeasurementValue> 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())

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,23 +42,20 @@ class AndroidCpuCollectorTest {
     @Test
     fun `collect works only after setup`() {
         val data = PerformanceCollectionData()
-        fixture.getSut().collect(listOf(data))
-        data.commitData()
-        assertTrue(data.cpuData.isEmpty())
+        fixture.getSut().collect(data)
+        assertNull(data.cpuData)
     }
 
     @Test
     fun `when collect cpu is collected`() {
         val data = PerformanceCollectionData()
         val collector = fixture.getSut()
         collector.setup()
-        collector.collect(listOf(data))
-        data.commitData()
+        collector.collect(data)
         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 +65,7 @@ class AndroidCpuCollectorTest {
         whenever(mockBuildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.KITKAT)
         val collector = fixture.getSut(mockBuildInfoProvider)
         collector.setup()
-        collector.collect(listOf(data))
-        data.commitData()
-        assertTrue(data.cpuData.isEmpty())
+        collector.collect(data)
+        assertNull(data.cpuData)
     }
 }

sentry-android-core/src/test/java/io/sentry/android/core/AndroidMemoryCollectorTest.kt

@@ -18,13 +18,11 @@ 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()
+        val memoryData = data.memoryData
         assertNotNull(memoryData)
         assertNotEquals(-1, memoryData.usedNativeMemory)
         assertEquals(usedNativeMemory, memoryData.usedNativeMemory)

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<DefaultTransactionPerformanceCollector>(fixture.sentryOptions.transactionPerformanceCollector)
+    }
 }

sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt

@@ -376,14 +376,18 @@ 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<PerformanceCollectionData>()
+        var singleData = PerformanceCollectionData()
+        singleData.addMemoryData(MemoryCollectionData(1, 2, 3))
+        singleData.addCpuData(CpuCollectionData(1, 1.4))
+        performanceCollectionData.add(singleData)
+
+        singleData = PerformanceCollectionData()
+        singleData.addMemoryData(MemoryCollectionData(2, 3, 4))
+        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 }