Use a single transactionPerfomanceCollector (#2464)

* the TransactionPerformanceCollector instance is now put in the options, instead of being created in every Hub
* TransactionPerformanceCollector reads memoryCollector on start() method, to allow option customization to be performed before reading it

CHANGELOG.md

@@ -11,6 +11,7 @@
 
 ### Fixes
 
+- Use a single TransactionPerfomanceCollector ([#2464](https://github.com/getsentry/sentry-java/pull/2464))
 - Don't override sdk name with Timber ([#2450](https://github.com/getsentry/sentry-java/pull/2450))
 - Set transactionNameSource to CUSTOM when setting transaction name ([#2405](https://github.com/getsentry/sentry-java/pull/2405))
 

sentry/api/sentry.api

@@ -1466,6 +1466,7 @@ public class io/sentry/SentryOptions {
 	public fun getTracesSampleRate ()Ljava/lang/Double;
 	public fun getTracesSampler ()Lio/sentry/SentryOptions$TracesSamplerCallback;
 	public fun getTracingOrigins ()Ljava/util/List;
+	public fun getTransactionPerformanceCollector ()Lio/sentry/TransactionPerformanceCollector;
 	public fun getTransactionProfiler ()Lio/sentry/ITransactionProfiler;
 	public fun getTransportFactory ()Lio/sentry/ITransportFactory;
 	public fun getTransportGate ()Lio/sentry/transport/ITransportGate;

sentry/src/main/java/io/sentry/Hub.java

@@ -45,7 +45,7 @@ private Hub(final @NotNull SentryOptions options, final @NotNull Stack stack) {
     this.tracesSampler = new TracesSampler(options);
     this.stack = stack;
     this.lastEventId = SentryId.EMPTY_ID;
-    this.transactionPerformanceCollector = new TransactionPerformanceCollector(options);
+    this.transactionPerformanceCollector = options.getTransactionPerformanceCollector();
 
     // Integrations will use this Hub instance once registered.
     // Make sure Hub ready to be used then.

sentry/src/main/java/io/sentry/SentryOptions.java

@@ -387,6 +387,10 @@ public class SentryOptions {
 
   private @NotNull IMemoryCollector memoryCollector = NoOpMemoryCollector.getInstance();
 
+  /** Performance collector that collect performance stats while transactions run. */
+  private final @NotNull TransactionPerformanceCollector transactionPerformanceCollector =
+      new TransactionPerformanceCollector(this);
+
   // TODO this should default to false on the next major
   /** Whether OPTIONS requests should be traced. */
   private boolean traceOptionsRequests = true;
@@ -1883,7 +1887,17 @@ public void setMainThreadChecker(final @NotNull IMainThreadChecker mainThreadChe
   }
 
   /**
-   * Gets the memory collector used to collect memory usage during while transaction runs.
+   * Gets the performance collector used to collect performance stats while transactions run.
+   *
+   * @return the performance collector.
+   */
+  @ApiStatus.Internal
+  public @NotNull TransactionPerformanceCollector getTransactionPerformanceCollector() {
+    return transactionPerformanceCollector;
+  }
+
+  /**
+   * Gets the memory collector used to collect memory usage while transactions run.
    *
    * @return the memory collector.
    */

sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java

@@ -21,17 +21,21 @@ public final class TransactionPerformanceCollector {
   private volatile @NotNull Timer timer = new Timer();
   private final @NotNull Map<String, ArrayList<MemoryCollectionData>> memoryMap =
       new ConcurrentHashMap<>();
-  private final @NotNull IMemoryCollector memoryCollector;
+  private @Nullable IMemoryCollector memoryCollector = null;
   private final @NotNull SentryOptions options;
   private final @NotNull AtomicBoolean isStarted = new AtomicBoolean(false);
 
   public TransactionPerformanceCollector(final @NotNull SentryOptions options) {
     this.options = Objects.requireNonNull(options, "The options object is required.");
-    this.memoryCollector = options.getMemoryCollector();
   }
 
   @SuppressWarnings("FutureReturnValueIgnored")
   public void start(final @NotNull ITransaction transaction) {
+    // We are putting the TransactionPerformanceCollector in the options, so we want to wait until
+    // the options are customized before reading the memory collector
+    if (memoryCollector == null) {
+      this.memoryCollector = options.getMemoryCollector();
+    }
     if (memoryCollector instanceof NoOpMemoryCollector) {
       options
           .getLogger()
@@ -60,11 +64,13 @@ public void start(final @NotNull ITransaction transaction) {
             new TimerTask() {
               @Override
               public void run() {
-                MemoryCollectionData memoryData = memoryCollector.collect();
-                if (memoryData != null) {
-                  synchronized (timerLock) {
-                    for (ArrayList<MemoryCollectionData> list : memoryMap.values()) {
-                      list.add(memoryData);
+                if (memoryCollector != null) {
+                  MemoryCollectionData memoryData = memoryCollector.collect();
+                  if (memoryData != null) {
+                    synchronized (timerLock) {
+                      for (ArrayList<MemoryCollectionData> list : memoryMap.values()) {
+                        list.add(memoryData);
+                      }
                     }
                   }
                 }

sentry/src/test/java/io/sentry/SentryOptionsTest.kt

@@ -7,6 +7,7 @@ import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
+import kotlin.test.assertIs
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
@@ -254,6 +255,11 @@ class SentryOptionsTest {
         assertEquals(0.2, options.profilesSampleRate)
     }
 
+    @Test
+    fun `when options is initialized, transactionPerformanceCollector is set`() {
+        assertIs<TransactionPerformanceCollector>(SentryOptions().transactionPerformanceCollector)
+    }
+
     @Test
     fun `when options is initialized, transactionProfiler is noop`() {
         assert(SentryOptions().transactionProfiler == NoOpTransactionProfiler.getInstance())

sentry/src/test/java/io/sentry/TransactionPerformanceCollectorTest.kt

@@ -18,6 +18,7 @@ import kotlin.test.Test
 import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertIs
+import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 
 class TransactionPerformanceCollectorTest {
@@ -141,4 +142,12 @@ class TransactionPerformanceCollectorTest {
         val data1 = collector.stop(fixture.transaction1)
         assertFalse(data1!!.isEmpty())
     }
+
+    @Test
+    fun `collector reads MemoryCollector on start`() {
+        val collector = fixture.getSut()
+        assertNull(collector.getProperty<IMemoryCollector?>("memoryCollector"))
+        collector.start(fixture.transaction1)
+        assertNotNull(collector.getProperty<IMemoryCollector>("memoryCollector"))
+    }
 }