diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java index 6f44750478..9ba097978d 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java @@ -205,7 +205,7 @@ private static synchronized void initInstrumentation(final ElasticApmTracer trac if (!tracer.getConfig(CoreConfiguration.class).isEnabled()) { return; } - GlobalTracer.set(tracer); + GlobalTracer.init(tracer); for (ElasticApmInstrumentation apmInstrumentation : instrumentations) { pluginClassLoaderByAdviceClass.put( apmInstrumentation.getAdviceClass().getName(), @@ -520,6 +520,8 @@ public static synchronized void reset() { if (instrumentation == null) { return; } + GlobalTracer.get().stop(); + GlobalTracer.setNoop(); Exception exception = null; if (resettableClassFileTransformer != null) { try { diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/GlobalTracer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/GlobalTracer.java index 0fadd7633f..2418c283c3 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/GlobalTracer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/GlobalTracer.java @@ -31,16 +31,12 @@ import co.elastic.apm.agent.impl.transaction.Span; import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; import co.elastic.apm.agent.impl.transaction.Transaction; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import javax.annotation.Nullable; import java.util.Objects; public class GlobalTracer implements Tracer { - private static final Logger logger = LoggerFactory.getLogger(GlobalTracer.class); - private static final GlobalTracer INSTANCE = new GlobalTracer(); private volatile Tracer tracer = NoopTracer.INSTANCE; @@ -64,20 +60,25 @@ public static ElasticApmTracer requireTracerImpl() { return Objects.requireNonNull(getTracerImpl(), "Registered tracer is not an instance of ElasticApmTracer"); } - public static void setNoop() { - set(NoopTracer.INSTANCE); - } - - public static void set(Tracer tracer) { + public static synchronized void setNoop() { TracerState currentTracerState = INSTANCE.tracer.getState(); if (currentTracerState != TracerState.UNINITIALIZED && currentTracerState != TracerState.STOPPED) { - logger.warn("Overriding running tracer"); - // TODO throw exception, requires changes in tests - // throw new IllegalStateException("Can't override tracer as current tracer is already running"); + throw new IllegalStateException("Can't override tracer as current tracer is already running"); + } + INSTANCE.tracer = NoopTracer.INSTANCE; + } + + public static synchronized void init(Tracer tracer) { + if (!isNoop()) { + throw new IllegalStateException("Tracer is already initialized"); } INSTANCE.tracer = tracer; } + public static boolean isNoop() { + return INSTANCE.tracer == NoopTracer.INSTANCE; + } + @Nullable public Transaction startRootTransaction(@Nullable ClassLoader initiatingClassLoader) { return tracer.startRootTransaction(initiatingClassLoader); diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Destination.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Destination.java index d8a88679db..8cc6b5fd48 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Destination.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Destination.java @@ -143,7 +143,7 @@ public boolean hasContent() { @Override public void resetState() { address.setLength(0); - port = -1; + port = 0; service.resetState(); } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java index b81e313358..a83d311021 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java @@ -26,19 +26,17 @@ import co.elastic.apm.agent.bci.ElasticApmAgent; import co.elastic.apm.agent.configuration.SpyConfiguration; -import co.elastic.apm.agent.impl.Tracer; import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.impl.Tracer; import co.elastic.apm.agent.impl.TracerInternalApiUtils; import co.elastic.apm.agent.objectpool.TestObjectPoolFactory; import net.bytebuddy.agent.ByteBuddyAgent; import org.junit.After; import org.junit.AfterClass; -import org.junit.Before; import org.junit.BeforeClass; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; import org.stagemonitor.configuration.ConfigurationRegistry; import static org.assertj.core.api.Assertions.assertThat; @@ -48,11 +46,12 @@ public abstract class AbstractInstrumentationTest { protected static MockReporter reporter; protected static ConfigurationRegistry config; protected static TestObjectPoolFactory objectPoolFactory; + private boolean validateRecycling = true; @BeforeAll @BeforeClass public static synchronized void beforeAll() { - MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.getOrCreateInstrumentationTracer(); + MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(); tracer = mockInstrumentationSetup.getTracer(); config = mockInstrumentationSetup.getConfig(); objectPoolFactory = mockInstrumentationSetup.getObjectPoolFactory(); @@ -67,15 +66,8 @@ public static synchronized void afterAll() { ElasticApmAgent.reset(); } - public static synchronized void staticReset() { - SpyConfiguration.reset(config); - reporter.reset(); - - // resume tracer in case it has been paused - // otherwise the 1st test that pauses tracer will have side effects on others - if (!tracer.isRunning()) { - TracerInternalApiUtils.resumeTracer(tracer); - } + protected void disableRecyclingValidation() { + validateRecycling = false; } public static Tracer getTracer() { @@ -90,15 +82,25 @@ public static ConfigurationRegistry getConfig() { return config; } - @Before - @BeforeEach - public final void reset() { - staticReset(); - } - @After @AfterEach public final void cleanUp() { + SpyConfiguration.reset(config); + try { + if (validateRecycling) { + reporter.assertRecycledAfterDecrementingReferences(); + objectPoolFactory.checkAllPooledObjectsHaveBeenRecycled(); + } + } finally { + // one faulty test should not affect others + reporter.resetWithoutRecycling(); + objectPoolFactory.reset(); + // resume tracer in case it has been paused + // otherwise the 1st test that pauses tracer will have side effects on others + if (!tracer.isRunning()) { + TracerInternalApiUtils.resumeTracer(tracer); + } + } tracer.resetServiceNameOverrides(); assertThat(tracer.getActive()) diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/MockReporter.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/MockReporter.java index d9da96469b..b3e78290b1 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/MockReporter.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/MockReporter.java @@ -42,7 +42,6 @@ import com.networknt.schema.JsonSchema; import com.networknt.schema.JsonSchemaFactory; import com.networknt.schema.ValidationMessage; -import org.awaitility.core.ConditionFactory; import org.awaitility.core.ThrowingRunnable; import java.io.IOException; @@ -56,10 +55,10 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.locks.LockSupport; import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; -import static org.awaitility.Awaitility.await; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -197,8 +196,7 @@ public synchronized Transaction getFirstTransaction() { } public Transaction getFirstTransaction(long timeoutMs) { - awaitTimeout(timeoutMs) - .untilAsserted(() -> assertThat(getTransactions()).isNotEmpty()); + awaitUntilAsserted(timeoutMs, () -> assertThat(getTransactions()).isNotEmpty()); return getFirstTransaction(); } @@ -209,24 +207,11 @@ public void assertNoTransaction() { } public void assertNoTransaction(long timeoutMs) { - awaitTimeout(timeoutMs) - .untilAsserted(this::assertNoTransaction); - } - - public void awaitUntilAsserted(long timeoutMs, ThrowingRunnable assertion){ - awaitTimeout(timeoutMs) - .untilAsserted(assertion); - } - - private static ConditionFactory awaitTimeout(long timeoutMs) { - return await() - .pollInterval(1, TimeUnit.MILLISECONDS) - .timeout(timeoutMs, TimeUnit.MILLISECONDS); + awaitUntilAsserted(timeoutMs, this::assertNoTransaction); } public Span getFirstSpan(long timeoutMs) { - awaitTimeout(timeoutMs) - .untilAsserted(() -> assertThat(getSpans()).isNotEmpty()); + awaitUntilAsserted(timeoutMs, () -> assertThat(getSpans()).isNotEmpty()); return getFirstSpan(); } @@ -237,30 +222,17 @@ public void assertNoSpan() { } public void assertNoSpan(long timeoutMs) { - awaitTimeout(timeoutMs) - .untilAsserted(() -> assertThat(getSpans()).isEmpty()); + awaitUntilAsserted(timeoutMs, () -> assertThat(getSpans()).isEmpty()); assertNoSpan(); } public void awaitTransactionCount(int count) { - awaitTimeout(1000) - .untilAsserted(() -> assertThat(getNumReportedTransactions()).isEqualTo(count)); - } - - public void awaitTransactionReported() { - awaitTimeout(1000) - .untilAsserted(() -> assertThat(getNumReportedTransactions()).isGreaterThan(0)); + awaitUntilAsserted(() -> assertThat(getNumReportedTransactions()).isEqualTo(count)); } public void awaitSpanCount(int count) { - awaitTimeout(1000) - .untilAsserted(() -> assertThat(getNumReportedSpans()).isEqualTo(count)); - } - - public void awaitSpanReported() { - awaitTimeout(1000) - .untilAsserted(() -> assertThat(getNumReportedSpans()).isGreaterThan(0)); + awaitUntilAsserted(() -> assertThat(getNumReportedSpans()).isEqualTo(count)); } @Override @@ -349,6 +321,11 @@ public synchronized void close() { } public synchronized void reset() { + assertRecycledAfterDecrementingReferences(); + resetWithoutRecycling(); + } + + public synchronized void resetWithoutRecycling() { transactions.clear(); spans.clear(); errors.clear(); @@ -383,33 +360,62 @@ public synchronized void assertRecycledAfterDecrementingReferences() { transactionsToFlush.forEach(Transaction::decrementReferences); spansToFlush.forEach(Span::decrementReferences); - // transactions might be active after they have already been reported - // after a short amount of time, all transactions and spans should have been recycled - await() - .timeout(1, TimeUnit.SECONDS) - .untilAsserted(() -> transactions.forEach(t -> { - assertThat(hasEmptyTraceContext(t)) - .describedAs("should have empty trace context : %s", t) - .isTrue(); - assertThat(t.isReferenced()) - .describedAs("should not have any reference left, but has %d : %s", t.getReferenceCount(), t) + awaitUntilAsserted(() -> { + spans.forEach(s -> { + assertThat(s.isReferenced()) + .describedAs("should not have any reference left, but has %d : %s", s.getReferenceCount(), s) .isFalse(); - })); - await() - .timeout(1, TimeUnit.SECONDS) - .untilAsserted(() -> spans.forEach(s -> { assertThat(hasEmptyTraceContext(s)) .describedAs("should have empty trace context : %s", s) .isTrue(); - assertThat(s.isReferenced()) - .describedAs("should not have any reference left, but has %d : %s", s.getReferenceCount(), s) + }); + transactions.forEach(t -> { + assertThat(t.isReferenced()) + .describedAs("should not have any reference left, but has %d : %s", t.getReferenceCount(), t) .isFalse(); - })); + assertThat(hasEmptyTraceContext(t)) + .describedAs("should have empty trace context : %s", t) + .isTrue(); + }); + }); // errors are recycled directly because they have no reference counter errors.forEach(ErrorCapture::recycle); } + /** + * Uses a timeout of 1s + * + * @see #awaitUntilAsserted(long, ThrowingRunnable) + */ + public void awaitUntilAsserted(ThrowingRunnable runnable) { + awaitUntilAsserted(1000, runnable); + } + + /** + * This is deliberately not using {@link org.awaitility.Awaitility} as it uses an {@link java.util.concurrent.Executor} for polling. + * This is an issue when testing instrumentations that instrument {@link java.util.concurrent.Executor}. + * + * @param timeoutMs the timeout of the condition + * @param runnable a runnable that trows an exception if the condition is not met + */ + public void awaitUntilAsserted(long timeoutMs, ThrowingRunnable runnable) { + Throwable thrown = null; + for (int i = 0; i < timeoutMs; i += 5) { + try { + runnable.run(); + thrown = null; + } catch (Throwable e) { + thrown = e; + LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(5)); + } + } + if (thrown != null) { + throw new RuntimeException(String.format("Condition not fulfilled within %d ms", timeoutMs), thrown); + } + } + + private static boolean hasEmptyTraceContext(AbstractSpan item) { return item.getTraceContext().getId().isEmpty(); } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/MockTracer.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/MockTracer.java index 345c089fbe..58dc40c209 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/MockTracer.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/MockTracer.java @@ -24,14 +24,10 @@ */ package co.elastic.apm.agent; -import co.elastic.apm.agent.bci.ElasticApmAgent; import co.elastic.apm.agent.configuration.SpyConfiguration; import co.elastic.apm.agent.context.ClosableLifecycleListenerAdapter; -import co.elastic.apm.agent.impl.ElasticApmTracerBuilder; import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.impl.GlobalTracer; -import co.elastic.apm.agent.impl.Tracer; -import co.elastic.apm.agent.impl.TracerInternalApiUtils; +import co.elastic.apm.agent.impl.ElasticApmTracerBuilder; import co.elastic.apm.agent.objectpool.TestObjectPoolFactory; import co.elastic.apm.agent.report.Reporter; import org.stagemonitor.configuration.ConfigurationRegistry; @@ -89,58 +85,35 @@ public static ElasticApmTracer createRealTracer(Reporter reporter, Configuration } /** - * If an instrumentation has already been initialized by some other test, returns the static - * {@link GlobalTracer#getTracerImpl()}. - * Otherwise, Creates a real tracer with a {@link MockReporter} and a mock configuration which returns default + * Creates a real tracer with a {@link MockReporter} and a mock configuration which returns default * values that can be customized by mocking the configuration. */ - public static synchronized MockInstrumentationSetup getOrCreateInstrumentationTracer() { - - ElasticApmTracer tracer = GlobalTracer.getTracerImpl(); - if (tracer == null || tracer.getState() == Tracer.TracerState.STOPPED || - !(tracer.getReporter() instanceof MockReporter) || !(tracer.getObjectPoolFactory() instanceof TestObjectPoolFactory)) { - - // use an object pool that does bookkeeping to allow for extra usage checks - TestObjectPoolFactory objectPoolFactory = new TestObjectPoolFactory(); - - MockReporter reporter = new MockReporter(); - - tracer = new ElasticApmTracerBuilder() - .configurationRegistry(SpyConfiguration.createSpyConfig()) - .reporter(reporter) - // use testing bookkeeper implementation here so we will check that no forgotten recyclable object - // is left behind - .withObjectPoolFactory(objectPoolFactory) - .withLifecycleListener(ClosableLifecycleListenerAdapter.of(() -> { - reporter.assertRecycledAfterDecrementingReferences(); - // checking proper object pool usage using tracer lifecycle events - objectPoolFactory.checkAllPooledObjectsHaveBeenRecycled(); - })) - .buildAndStart(); - } else { - ElasticApmAgent.reset(); - if (!tracer.isRunning()) { - TracerInternalApiUtils.resumeTracer(tracer); - } - ((MockReporter) tracer.getReporter()).reset(); - SpyConfiguration.reset(tracer.getConfigurationRegistry()); - } + public static synchronized MockInstrumentationSetup createMockInstrumentationSetup() { + // use an object pool that does bookkeeping to allow for extra usage checks + TestObjectPoolFactory objectPoolFactory = new TestObjectPoolFactory(); + + MockReporter reporter = new MockReporter(); + + ElasticApmTracer tracer = new ElasticApmTracerBuilder() + .configurationRegistry(SpyConfiguration.createSpyConfig()) + .reporter(reporter) + // use testing bookkeeper implementation here so we will check that no forgotten recyclable object + // is left behind + .withObjectPoolFactory(objectPoolFactory) + .withLifecycleListener(ClosableLifecycleListenerAdapter.of(() -> { + reporter.assertRecycledAfterDecrementingReferences(); + // checking proper object pool usage using tracer lifecycle events + objectPoolFactory.checkAllPooledObjectsHaveBeenRecycled(); + })) + .buildAndStart(); return new MockInstrumentationSetup( tracer, - (MockReporter) tracer.getReporter(), + reporter, tracer.getConfigurationRegistry(), - (TestObjectPoolFactory) tracer.getObjectPoolFactory() + objectPoolFactory ); } - public static synchronized void resetTracer() { - ElasticApmTracer tracer = GlobalTracer.getTracerImpl(); - if (tracer != null) { - ElasticApmAgent.reset(); - tracer.stop(); - } - } - /** * Like {@link #create(ConfigurationRegistry)} but with a {@link SpyConfiguration#createSpyConfig() mocked ConfigurationRegistry}. * diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java index cfda9cb946..7bfc5c2fef 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java @@ -27,7 +27,6 @@ import co.elastic.apm.agent.MockTracer; import co.elastic.apm.agent.bci.subpackage.AdviceInSubpackageInstrumentation; import co.elastic.apm.agent.configuration.CoreConfiguration; -import co.elastic.apm.agent.configuration.SpyConfiguration; import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.impl.transaction.AbstractSpan; import co.elastic.apm.agent.impl.transaction.Span; @@ -90,14 +89,14 @@ class InstrumentationTest { @BeforeEach void setup() { - configurationRegistry = SpyConfiguration.createSpyConfig(); + tracer = MockTracer.createRealTracer(); + configurationRegistry = tracer.getConfigurationRegistry(); coreConfig = configurationRegistry.getConfig(CoreConfiguration.class); - tracer = MockTracer.create(configurationRegistry); } @AfterEach void reset() { - MockTracer.resetTracer(); + ElasticApmAgent.reset(); } @Test diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/methodmatching/TraceMethodInstrumentationTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/methodmatching/TraceMethodInstrumentationTest.java index 98ddde930a..b8ea911d14 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/methodmatching/TraceMethodInstrumentationTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/methodmatching/TraceMethodInstrumentationTest.java @@ -65,7 +65,7 @@ class TraceMethodInstrumentationTest { @BeforeEach void setUp(TestInfo testInfo) { - MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.getOrCreateInstrumentationTracer(); + MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(); reporter = mockInstrumentationSetup.getReporter(); objectPoolFactory = mockInstrumentationSetup.getObjectPoolFactory(); ConfigurationRegistry config = mockInstrumentationSetup.getConfig(); diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/objectpool/TestObjectPoolFactory.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/objectpool/TestObjectPoolFactory.java index 0eaa2ad1ff..aae6a48fdd 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/objectpool/TestObjectPoolFactory.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/objectpool/TestObjectPoolFactory.java @@ -69,6 +69,10 @@ public void checkAllPooledObjectsHaveBeenRecycled() { } } + public void reset() { + createdPools.forEach(BookkeeperObjectPool::reset); + } + @Override public ObjectPool createTransactionPool(int maxCapacity, ElasticApmTracer tracer) { transactionPool = (BookkeeperObjectPool) super.createTransactionPool(maxCapacity, tracer); diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/objectpool/impl/BookkeeperObjectPool.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/objectpool/impl/BookkeeperObjectPool.java index 669e550452..c026c3637e 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/objectpool/impl/BookkeeperObjectPool.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/objectpool/impl/BookkeeperObjectPool.java @@ -115,4 +115,9 @@ public Collection getRecyclablesToReturn() { public int getRequestedObjectCount() { return objectCounter.get(); } + + public void reset() { + objectCounter.set(0); + toReturn.clear(); + } } diff --git a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/state/GlobalThreadLocal.java b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/state/GlobalThreadLocal.java index 2ac3960e03..8a6afbd88a 100644 --- a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/state/GlobalThreadLocal.java +++ b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/state/GlobalThreadLocal.java @@ -74,6 +74,14 @@ public T getAndRemove() { return value; } + public T get(T defaultValue) { + T value = get(); + if (value != null) { + return value; + } + return defaultValue; + } + @Override @Nullable protected T initialValue(Thread thread) { diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/plugin/api/SpanInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/plugin/api/SpanInstrumentationTest.java index f9db33a377..eaf8fc6450 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/plugin/api/SpanInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/plugin/api/SpanInstrumentationTest.java @@ -26,18 +26,16 @@ import co.elastic.apm.agent.AbstractInstrumentationTest; import co.elastic.apm.agent.impl.TextHeaderMapAccessor; -import co.elastic.apm.agent.impl.transaction.TextHeaderGetter; import co.elastic.apm.agent.impl.transaction.TraceContext; import co.elastic.apm.api.ElasticApm; import co.elastic.apm.api.Scope; import co.elastic.apm.api.Span; import co.elastic.apm.api.Transaction; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import javax.annotation.Nullable; import java.util.HashMap; -import java.util.List; import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; @@ -51,6 +49,11 @@ void setUp() { transaction = ElasticApm.startTransaction(); } + @AfterEach + void tearDown() { + transaction.end(); + } + @Test void testSetName() { Span span = transaction.startSpan(); @@ -118,7 +121,10 @@ void testSampled() { assertThat(ElasticApm.currentTransaction().isSampled()).isFalse(); final Transaction transaction = ElasticApm.startTransaction(); assertThat(transaction.isSampled()).isTrue(); - assertThat(transaction.startSpan().isSampled()).isTrue(); + Span span = transaction.startSpan(); + assertThat(span.isSampled()).isTrue(); + span.end(); + transaction.end(); } @Test @@ -132,6 +138,7 @@ void testTraceHeaders() { Span span = transaction.startSpan(); assertContainsTracingHeaders(span); assertContainsTracingHeaders(transaction); + span.end(); } private void assertContainsNoTracingHeaders(Span span) { diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationApiTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationApiTest.java index 82b40b77f7..99154728b7 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationApiTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationApiTest.java @@ -99,7 +99,6 @@ private void testTransactionAndSpanTypes(boolean useLegacyTyping) { @Test void testMissingSubtype() { - reporter.reset(); AnnotationTestClass.transactionForMissingSpanSubtype(); assertThat(reporter.getSpans()).hasSize(1); Span internalSpan = reporter.getFirstSpan(); diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/BlockingQueueContextPropagationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/BlockingQueueContextPropagationTest.java index db75762f23..5dfcc782d2 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/BlockingQueueContextPropagationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/BlockingQueueContextPropagationTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -25,7 +25,6 @@ package co.elastic.apm.api; import co.elastic.apm.agent.AbstractInstrumentationTest; -import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -79,11 +78,6 @@ public static void tearDown() { executorService.shutdownNow(); } - @After - public void after() { - reporter.reset(); - } - @Test public void testAsyncTransactionDelegation() throws ExecutionException, InterruptedException { Transaction transaction = ElasticApm.startTransaction(); diff --git a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java index 237c84720b..9a5d644886 100644 --- a/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java +++ b/apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/ElasticApmApiInstrumentationTest.java @@ -41,8 +41,10 @@ class ElasticApmApiInstrumentationTest extends AbstractInstrumentationTest { @Test void testCreateTransaction() { - assertThat(ElasticApm.startTransaction()).isNotSameAs(NoopTransaction.INSTANCE); + Transaction transaction = ElasticApm.startTransaction(); + assertThat(transaction).isNotSameAs(NoopTransaction.INSTANCE); assertThat(ElasticApm.currentTransaction()).isSameAs(NoopTransaction.INSTANCE); + transaction.end(); } @Test @@ -52,14 +54,22 @@ void testNoCurrentTransaction() { @Test void testLegacyTransactionCreateSpan() { - assertThat(ElasticApm.startTransaction().createSpan()).isNotSameAs(NoopSpan.INSTANCE); + Transaction transaction = ElasticApm.startTransaction(); + Span span = transaction.createSpan(); + assertThat(span).isNotSameAs(NoopSpan.INSTANCE); assertThat(ElasticApm.currentSpan()).isSameAs(NoopSpan.INSTANCE); + span.end(); + transaction.end(); } @Test void testStartSpan() { - assertThat(ElasticApm.startTransaction().startSpan()).isNotSameAs(NoopSpan.INSTANCE); + Transaction transaction = ElasticApm.startTransaction(); + Span span = transaction.startSpan(); + assertThat(span).isNotSameAs(NoopSpan.INSTANCE); assertThat(ElasticApm.currentSpan()).isSameAs(NoopSpan.INSTANCE); + span.end(); + transaction.end(); } @Test @@ -242,6 +252,7 @@ void testEnsureParentId() { assertThat(tracer.currentTransaction().getTraceContext().getParentId().toString()).isEqualTo(rumTransactionId); assertThat(transaction.ensureParentId()).isEqualTo(rumTransactionId); } + transaction.end(); } @Test @@ -252,6 +263,7 @@ void testTransactionWithRemoteParentFunction() { parent.propagateTraceContext(headerMap, TextHeaderMapAccessor.INSTANCE); ElasticApm.startTransactionWithRemoteParent(headerMap::get).end(); assertThat(reporter.getFirstTransaction().isChildOf(parent)).isTrue(); + parent.end(); } @Test @@ -262,6 +274,7 @@ void testTransactionWithRemoteParentFunctions() { parent.propagateTraceContext(headerMap, TextHeaderMapAccessor.INSTANCE); ElasticApm.startTransactionWithRemoteParent(headerMap::get, key -> Collections.singletonList(headerMap.get(key))).end(); assertThat(reporter.getFirstTransaction().isChildOf(parent)).isTrue(); + parent.end(); } @Test @@ -272,6 +285,7 @@ void testTransactionWithRemoteParentHeaders() { parent.propagateTraceContext(headerMap, TextHeaderMapAccessor.INSTANCE); ElasticApm.startTransactionWithRemoteParent(null, key -> Collections.singletonList(headerMap.get(key))).end(); assertThat(reporter.getFirstTransaction().isChildOf(parent)).isTrue(); + parent.end(); } @Test diff --git a/apm-agent-plugins/apm-dubbo-plugin/src/test/java/co/elastic/apm/agent/dubbo/api/impl/DubboTestApiImpl.java b/apm-agent-plugins/apm-dubbo-plugin/src/test/java/co/elastic/apm/agent/dubbo/api/impl/DubboTestApiImpl.java index ad4d2420e6..daed12f1d0 100644 --- a/apm-agent-plugins/apm-dubbo-plugin/src/test/java/co/elastic/apm/agent/dubbo/api/impl/DubboTestApiImpl.java +++ b/apm-agent-plugins/apm-dubbo-plugin/src/test/java/co/elastic/apm/agent/dubbo/api/impl/DubboTestApiImpl.java @@ -26,15 +26,14 @@ import co.elastic.apm.agent.dubbo.api.DubboTestApi; import co.elastic.apm.agent.dubbo.api.exception.BizException; +import co.elastic.apm.agent.impl.GlobalTracer; import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; import okhttp3.OkHttpClient; -import okhttp3.Request; import org.apache.dubbo.rpc.AsyncContext; import org.apache.dubbo.rpc.RpcContext; -import java.io.IOException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.concurrent.Executors; @@ -135,21 +134,10 @@ public void run() { return null; } - private void invokeHttp() { - Request request = new Request.Builder() - .url("http://localhost:" + server.port() + "/") - .build(); - try { - client.newCall(request).execute().body().close(); - } catch (IOException e) { - - } - } - private void doSomething() { try { Thread.sleep(10); - invokeHttp(); + GlobalTracer.get().getActive().createSpan().withName("doSomething").end(); } catch (InterruptedException e) { } } diff --git a/apm-agent-plugins/apm-error-logging-plugin/src/test/java/co/elastic/apm/agent/error/logging/AbstractErrorLoggingInstrumentationTest.java b/apm-agent-plugins/apm-error-logging-plugin/src/test/java/co/elastic/apm/agent/error/logging/AbstractErrorLoggingInstrumentationTest.java index b70197b33a..041e69d659 100644 --- a/apm-agent-plugins/apm-error-logging-plugin/src/test/java/co/elastic/apm/agent/error/logging/AbstractErrorLoggingInstrumentationTest.java +++ b/apm-agent-plugins/apm-error-logging-plugin/src/test/java/co/elastic/apm/agent/error/logging/AbstractErrorLoggingInstrumentationTest.java @@ -33,19 +33,17 @@ abstract class AbstractErrorLoggingInstrumentationTest extends AbstractInstrumentationTest { + private Transaction transaction; + @BeforeEach void startTransaction() { - reporter.reset(); - tracer.startRootTransaction(null).activate(); + transaction = tracer.startRootTransaction(null); + transaction.activate(); } @AfterEach void endTransaction() { - Transaction currentTransaction = tracer.currentTransaction(); - if (currentTransaction != null) { - currentTransaction.deactivate().end(); - } - reporter.reset(); + transaction.deactivate().end(); } void verifyThatExceptionCaptured(int errorCount, String exceptionMessage, Class exceptionClass) { diff --git a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/test/java/co/elastic/apm/agent/es/restclient/AbstractEsClientInstrumentationTest.java b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/test/java/co/elastic/apm/agent/es/restclient/AbstractEsClientInstrumentationTest.java index f7128d5ce1..c5df95e0d4 100644 --- a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/test/java/co/elastic/apm/agent/es/restclient/AbstractEsClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/test/java/co/elastic/apm/agent/es/restclient/AbstractEsClientInstrumentationTest.java @@ -25,10 +25,10 @@ package co.elastic.apm.agent.es.restclient; import co.elastic.apm.agent.AbstractInstrumentationTest; -import co.elastic.apm.agent.impl.context.Destination; -import co.elastic.apm.agent.impl.error.ErrorCapture; import co.elastic.apm.agent.impl.context.Db; +import co.elastic.apm.agent.impl.context.Destination; import co.elastic.apm.agent.impl.context.Http; +import co.elastic.apm.agent.impl.error.ErrorCapture; import co.elastic.apm.agent.impl.transaction.Span; import co.elastic.apm.agent.impl.transaction.Transaction; import org.junit.After; @@ -78,13 +78,9 @@ public void startTransaction() { @After public void endTransaction() { - try { - Transaction currentTransaction = tracer.currentTransaction(); - if (currentTransaction != null) { - currentTransaction.deactivate().end(); - } - } finally { - reporter.reset(); + Transaction currentTransaction = tracer.currentTransaction(); + if (currentTransaction != null) { + currentTransaction.deactivate().end(); } } diff --git a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcClientInstrumentationTest.java b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcClientInstrumentationTest.java index 7d9a5061f2..9ee1634bd6 100644 --- a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcClientInstrumentationTest.java @@ -75,20 +75,9 @@ void afterEach() throws Exception { .end(); } - try { - // make sure we do not leave anything behind - reporter.assertRecycledAfterDecrementingReferences(); - - // use a try/finally block here to make sure that if the assertion above fails - // we do not have a side effect on other tests execution by leaving app running. - } finally { - reporter.reset(); - app.stop(); - } - + app.stop(); } - @Test public void simpleCall() { doSimpleCall("bob"); diff --git a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcContextHeadersTest.java b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcContextHeadersTest.java index cbb48d53c4..32687798b7 100644 --- a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcContextHeadersTest.java +++ b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcContextHeadersTest.java @@ -54,13 +54,7 @@ void beforeEach() throws Exception { @AfterEach void afterEach() throws Exception { - // make sure we do not leave anything behind - try { - reporter.assertRecycledAfterDecrementingReferences(); - reporter.reset(); - } finally { - app.stop(); - } + app.stop(); } @Test diff --git a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcServerInstrumentationTest.java b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcServerInstrumentationTest.java index b3734c83e3..f475fb3229 100644 --- a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcServerInstrumentationTest.java +++ b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcServerInstrumentationTest.java @@ -52,16 +52,7 @@ void beforeEach() throws Exception { @AfterEach void afterEach() throws Exception { - try { - // make sure we do not leave anything behind - reporter.assertRecycledAfterDecrementingReferences(); - - // use a try/finally block here to make sure that if the assertion above fails - // we do not have a side effect on other tests execution by leaving app running. - } finally { - app.stop(); - reporter.reset(); - } + app.stop(); } @Test diff --git a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/http/client/HttpClientHelperTest.java b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/http/client/HttpClientHelperTest.java index d587c8d7b0..9ad5b9827b 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/http/client/HttpClientHelperTest.java +++ b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/http/client/HttpClientHelperTest.java @@ -51,7 +51,6 @@ void beforeTest() { @AfterEach void afterTest() { tracer.currentTransaction().deactivate().end(); - reporter.reset(); } @Test diff --git a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java index 657dd08ac8..341989e8c0 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java @@ -73,7 +73,7 @@ public final void setUpWiremock() { wireMockRule.stubFor(get(urlEqualTo("/circular-redirect")) .willReturn(seeOther("/circular-redirect"))); final Transaction transaction = tracer.startRootTransaction(getClass().getClassLoader()); - transaction.withType("request").activate(); + transaction.withName("parent of http span").withType("request").activate(); } @After diff --git a/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/ExecutorInstrumentation.java b/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/ExecutorInstrumentation.java index 44463c1e88..ea4779b3a7 100644 --- a/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/ExecutorInstrumentation.java +++ b/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/ExecutorInstrumentation.java @@ -74,6 +74,7 @@ public abstract class ExecutorInstrumentation extends TracerAwareInstrumentation excludedClasses.add("org.apache.tomcat.util.threads.ThreadPoolExecutor"); } + @Override public ElementMatcher getTypeMatcherPreFilter() { return nameContains("Execut") diff --git a/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/JavaConcurrent.java b/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/JavaConcurrent.java index 652944821f..6f89702912 100644 --- a/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/JavaConcurrent.java +++ b/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/JavaConcurrent.java @@ -167,9 +167,11 @@ public static Collection> withContext(@Nullable Collec } else { wrapped = null; } + Boolean context = needsContext.get(); for (Callable callable : callables) { + // restore previous state as withContext always sets to false + needsContext.set(context); final Callable potentiallyWrappedCallable = withContext(callable, tracer); - needsContext.set(Boolean.TRUE); if (wrapped != null) { wrapped.add(potentiallyWrappedCallable); } diff --git a/apm-agent-plugins/apm-java-concurrent-plugin/src/test/java/co/elastic/apm/agent/concurrent/AsyncTraceMethodInstrumentationTest.java b/apm-agent-plugins/apm-java-concurrent-plugin/src/test/java/co/elastic/apm/agent/concurrent/AsyncTraceMethodInstrumentationTest.java index 43ff6bfbe3..497cd6d90f 100644 --- a/apm-agent-plugins/apm-java-concurrent-plugin/src/test/java/co/elastic/apm/agent/concurrent/AsyncTraceMethodInstrumentationTest.java +++ b/apm-agent-plugins/apm-java-concurrent-plugin/src/test/java/co/elastic/apm/agent/concurrent/AsyncTraceMethodInstrumentationTest.java @@ -55,7 +55,7 @@ class AsyncTraceMethodInstrumentationTest { @BeforeEach void setUp(TestInfo testInfo) { - MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.getOrCreateInstrumentationTracer(); + MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(); reporter = mockInstrumentationSetup.getReporter(); ConfigurationRegistry config = mockInstrumentationSetup.getConfig(); coreConfiguration = config.getConfig(CoreConfiguration.class); diff --git a/apm-agent-plugins/apm-java-concurrent-plugin/src/test/java/co/elastic/apm/agent/concurrent/ScopeManagementTest.java b/apm-agent-plugins/apm-java-concurrent-plugin/src/test/java/co/elastic/apm/agent/concurrent/ScopeManagementTest.java index 337a8dd0d7..83ccc0a5f0 100644 --- a/apm-agent-plugins/apm-java-concurrent-plugin/src/test/java/co/elastic/apm/agent/concurrent/ScopeManagementTest.java +++ b/apm-agent-plugins/apm-java-concurrent-plugin/src/test/java/co/elastic/apm/agent/concurrent/ScopeManagementTest.java @@ -51,8 +51,8 @@ void testWrongDeactivationOrder() { runTestWithAssertionsDisabled(() -> { final Transaction transaction = tracer.startRootTransaction(null).activate(); final Span span = transaction.createSpan().activate(); - transaction.deactivate(); - span.deactivate(); + transaction.deactivate().end(); + span.deactivate().end(); assertThat(tracer.getActive()).isNull(); }); @@ -63,7 +63,8 @@ void testActivateTwice() { runTestWithAssertionsDisabled(() -> { tracer.startRootTransaction(null) .activate().activate() - .deactivate().deactivate(); + .deactivate().deactivate() + .end(); assertThat(tracer.getActive()).isNull(); }); @@ -71,12 +72,13 @@ void testActivateTwice() { @Test void testRedundantActivation() { + disableRecyclingValidation(); runTestWithAssertionsDisabled(() -> { final Transaction transaction = tracer.startRootTransaction(null).activate(); - transaction.createSpan().activate(); + transaction.createSpan().activate().end(); transaction.deactivate(); assertThat(tracer.getActive()).isEqualTo(transaction); - transaction.deactivate(); + transaction.deactivate().end(); assertThat(tracer.getActive()).isNull(); }); } @@ -91,7 +93,7 @@ void testSpanAndContextCallableActivation() { } catch (Exception e) { e.printStackTrace(); } - transaction.deactivate(); + transaction.deactivate().end(); assertThat(tracer.getActive()).isNull(); }); @@ -104,7 +106,7 @@ void testContextAndSpanRunnableActivationInDifferentThread() throws Exception { assertThat(tracer.getActive()).isSameAs(transaction); assertThat(tracer.currentTransaction()).isSameAs(transaction); }).get(); - transaction.deactivate(); + transaction.deactivate().end(); assertThat(tracer.getActive()).isNull(); } @@ -117,7 +119,7 @@ void testContextAndSpanCallableActivationInDifferentThread() throws Exception { return tracer.currentTransaction(); }); assertThat(transactionFuture.get()).isSameAs(transaction); - transaction.deactivate(); + transaction.deactivate().end(); assertThat(tracer.getActive()).isNull(); } @@ -130,7 +132,7 @@ void testSpanAndContextRunnableActivationInDifferentThread() throws Exception { assertThat(tracer.getActive()).isSameAs(transaction); }; Executors.newSingleThreadExecutor().submit(runnable).get(); - transaction.deactivate(); + transaction.deactivate().end(); assertThat(tracer.getActive()).isNull(); } @@ -142,7 +144,7 @@ void testSpanAndContextCallableActivationInDifferentThread() throws Exception { assertThat(tracer.currentTransaction()).isSameAs(transaction); return tracer.currentTransaction(); }).get()).isSameAs(transaction); - transaction.deactivate(); + transaction.deactivate().end(); assertThat(tracer.getActive()).isNull(); } diff --git a/apm-agent-plugins/apm-jaxrs-plugin/src/test/java/co/elastic/apm/agent/jaxrs/JaxRsTransactionNameInstrumentationTest.java b/apm-agent-plugins/apm-jaxrs-plugin/src/test/java/co/elastic/apm/agent/jaxrs/JaxRsTransactionNameInstrumentationTest.java index 5570cb1ae5..b7b935cf63 100644 --- a/apm-agent-plugins/apm-jaxrs-plugin/src/test/java/co/elastic/apm/agent/jaxrs/JaxRsTransactionNameInstrumentationTest.java +++ b/apm-agent-plugins/apm-jaxrs-plugin/src/test/java/co/elastic/apm/agent/jaxrs/JaxRsTransactionNameInstrumentationTest.java @@ -31,12 +31,12 @@ import co.elastic.apm.agent.configuration.SpyConfiguration; import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.impl.transaction.Transaction; +import co.elastic.apm.agent.objectpool.TestObjectPoolFactory; import net.bytebuddy.agent.ByteBuddyAgent; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; import org.junit.After; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; import org.stagemonitor.configuration.ConfigurationRegistry; @@ -56,28 +56,30 @@ */ public class JaxRsTransactionNameInstrumentationTest extends JerseyTest { - private static ElasticApmTracer tracer; - private static MockReporter reporter; - private static ConfigurationRegistry config; + private ElasticApmTracer tracer; + private MockReporter reporter; + private ConfigurationRegistry config; + private TestObjectPoolFactory objectPoolFactory; - @BeforeClass - public static void beforeClass() { - MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.getOrCreateInstrumentationTracer(); + @Before + public void before() { + MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(); reporter = mockInstrumentationSetup.getReporter(); config = mockInstrumentationSetup.getConfig(); tracer = mockInstrumentationSetup.getTracer(); + objectPoolFactory = mockInstrumentationSetup.getObjectPoolFactory(); } @After public void after() { - //reset after each method to test different non-dynamic parameters - ElasticApmAgent.reset(); - } - - @Before - public void before() { - SpyConfiguration.reset(config); - reporter.reset(); + try { + reporter.assertRecycledAfterDecrementingReferences(); + objectPoolFactory.checkAllPooledObjectsHaveBeenRecycled(); + } finally { + reporter.reset(); + objectPoolFactory.reset(); + ElasticApmAgent.reset(); + } } @Test diff --git a/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/JmsInstrumentationIT.java b/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/JmsInstrumentationIT.java index deb9c8e9d0..d5732e8681 100644 --- a/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/JmsInstrumentationIT.java +++ b/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/JmsInstrumentationIT.java @@ -118,7 +118,6 @@ public static void closeResources() throws Exception { public void startTransaction() throws Exception { receiveNoWaitFlow.set(false); expectNoTraces.set(false); - reporter.reset(); startAndActivateTransaction(null); brokerFacade.beforeTest(); noopQ = brokerFacade.createQueue("NOOP"); @@ -147,7 +146,6 @@ public void endTransaction() throws Exception { currentTransaction.deactivate().end(); } brokerFacade.afterTest(); - reporter.reset(); } @Test diff --git a/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringJmsTest.java b/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringJmsTest.java index 4c511caef0..de685baa7d 100644 --- a/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringJmsTest.java +++ b/apm-agent-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/spring/SpringJmsTest.java @@ -75,7 +75,6 @@ public static void teardown() throws JMSException { @Test public void testSendListenSpringQueue() throws JMSException, InterruptedException { - reporter.reset(); try (Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE)) { Transaction transaction = tracer.startRootTransaction(null).activate(); diff --git a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-base-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyClientIT.java b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-base-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyClientIT.java index 66620b4fbd..1b717600ed 100644 --- a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-base-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyClientIT.java +++ b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-base-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyClientIT.java @@ -136,7 +136,6 @@ public static void tearDown() { @Before public void startTransaction() { - reporter.reset(); Transaction transaction = tracer.startRootTransaction(null).activate(); transaction.withName("Kafka-Test Transaction"); transaction.withType("request"); diff --git a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaIT.java b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaIT.java index 04ae9420ed..c5a90829f9 100644 --- a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaIT.java +++ b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaIT.java @@ -144,7 +144,6 @@ public static void tearDown() { @Before public void startTransaction() { - reporter.reset(); startAndActivateTransaction(null); testScenario = TestScenario.NORMAL; } diff --git a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyBrokerIT.java b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyBrokerIT.java index 4d337dfe74..9df62f9cce 100644 --- a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyBrokerIT.java +++ b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyBrokerIT.java @@ -140,7 +140,6 @@ public static void tearDown() { @Before public void startTransaction() { - reporter.reset(); Transaction transaction = tracer.startRootTransaction(null).activate(); transaction.withName("Kafka-Test Transaction"); transaction.withType("request"); diff --git a/apm-agent-plugins/apm-log-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerTest.java b/apm-agent-plugins/apm-log-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerTest.java index 12a0ac80c3..04de346edc 100644 --- a/apm-agent-plugins/apm-log-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerTest.java +++ b/apm-agent-plugins/apm-log-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerTest.java @@ -85,8 +85,10 @@ void testMdcIntegration() { try (Scope grandchildScope = grandchild.activateInScope()) { assertMdcIsSet(grandchild); } + grandchild.end(); assertMdcIsSet(child); } + child.end(); assertMdcIsSet(transaction); } assertMdcIsEmpty(); @@ -114,6 +116,7 @@ void testInactivationWhileMdcIsSet() { try (Scope childScope = child.activateInScope()) { assertMdcIsSet(transaction); } + child.end(); assertMdcIsSet(transaction); } assertMdcIsEmpty(); @@ -248,6 +251,7 @@ void testMdcIntegrationContextScopeInDifferentThread() throws Exception { }).get(); assertMdcIsSet(child); } + child.end(); assertMdcIsSet(transaction); } assertMdcIsEmpty(); diff --git a/apm-agent-plugins/apm-mongoclient-plugin/src/test/java/co/elastic/apm/agent/mongoclient/AbstractMongoClientInstrumentationTest.java b/apm-agent-plugins/apm-mongoclient-plugin/src/test/java/co/elastic/apm/agent/mongoclient/AbstractMongoClientInstrumentationTest.java index 08352e9992..e46b55ffb6 100644 --- a/apm-agent-plugins/apm-mongoclient-plugin/src/test/java/co/elastic/apm/agent/mongoclient/AbstractMongoClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-mongoclient-plugin/src/test/java/co/elastic/apm/agent/mongoclient/AbstractMongoClientInstrumentationTest.java @@ -71,17 +71,13 @@ public void startTransaction() throws Exception { @After public void endTransaction() throws Exception { - try { - reporter.reset(); - dropCollection(); - assertThat(reporter.getSpans()).hasSize(1); - assertThat(reporter.getFirstSpan().getNameAsString()).isEqualTo("testdb.testcollection.drop"); - Transaction currentTransaction = tracer.currentTransaction(); - if (currentTransaction != null) { - currentTransaction.deactivate().end(); - } - } finally { - reporter.reset(); + reporter.reset(); + dropCollection(); + assertThat(reporter.getSpans()).hasSize(1); + assertThat(reporter.getFirstSpan().getNameAsString()).isEqualTo("testdb.testcollection.drop"); + Transaction currentTransaction = tracer.currentTransaction(); + if (currentTransaction != null) { + currentTransaction.deactivate().end(); } } diff --git a/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/ProcessHelperTest.java b/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/ProcessHelperTest.java index 741aff8fc4..5d39903ebe 100644 --- a/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/ProcessHelperTest.java +++ b/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/ProcessHelperTest.java @@ -97,6 +97,7 @@ void startTwiceShouldIgnore() { assertThat(storageMap.get(process)) .describedAs("initial span should not be overwritten") .isSameAs(span); + helper.doEndProcess(process, true); } @Test diff --git a/apm-agent-plugins/apm-redis-plugin/apm-jedis-2-tests/src/test/java/co/elastic/apm/agent/redis/jedis/Jedis2InstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-jedis-2-tests/src/test/java/co/elastic/apm/agent/redis/jedis/Jedis2InstrumentationTest.java index 4ddd975116..4808606bf0 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-jedis-2-tests/src/test/java/co/elastic/apm/agent/redis/jedis/Jedis2InstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-jedis-2-tests/src/test/java/co/elastic/apm/agent/redis/jedis/Jedis2InstrumentationTest.java @@ -24,7 +24,6 @@ */ package co.elastic.apm.agent.redis.jedis; -import co.elastic.apm.agent.impl.Scope; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -55,20 +54,16 @@ void tearDown() { @Test void testShardedJedis() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - shardedJedis.set("foo", "bar"); - assertThat(shardedJedis.get("foo".getBytes())).isEqualTo("bar".getBytes()); - } + shardedJedis.set("foo", "bar"); + assertThat(shardedJedis.get("foo".getBytes())).isEqualTo("bar".getBytes()); assertTransactionWithRedisSpans("SET", "GET"); } @Test void testBinaryJedis() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - binaryJedis.set("foo".getBytes(), "bar".getBytes()); - assertThat(binaryJedis.get("foo".getBytes())).isEqualTo("bar".getBytes()); - } + binaryJedis.set("foo".getBytes(), "bar".getBytes()); + assertThat(binaryJedis.get("foo".getBytes())).isEqualTo("bar".getBytes()); assertTransactionWithRedisSpans("SET", "GET"); } diff --git a/apm-agent-plugins/apm-redis-plugin/apm-jedis-plugin/src/test/java/co/elastic/apm/agent/redis/jedis/Jedis1InstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-jedis-plugin/src/test/java/co/elastic/apm/agent/redis/jedis/Jedis1InstrumentationTest.java index e92231c876..2e271c6c8a 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-jedis-plugin/src/test/java/co/elastic/apm/agent/redis/jedis/Jedis1InstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-jedis-plugin/src/test/java/co/elastic/apm/agent/redis/jedis/Jedis1InstrumentationTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -24,7 +24,6 @@ */ package co.elastic.apm.agent.redis.jedis; -import co.elastic.apm.agent.impl.Scope; import co.elastic.apm.agent.redis.AbstractRedisInstrumentationTest; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -56,10 +55,8 @@ void tearDownJedis() { @Test void testJedis() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - jedis.set("foo", "bar"); - assertThat(jedis.get("foo".getBytes())).isEqualTo("bar".getBytes()); - } + jedis.set("foo", "bar"); + assertThat(jedis.get("foo".getBytes())).isEqualTo("bar".getBytes()); assertTransactionWithRedisSpans("SET", "GET"); } diff --git a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-3-tests/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce3InstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-3-tests/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce3InstrumentationTest.java index 3083613cc2..8d174ef289 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-3-tests/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce3InstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-3-tests/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce3InstrumentationTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -24,7 +24,6 @@ */ package co.elastic.apm.agent.redis.lettuce; -import co.elastic.apm.agent.impl.Scope; import co.elastic.apm.agent.redis.AbstractRedisInstrumentationTest; import com.lambdaworks.redis.RedisClient; import com.lambdaworks.redis.RedisConnection; @@ -50,10 +49,8 @@ public void setUpLettuce() { @Test public void testSyncLettuce() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - connection.set("foo", "bar"); - assertThat(connection.get("foo")).isEqualTo("bar"); - } + connection.set("foo", "bar"); + assertThat(connection.get("foo")).isEqualTo("bar"); assertTransactionWithRedisSpans("SET", "GET"); } diff --git a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce4InstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce4InstrumentationTest.java index d8ef2e9200..d41ea88336 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce4InstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce4InstrumentationTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -24,12 +24,10 @@ */ package co.elastic.apm.agent.redis.lettuce; -import co.elastic.apm.agent.impl.Scope; import co.elastic.apm.agent.redis.AbstractRedisInstrumentationTest; import com.lambdaworks.redis.RedisClient; import com.lambdaworks.redis.api.StatefulRedisConnection; import com.lambdaworks.redis.api.async.RedisAsyncCommands; -import com.lambdaworks.redis.api.rx.RedisReactiveCommands; import com.lambdaworks.redis.api.sync.RedisCommands; import org.junit.After; import org.junit.Before; @@ -52,29 +50,24 @@ public void setUpLettuce() { @Test public void testClusterCommand() { RedisCommands sync = connection.sync(); - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - sync.set("foo", "bar"); - assertThat(sync.get("foo")).isEqualTo("bar"); - } + sync.set("foo", "bar"); + assertThat(sync.get("foo")).isEqualTo("bar"); assertTransactionWithRedisSpans("SET", "GET"); } + @Test public void testSyncLettuce() { RedisCommands sync = connection.sync(); - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - sync.set("foo", "bar"); - assertThat(sync.get("foo")).isEqualTo("bar"); - } + sync.set("foo", "bar"); + assertThat(sync.get("foo")).isEqualTo("bar"); assertTransactionWithRedisSpans("SET", "GET"); } @Test public void testAsyncLettuce() throws Exception { RedisAsyncCommands async = connection.async(); - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - async.set("foo", "bar").get(); - assertThat(async.get("foo").get()).isEqualTo("bar"); - } + async.set("foo", "bar").get(); + assertThat(async.get("foo").get()).isEqualTo("bar"); assertTransactionWithRedisSpans("SET", "GET"); } diff --git a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce5InstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce5InstrumentationTest.java index 61885d90b9..8c4a14777a 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce5InstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-lettuce-plugin/src/test/java/co/elastic/apm/agent/redis/lettuce/Lettuce5InstrumentationTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -24,7 +24,6 @@ */ package co.elastic.apm.agent.redis.lettuce; -import co.elastic.apm.agent.impl.Scope; import co.elastic.apm.agent.redis.AbstractRedisInstrumentationTest; import io.lettuce.core.LettuceFutures; import io.lettuce.core.RedisClient; @@ -57,52 +56,43 @@ public void setUpLettuce() { @Test public void testClusterCommand() { RedisCommands sync = connection.sync(); - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - sync.set("foo", "bar"); - assertThat(sync.get("foo")).isEqualTo("bar"); - } + sync.set("foo", "bar"); + assertThat(sync.get("foo")).isEqualTo("bar"); assertTransactionWithRedisSpans("SET", "GET"); } + @Test public void testSyncLettuce() { RedisCommands sync = connection.sync(); - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - sync.set("foo", "bar"); - assertThat(sync.get("foo")).isEqualTo("bar"); - } + sync.set("foo", "bar"); + assertThat(sync.get("foo")).isEqualTo("bar"); assertTransactionWithRedisSpans("SET", "GET"); } @Test public void testAsyncLettuce() throws Exception { RedisAsyncCommands async = connection.async(); - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - async.set("foo", "bar").get(); - assertThat(async.get("foo").get()).isEqualTo("bar"); - } + async.set("foo", "bar").get(); + assertThat(async.get("foo").get()).isEqualTo("bar"); assertTransactionWithRedisSpans("SET", "GET"); } @Test public void testBatchedLettuce() throws Exception { RedisAsyncCommands async = connection.async(); - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - async.set("foo", "bar").get(); - async.setAutoFlushCommands(false); - List> futures = List.of(async.get("foo"), async.get("foo")); - async.flushCommands(); - LettuceFutures.awaitAll(Duration.ofSeconds(5), futures.toArray(new RedisFuture[0])); - } + async.set("foo", "bar").get(); + async.setAutoFlushCommands(false); + List> futures = List.of(async.get("foo"), async.get("foo")); + async.flushCommands(); + LettuceFutures.awaitAll(Duration.ofSeconds(5), futures.toArray(new RedisFuture[0])); assertTransactionWithRedisSpans("SET", "GET", "GET"); } @Test public void testReactiveLettuce() { RedisReactiveCommands async = connection.reactive(); - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - async.set("foo", "bar").block(); - assertThat(async.get("foo").block()).isEqualTo("bar"); - } + async.set("foo", "bar").block(); + assertThat(async.get("foo").block()).isEqualTo("bar"); assertTransactionWithRedisSpans("SET", "GET"); } diff --git a/apm-agent-plugins/apm-redis-plugin/apm-redis-common/src/test/java/co/elastic/apm/agent/redis/AbstractRedisInstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-redis-common/src/test/java/co/elastic/apm/agent/redis/AbstractRedisInstrumentationTest.java index d7c55af589..636bad991a 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-redis-common/src/test/java/co/elastic/apm/agent/redis/AbstractRedisInstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-redis-common/src/test/java/co/elastic/apm/agent/redis/AbstractRedisInstrumentationTest.java @@ -71,11 +71,13 @@ public final void initRedis() throws IOException { .port(redisPort) .build(); server.start(); + tracer.startRootTransaction(null).activate(); } @After @AfterEach public final void stopRedis() { + tracer.currentTransaction().deactivate().end(); server.stop(); } diff --git a/apm-agent-plugins/apm-redis-plugin/apm-redisson-plugin/src/test/java/co/elastic/apm/agent/redis/redisson/RedissonInstrumentationTest.java b/apm-agent-plugins/apm-redis-plugin/apm-redisson-plugin/src/test/java/co/elastic/apm/agent/redis/redisson/RedissonInstrumentationTest.java index 9ac41d56c3..06ab25969a 100644 --- a/apm-agent-plugins/apm-redis-plugin/apm-redisson-plugin/src/test/java/co/elastic/apm/agent/redis/redisson/RedissonInstrumentationTest.java +++ b/apm-agent-plugins/apm-redis-plugin/apm-redisson-plugin/src/test/java/co/elastic/apm/agent/redis/redisson/RedissonInstrumentationTest.java @@ -24,21 +24,20 @@ */ package co.elastic.apm.agent.redis.redisson; -import co.elastic.apm.agent.impl.Scope; import co.elastic.apm.agent.redis.AbstractRedisInstrumentationTest; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.redisson.Redisson; +import org.redisson.api.RAtomicLong; import org.redisson.api.RBatch; import org.redisson.api.RBucket; import org.redisson.api.RList; -import org.redisson.api.RedissonClient; +import org.redisson.api.RLock; import org.redisson.api.RMap; -import org.redisson.api.RSet; import org.redisson.api.RScoredSortedSet; -import org.redisson.api.RAtomicLong; -import org.redisson.api.RLock; +import org.redisson.api.RSet; +import org.redisson.api.RedissonClient; import org.redisson.config.Config; import java.util.HashMap; @@ -68,93 +67,78 @@ void shutdown() { @Test void testString() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - RBucket keyObject = redisson.getBucket("foo"); - keyObject.set("bar"); - - assertThat(keyObject.get()).isEqualTo("bar"); - } + RBucket keyObject = redisson.getBucket("foo"); + keyObject.set("bar"); + assertThat(keyObject.get()).isEqualTo("bar"); assertTransactionWithRedisSpans("SET", "GET"); } @Test void testBatch() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - RBatch batch = redisson.createBatch(); - batch.getBucket("batch1").setAsync("v1"); - batch.getBucket("batch2").setAsync("v2"); - batch.execute(); + RBatch batch = redisson.createBatch(); + batch.getBucket("batch1").setAsync("v1"); + batch.getBucket("batch2").setAsync("v2"); + batch.execute(); - assertThat(redisson.getBucket("batch1").get()).isEqualTo("v1"); - assertThat(redisson.getBucket("batch2").get()).isEqualTo("v2"); - } + assertThat(redisson.getBucket("batch1").get()).isEqualTo("v1"); + assertThat(redisson.getBucket("batch2").get()).isEqualTo("v2"); assertTransactionWithRedisSpans("SET... [bulk]", "GET", "GET"); } @Test void testList() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - RList strings = redisson.getList("list1"); - strings.add("a"); + RList strings = redisson.getList("list1"); + strings.add("a"); - assertThat(strings.size()).isEqualTo(1); - assertThat(strings.get(0)).isEqualTo("a"); - } + assertThat(strings.size()).isEqualTo(1); + assertThat(strings.get(0)).isEqualTo("a"); assertTransactionWithRedisSpans("RPUSH", "LLEN", "LINDEX"); } @Test void testHash() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - RMap rMap = redisson.getMap("map1"); - rMap.put("key1", "value1"); + RMap rMap = redisson.getMap("map1"); + rMap.put("key1", "value1"); - assertThat(rMap.get("key1")).isEqualTo("value1"); - } + assertThat(rMap.get("key1")).isEqualTo("value1"); assertTransactionWithRedisSpans("EVAL", "HGET"); } @Test void testSet() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - RSet rSet = redisson.getSet("set1"); - rSet.add("s1"); + RSet rSet = redisson.getSet("set1"); + rSet.add("s1"); - assertThat(rSet.contains("s1")).isTrue(); - } + assertThat(rSet.contains("s1")).isTrue(); assertTransactionWithRedisSpans("SADD", "SISMEMBER"); } @Test void testSortedSet() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - Map scores = new HashMap<>(); - scores.put("u1", 1.0d); - scores.put("u2", 3.0d); - scores.put("u3", 0.0d); - RScoredSortedSet sortSet = redisson.getScoredSortedSet("sort_set1"); - sortSet.addAll(scores); - - assertThat(sortSet.rank("u1")).isEqualTo(1); - assertThat(sortSet.rank("u3")).isEqualTo(0); - } + Map scores = new HashMap<>(); + scores.put("u1", 1.0d); + scores.put("u2", 3.0d); + scores.put("u3", 0.0d); + RScoredSortedSet sortSet = redisson.getScoredSortedSet("sort_set1"); + sortSet.addAll(scores); + + assertThat(sortSet.rank("u1")).isEqualTo(1); + assertThat(sortSet.rank("u3")).isEqualTo(0); assertTransactionWithRedisSpans("ZADD", "ZRANK", "ZRANK"); } @Test void testAtomicLong() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - RAtomicLong atomicLong = redisson.getAtomicLong("AtomicLong"); - atomicLong.incrementAndGet(); + RAtomicLong atomicLong = redisson.getAtomicLong("AtomicLong"); + atomicLong.incrementAndGet(); - assertThat(atomicLong.get()).isEqualTo(1); - } + assertThat(atomicLong.get()).isEqualTo(1); assertTransactionWithRedisSpans("INCR", "GET"); } @@ -164,11 +148,9 @@ void testAtomicLong() { */ @Test void testLock() { - try (Scope scope = tracer.startRootTransaction(getClass().getClassLoader()).withName("transaction").activateInScope()) { - RLock lock = redisson.getLock("lock"); - lock.lock(); - lock.unlock(); - } + RLock lock = redisson.getLock("lock"); + lock.lock(); + lock.unlock(); assertTransactionWithRedisSpans("EVAL", "EVAL"); } diff --git a/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/spring/scheduled/ScheduledTransactionNameInstrumentationTest.java b/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/spring/scheduled/ScheduledTransactionNameInstrumentationTest.java index be7b91fe79..dd767f9cb0 100644 --- a/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/spring/scheduled/ScheduledTransactionNameInstrumentationTest.java +++ b/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/spring/scheduled/ScheduledTransactionNameInstrumentationTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -24,15 +24,14 @@ */ package co.elastic.apm.agent.spring.scheduled; -import java.util.concurrent.atomic.AtomicInteger; - -import javax.ejb.Schedule; - import co.elastic.apm.agent.AbstractInstrumentationTest; import org.junit.jupiter.api.Test; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.scheduling.annotation.Schedules; +import javax.ejb.Schedule; +import java.util.concurrent.atomic.AtomicInteger; + import static org.assertj.core.api.Assertions.assertThat; @@ -40,7 +39,6 @@ class ScheduledTransactionNameInstrumentationTest extends AbstractInstrumentatio @Test void testSpringScheduledAnnotatedMethodsAreTraced() { - reporter.reset(); SpringCounter springCounter = new SpringCounter(); springCounter.scheduled(); springCounter.scheduled(); @@ -50,7 +48,6 @@ void testSpringScheduledAnnotatedMethodsAreTraced() { @Test void testSpringJ8RepeatableScheduledAnnotatedMethodsAreTraced() { - reporter.reset(); SpringCounter springCounter = new SpringCounter(); springCounter.scheduledJava8Repeatable(); springCounter.scheduledJava8Repeatable(); @@ -60,7 +57,6 @@ void testSpringJ8RepeatableScheduledAnnotatedMethodsAreTraced() { @Test void testSpringJ7RepeatableScheduledAnnotatedMethodsAreTraced() { - reporter.reset(); SpringCounter springCounter = new SpringCounter(); springCounter.scheduledJava7Repeatable(); springCounter.scheduledJava7Repeatable(); @@ -70,7 +66,6 @@ void testSpringJ7RepeatableScheduledAnnotatedMethodsAreTraced() { @Test void testJeeScheduledAnnotatedMethodsAreTraced() { - reporter.reset(); JeeCounter jeeCounter = new JeeCounter(); jeeCounter.scheduled(); jeeCounter.scheduled(); @@ -80,7 +75,6 @@ void testJeeScheduledAnnotatedMethodsAreTraced() { @Test void testJeeJ7RepeatableScheduledAnnotatedMethodsAreTraced() { - reporter.reset(); JeeCounter jeeCounter = new JeeCounter(); jeeCounter.scheduledJava7Repeatable(); jeeCounter.scheduledJava7Repeatable(); diff --git a/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/spring/scheduled/TimerTaskInstrumentationTest.java b/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/spring/scheduled/TimerTaskInstrumentationTest.java index fde5cd4f61..8adc6d8b1e 100644 --- a/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/spring/scheduled/TimerTaskInstrumentationTest.java +++ b/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/spring/scheduled/TimerTaskInstrumentationTest.java @@ -25,6 +25,7 @@ package co.elastic.apm.agent.spring.scheduled; import co.elastic.apm.agent.AbstractInstrumentationTest; +import co.elastic.apm.agent.impl.transaction.AbstractSpan; import co.elastic.apm.agent.impl.transaction.Transaction; import org.junit.jupiter.api.Test; @@ -38,82 +39,68 @@ public class TimerTaskInstrumentationTest extends AbstractInstrumentationTest { @Test void testTimerTask_scheduleWithFixedRate() { - reporter.reset(); - TestTimerTask timerTask = new TestTimerTask(); - Timer timer = new Timer(true); - timer.scheduleAtFixedRate(timerTask, 0, 10L); - - reporter.awaitTransactionReported(); - timer.cancel(); - - reporter.awaitUntilAsserted(1000L, () -> { - assertThat(reporter.getNumReportedTransactions()).isEqualTo(timerTask.getInvocationCount()); - }); - - Transaction firstTransaction = reporter.getTransactions().get(0); - assertThat(firstTransaction.getNameAsString()).isEqualTo("TestTimerTask#run"); - assertThat(firstTransaction.getFrameworkName()).isEqualTo("TimerTask"); + new Timer(true) + .scheduleAtFixedRate(new TestTimerTask(2), 1, 1); + + reporter.awaitTransactionCount(2); + + assertThat(reporter.getTransactions() + .stream() + .map(AbstractSpan::getNameAsString)) + .containsExactly("TestTimerTask#run", "TestTimerTask#run"); + assertThat(reporter.getTransactions() + .stream() + .map(Transaction::getFrameworkName)) + .containsExactly("TimerTask", "TimerTask"); } @Test void testTimerTask_scheduleWithFixedDelay() { - reporter.reset(); - TestTimerTask timerTask = new TestTimerTask(); - Timer timer = new Timer("Timer"); - timer.schedule(timerTask, 1L, 10L); - - reporter.awaitTransactionReported(); - timer.cancel(); + new Timer("Timer") + .schedule(new TestTimerTask(2), 1, 1); - reporter.awaitUntilAsserted(1000L, () -> { - assertThat(reporter.getNumReportedTransactions()).isEqualTo(timerTask.getInvocationCount()); - }); + reporter.awaitTransactionCount(2); - assertThat(reporter.getTransactions().get(0).getNameAsString()).isEqualTo("TestTimerTask#run"); + assertThat(reporter.getTransactions() + .stream() + .map(AbstractSpan::getNameAsString)) + .containsExactly("TestTimerTask#run", "TestTimerTask#run"); } @Test void testTimerTask_scheduleOnce() { - reporter.reset(); - TestTimerTask timerTask = new TestTimerTask(); - Timer timer = new Timer("Timer"); - long delay = 50L; - timer.schedule(timerTask, delay); - - reporter.awaitTransactionReported(); - assertThat(reporter.getTransactions().size()).isEqualTo(1); + new Timer("Timer") + .schedule(new TestTimerTask(1), 1); + + reporter.awaitTransactionCount(1); assertThat(reporter.getTransactions().get(0).getNameAsString()).isEqualTo("TestTimerTask#run"); } @Test void testTimerTask_withAnonymousClass() { - reporter.reset(); - AtomicInteger count = new AtomicInteger(0); - - TimerTask repeatedTask = new TimerTask() { - public void run() { - count.incrementAndGet(); - } - }; - Timer timer = new Timer("Timer"); - long delay = 50L; - timer.schedule(repeatedTask, delay); - - reporter.awaitTransactionReported(); - assertThat(reporter.getTransactions().size()).isEqualTo(1); + new Timer("Timer") + .schedule(new TimerTask() { + public void run() { + cancel(); + } + }, 1); + + reporter.awaitTransactionCount(1); assertThat(reporter.getTransactions().get(0).getNameAsString()).isEqualTo("1#run"); } public static class TestTimerTask extends TimerTask { - private AtomicInteger count = new AtomicInteger(0); + private final AtomicInteger credits; - @Override - public void run() { - this.count.incrementAndGet(); + public TestTimerTask(int maxInvocations) { + credits = new AtomicInteger(maxInvocations); } - public int getInvocationCount() { - return this.count.get(); + @Override + public void run() { + if (credits.decrementAndGet() == 0) { + cancel(); + } } } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java index 179ea52fc7..753ff545b5 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java @@ -32,7 +32,6 @@ import co.elastic.apm.agent.report.serialize.DslJsonSerializer; import co.elastic.apm.agent.util.PotentiallyMultiValuedMap; import org.apache.commons.lang3.RandomStringUtils; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -112,15 +111,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I } - @AfterEach - void tearDown() { - Transaction transaction = reporter.getFirstTransaction(); - reporter.assertRecycledAfterDecrementingReferences(); - assertThat(transaction.getContext().getRequest().getRawBody()).isNull(); - assertThat(transaction.getContext().getRequest().getBody()).isNull(); - assertThat((CharSequence) transaction.getContext().getRequest().getBodyBufferForSerialization()).isNull(); - } - @ParameterizedTest @MethodSource("streamConsumers") void testReadTextPlain(InputStreamConsumer consumer) throws Exception { diff --git a/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/AbstractExceptionHandlerInstrumentationTest.java b/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/AbstractExceptionHandlerInstrumentationTest.java index 6d3f31d859..23c3499038 100644 --- a/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/AbstractExceptionHandlerInstrumentationTest.java +++ b/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/AbstractExceptionHandlerInstrumentationTest.java @@ -70,7 +70,7 @@ public abstract class AbstractExceptionHandlerInstrumentationTest { @BeforeClass @BeforeAll public static void setUpAll() { - MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.getOrCreateInstrumentationTracer(); + MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(); reporter = mockInstrumentationSetup.getReporter(); tracer = mockInstrumentationSetup.getTracer(); config = mockInstrumentationSetup.getConfig(); diff --git a/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/template/AbstractViewRenderingInstrumentationTest.java b/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/template/AbstractViewRenderingInstrumentationTest.java index 94fb6e0b2e..86cfd84a94 100644 --- a/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/template/AbstractViewRenderingInstrumentationTest.java +++ b/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/template/AbstractViewRenderingInstrumentationTest.java @@ -69,7 +69,7 @@ abstract class AbstractViewRenderingInstrumentationTest { @BeforeAll static void beforeAll() { - MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.getOrCreateInstrumentationTracer(); + MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(); reporter = mockInstrumentationSetup.getReporter(); config = mockInstrumentationSetup.getConfig(); tracer = mockInstrumentationSetup.getTracer(); diff --git a/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeCloseTest.java b/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeCloseTest.java index 9514bfc39d..87bfae2058 100644 --- a/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeCloseTest.java +++ b/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeCloseTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -36,7 +36,9 @@ public class OpenTracingBridgeCloseTest extends AbstractInstrumentationTest { @BeforeEach void setUp() { - reporter.reset(); + // OT always leaks the spans + // see co.elastic.apm.agent.opentracing.impl.ApmSpanBuilderInstrumentation.CreateSpanInstrumentation.doCreateTransactionOrSpan + disableRecyclingValidation(); } @Test diff --git a/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java b/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java index b8a93c6f24..0e92fab610 100644 --- a/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java +++ b/apm-opentracing/src/test/java/co/elastic/apm/opentracing/OpenTracingBridgeTest.java @@ -60,7 +60,9 @@ class OpenTracingBridgeTest extends AbstractInstrumentationTest { @BeforeEach void setUp() { apmTracer = new ElasticApmTracer(); - reporter.reset(); + // OT always leaks the spans + // see co.elastic.apm.agent.opentracing.impl.ApmSpanBuilderInstrumentation.CreateSpanInstrumentation.doCreateTransactionOrSpan + disableRecyclingValidation(); } @AfterEach @@ -560,7 +562,7 @@ private Transaction createTransactionFromOtTags(Map tags) { spanBuilder.start().finish(); assertThat(reporter.getTransactions()).hasSize(1); final Transaction transaction = reporter.getFirstTransaction(); - reporter.reset(); + reporter.resetWithoutRecycling(); return transaction; } @@ -575,7 +577,7 @@ private co.elastic.apm.agent.impl.transaction.Span createSpanFromOtTags(Map