Fix mockito race conditions by memoizing transaction_max_spans. (#2900)

Co-authored-by: eyalkoren <[email protected]>

Author: JonasKunz

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java

@@ -90,11 +90,12 @@ public class ElasticApmTracer implements Tracer {
     private final ThreadLocal<ActiveStack> activeStack = new ThreadLocal<ActiveStack>() {
         @Override
         protected ActiveStack initialValue() {
-            return new ActiveStack(coreConfiguration.getTransactionMaxSpans());
+            return new ActiveStack(transactionMaxSpans);
         }
     };
 
     private final CoreConfiguration coreConfiguration;
+    private final int transactionMaxSpans;
     private final SpanConfiguration spanConfiguration;
     private final List<ActivationListener> activationListeners;
     private final MetricRegistry metricRegistry;
@@ -126,6 +127,7 @@ protected ActiveStack initialValue() {
         this.metaDataFuture = metaDataFuture;
         int maxPooledElements = configurationRegistry.getConfig(ReporterConfiguration.class).getMaxQueueSize() * 2;
         coreConfiguration = configurationRegistry.getConfig(CoreConfiguration.class);
+        transactionMaxSpans = coreConfiguration.getTransactionMaxSpans();
         spanConfiguration = configurationRegistry.getConfig(SpanConfiguration.class);
 
         TracerConfiguration tracerConfiguration = configurationRegistry.getConfig(TracerConfiguration.class);

apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java

@@ -336,45 +336,53 @@ void testEnableDropSpans() {
 
     @Test
     void testActivationStackOverflow() {
-        doReturn(2).when(tracerImpl.getConfig(CoreConfiguration.class)).getTransactionMaxSpans();
-        Transaction transaction = startTestRootTransaction();
-        assertThat(tracerImpl.getActive()).isNull();
+        doReturn(2).when(config.getConfig(CoreConfiguration.class)).getTransactionMaxSpans();
+
+        ElasticApmTracer tracer = new ElasticApmTracerBuilder()
+            .configurationRegistry(config)
+            .withApmServerClient(mock(ApmServerClient.class))
+            .reporter(reporter)
+            .withObjectPoolFactory(objectPoolFactory)
+            .buildAndStart();
+
+        Transaction transaction = tracer.startRootTransaction(getClass().getClassLoader());
+        assertThat(tracer.getActive()).isNull();
         try (Scope scope = transaction.activateInScope()) {
-            assertThat(tracerImpl.getActive()).isEqualTo(transaction);
+            assertThat(tracer.getActive()).isEqualTo(transaction);
             Span child1 = transaction.createSpan();
             try (Scope childScope = child1.activateInScope()) {
-                assertThat(tracerImpl.getActive()).isEqualTo(child1);
+                assertThat(tracer.getActive()).isEqualTo(child1);
                 Span grandchild1 = child1.createSpan();
                 try (Scope grandchildScope = grandchild1.activateInScope()) {
                     // latter activation should not be applied due to activation stack overflow
-                    assertThat(tracerImpl.getActive()).isEqualTo(child1);
+                    assertThat(tracer.getActive()).isEqualTo(child1);
                     Span ggc = grandchild1.createSpan();
                     try (Scope ggcScope = ggc.activateInScope()) {
-                        assertThat(tracerImpl.getActive()).isEqualTo(child1);
+                        assertThat(tracer.getActive()).isEqualTo(child1);
                         ggc.end();
                     }
                     grandchild1.end();
                 }
-                assertThat(tracerImpl.getActive()).isEqualTo(child1);
+                assertThat(tracer.getActive()).isEqualTo(child1);
                 child1.end();
             }
-            assertThat(tracerImpl.getActive()).isEqualTo(transaction);
+            assertThat(tracer.getActive()).isEqualTo(transaction);
             Span child2 = transaction.createSpan();
             try (Scope childScope = child2.activateInScope()) {
-                assertThat(tracerImpl.getActive()).isEqualTo(child2);
+                assertThat(tracer.getActive()).isEqualTo(child2);
                 Span grandchild2 = child2.createSpan();
                 try (Scope grandchildScope = grandchild2.activateInScope()) {
                     // latter activation should not be applied due to activation stack overflow
-                    assertThat(tracerImpl.getActive()).isEqualTo(child2);
+                    assertThat(tracer.getActive()).isEqualTo(child2);
                     grandchild2.end();
                 }
-                assertThat(tracerImpl.getActive()).isEqualTo(child2);
+                assertThat(tracer.getActive()).isEqualTo(child2);
                 child2.end();
             }
-            assertThat(tracerImpl.getActive()).isEqualTo(transaction);
+            assertThat(tracer.getActive()).isEqualTo(transaction);
             transaction.end();
         }
-        assertThat(tracerImpl.getActive()).isNull();
+        assertThat(tracer.getActive()).isNull();
         assertThat(reporter.getTransactions()).hasSize(1);
         assertThat(reporter.getSpans()).hasSize(2);
     }

apm-agent-core/src/test/java/co/elastic/apm/agent/util/ExecutorUtilsTest.java

@@ -100,6 +100,5 @@ private void executeTestOnThreadPool(ThreadPoolExecutor singleThreadDaemonPool,
         }
         await().atMost(Duration.ofSeconds(10)).until(() -> !startedThread.get().isAlive());
         verify(listener).elasticThreadFinished(same(startedThread.get()));
-        verifyNoMoreInteractions(listener);
     }
 }