Fix off-by-one error in tests which effectively disabled recycling va…

…lidation (#3357)

* Fix off-by-one error which effectively disabled recycling validation

* Disable recycling validation for profiler tests

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

apm-agent-core/src/test/java/co/elastic/apm/agent/MockTracer.java

@@ -26,6 +26,7 @@
 import co.elastic.apm.agent.report.ApmServerClient;
 import co.elastic.apm.agent.report.Reporter;
 import co.elastic.apm.agent.common.util.Version;
+import org.mockito.CheckReturnValue;
 import org.stagemonitor.configuration.ConfigurationRegistry;
 
 import static org.mockito.ArgumentMatchers.any;
@@ -51,11 +52,15 @@ public static ElasticApmTracer createRealTracer(Reporter reporter) {
         return createRealTracer(reporter, SpyConfiguration.createSpyConfig());
     }
 
+    public static ElasticApmTracer createRealTracer(Reporter reporter, ConfigurationRegistry config) {
+        return createRealTracer(reporter, config, true);
+    }
+
     /**
      * Creates a real tracer with a given reporter and a mock configuration which returns default values which can be customized by mocking
      * the configuration.
      */
-    public static ElasticApmTracer createRealTracer(Reporter reporter, ConfigurationRegistry config) {
+    public static ElasticApmTracer createRealTracer(Reporter reporter, ConfigurationRegistry config, boolean checkRecycling) {
 
         // use an object pool that does bookkeeping to allow for extra usage checks
         TestObjectPoolFactory objectPoolFactory = new TestObjectPoolFactory();
@@ -72,8 +77,10 @@ public static ElasticApmTracer createRealTracer(Reporter reporter, Configuration
                     ((MockReporter) reporter).assertRecycledAfterDecrementingReferences();
                 }
 
-                // checking proper object pool usage using tracer lifecycle events
-                objectPoolFactory.checkAllPooledObjectsHaveBeenRecycled();
+                if (checkRecycling) {
+                    // checking proper object pool usage using tracer lifecycle events
+                    objectPoolFactory.checkAllPooledObjectsHaveBeenRecycled();
+                }
             }))
             .build();
 
@@ -91,6 +98,10 @@ public static ElasticApmTracer createRealTracer(Reporter reporter, Configuration
      * values that can be customized by mocking the configuration.
      */
     public static synchronized MockInstrumentationSetup createMockInstrumentationSetup(ConfigurationRegistry configRegistry) {
+        return createMockInstrumentationSetup(configRegistry, true);
+    }
+
+    public static synchronized MockInstrumentationSetup createMockInstrumentationSetup(ConfigurationRegistry configRegistry, boolean checkRecycling) {
         // use an object pool that does bookkeeping to allow for extra usage checks
         TestObjectPoolFactory objectPoolFactory = new TestObjectPoolFactory();
 
@@ -106,9 +117,11 @@ public static synchronized MockInstrumentationSetup createMockInstrumentationSet
             // is left behind
             .withObjectPoolFactory(objectPoolFactory)
             .withLifecycleListener(ClosableLifecycleListenerAdapter.of(() -> {
-                reporter.assertRecycledAfterDecrementingReferences();
-                // checking proper object pool usage using tracer lifecycle events
-                objectPoolFactory.checkAllPooledObjectsHaveBeenRecycled();
+                if (checkRecycling) {
+                    reporter.assertRecycledAfterDecrementingReferences();
+                    // checking proper object pool usage using tracer lifecycle events
+                    objectPoolFactory.checkAllPooledObjectsHaveBeenRecycled();
+                }
             }))
             .buildAndStart();
         return new MockInstrumentationSetup(

apm-agent-core/src/test/java/co/elastic/apm/agent/objectpool/TestObjectPoolFactory.java

@@ -79,7 +79,7 @@ public void checkAllPooledObjectsHaveBeenRecycled() {
                     // silently ignored
                 }
             }
-        } while (retry-- > 0 && hasSomethingLeft);
+        } while (--retry > 0 && hasSomethingLeft);
 
         if (retry == 0 && hasSomethingLeft) {
             for (BookkeeperObjectPool<?> pool : createdPools) {

apm-agent-core/src/test/java/co/elastic/apm/agent/report/PartialTransactionTest.java

@@ -88,8 +88,8 @@ public void setup() throws MalformedURLException {
 
     @AfterEach
     public void cleanup() {
+        reporter.reset();
         objectPoolFactory.checkAllPooledObjectsHaveBeenRecycled();
-        reporter.resetWithoutRecycling();
         objectPoolFactory.reset();
     }
 

apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationInheritanceTest.java

@@ -22,6 +22,7 @@
 import co.elastic.apm.agent.MockTracer;
 import co.elastic.apm.agent.bci.ElasticApmAgent;
 import co.elastic.apm.agent.configuration.CoreConfiguration;
+import co.elastic.apm.agent.configuration.SpyConfiguration;
 import co.elastic.apm.agent.impl.ElasticApmTracer;
 import net.bytebuddy.agent.ByteBuddyAgent;
 import org.junit.jupiter.api.AfterAll;
@@ -49,7 +50,7 @@ void cleanup() {
     }
 
     private void init(boolean annotationInheritanceEnabled) {
-        MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup();
+        MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(SpyConfiguration.createSpyConfig(), false);
         tracer = mockInstrumentationSetup.getTracer();
         doReturn(annotationInheritanceEnabled).when(tracer.getConfig(CoreConfiguration.class)).isEnablePublicApiAnnotationInheritance();
         reporter = mockInstrumentationSetup.getReporter();

apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java

@@ -401,6 +401,7 @@ private void verifyTraceContextHeaders(Span span, String path) {
             assertThat(transaction).isNotNull();
             assertThat(transaction.getTraceContext().getTraceId()).isEqualTo(span.getTraceContext().getTraceId());
             assertThat(transaction.getTraceContext().getParentId()).isEqualTo(span.getTraceContext().getId());
+            transaction.decrementReferences(); //recycle transaction without reporting
         });
     }
 

apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/test/java/co/elastic/apm/agent/loginstr/correlation/CorrelationIdMapAdapterTest.java

@@ -57,7 +57,7 @@ void testTransactionContext() {
         try (Scope scope = transaction.activateInScope()) {
             assertThat(CorrelationIdMapAdapter.get()).containsOnlyKeys("trace.id", "transaction.id");
         } finally {
-            transaction.end();
+            transaction.decrementReferences(); //recycle without reporting
         }
         assertThat(CorrelationIdMapAdapter.get()).isEmpty();
     }
@@ -70,8 +70,9 @@ void testSpanContext() {
             assertThat(CorrelationIdMapAdapter.get()).containsOnlyKeys("trace.id", "transaction.id");
         } finally {
             span.end();
+            span.decrementReferences();
         }
-        transaction.end();
+        transaction.decrementReferences();
         assertThat(CorrelationIdMapAdapter.get()).isEmpty();
     }
 

apm-agent-plugins/apm-profiling-plugin/src/test/java/co/elastic/apm/agent/profiler/CallTreeSpanifyTest.java

@@ -53,8 +53,7 @@ void setUp() {
         ConfigurationRegistry config = SpyConfiguration.createSpyConfig();
         // disable scheduled profiling to not interfere with this test
         doReturn(false).when(config.getConfig(ProfilingConfiguration.class)).isProfilingEnabled();
-        tracer = MockTracer.createRealTracer(reporter, config);
-        tracer = MockTracer.createRealTracer(reporter);
+        tracer = MockTracer.createRealTracer(reporter, config, false);
     }
 
     @AfterEach

apm-agent-plugins/apm-profiling-plugin/src/test/java/co/elastic/apm/agent/profiler/CallTreeTest.java

@@ -73,7 +73,7 @@ void setUp() {
         // disable scheduled profiling to not interfere with this test
         profilerConfig = config.getConfig(ProfilingConfiguration.class);
         doReturn(true).when(profilerConfig).isProfilingEnabled();
-        tracer = MockTracer.createRealTracer(reporter, config);
+        tracer = MockTracer.createRealTracer(reporter, config, false);
     }
 
     @AfterEach