From 3a2163e17f50f57a6f59041a451488f36cdeb844 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Fri, 25 Aug 2023 11:09:11 +0200 Subject: [PATCH] Fix invalid currentTransaction() logic (#3294) --- CHANGELOG.asciidoc | 1 + .../co/elastic/apm/agent/impl/ActiveStack.java | 9 +-------- .../elastic/apm/agent/impl/ElasticApmTracer.java | 2 +- .../apm/agent/impl/transaction/TraceContext.java | 15 --------------- .../apm/agent/impl/ElasticApmTracerTest.java | 2 +- 5 files changed, 4 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index b7374fd8bf..52d52f1fe7 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -36,6 +36,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: * Prevent bad serialization in edge cases for span compression - {pull}3293[#3293] * Allow overriding of transaction type for Servlet-API transactions - {pull}3226[#3226] * Fix micrometer histogram serialization - {pull}3290[#3290] +* Fix transactions not being correctly handled in certain edge cases - {pull}3294[#3294] [float] ===== Features diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ActiveStack.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ActiveStack.java index 90350b08bc..e0d421b4f0 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ActiveStack.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ActiveStack.java @@ -21,7 +21,6 @@ import co.elastic.apm.agent.impl.transaction.AbstractSpan; import co.elastic.apm.agent.impl.transaction.ElasticContext; import co.elastic.apm.agent.impl.transaction.ElasticContextWrapper; -import co.elastic.apm.agent.impl.transaction.Transaction; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; @@ -65,12 +64,6 @@ class ActiveStack { this.emptyContext = emptyContextForTracer; } - @Nullable - Transaction currentTransaction() { - final ElasticContext bottomOfStack = activeContextStack.peekLast(); - return bottomOfStack != null ? bottomOfStack.getTransaction() : null; - } - /** * @return the current context, potentially empty when no span, transaction or baggage is currently active. */ @@ -94,7 +87,7 @@ boolean activate(ElasticContext context, List activationL if (activeContextStack.size() == stackMaxDepth) { if (overflowCounter == 0) { logger.error(String.format("Activation stack depth reached its maximum - %s. This is likely related to activation" + - " leak. Current transaction: %s", stackMaxDepth, currentTransaction()), + " leak. Current transaction: %s", stackMaxDepth, currentContext().getTransaction()), new Throwable("Stack of threshold-crossing activation: ") ); } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java index 69b0d74d14..254bd3a42d 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java @@ -355,7 +355,7 @@ private Transaction createTransaction() { @Override @Nullable public Transaction currentTransaction() { - return activeStack.get().currentTransaction(); + return currentContext().getTransaction(); } /** diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java index e9a0bb7d79..7252a29737 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java @@ -20,7 +20,6 @@ import co.elastic.apm.agent.configuration.CoreConfiguration; import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.impl.Tracer; import co.elastic.apm.agent.impl.baggage.Baggage; import co.elastic.apm.agent.impl.sampling.Sampler; import co.elastic.apm.agent.sdk.logging.Logger; @@ -177,17 +176,7 @@ public boolean asChildOf(TraceContext child, @Nullable Object carrier, HeaderGet return false; } }; - private static final ChildContextCreator FROM_ACTIVE = new ChildContextCreator() { - @Override - public boolean asChildOf(TraceContext child, Tracer tracer) { - final AbstractSpan active = tracer.getActive(); - if (active != null) { - return fromParent().asChildOf(child, active); - } - return false; - } - }; private static final ChildContextCreator AS_ROOT = new ChildContextCreator() { @Override public boolean asChildOf(TraceContext child, Object ignore) { @@ -292,10 +281,6 @@ public static HeaderChildContextCreator getFromTraceContextBinary return (HeaderChildContextCreator) FROM_TRACE_CONTEXT_BINARY_HEADERS; } - public static ChildContextCreator fromActive() { - return FROM_ACTIVE; - } - public static ChildContextCreator fromParentContext() { return FROM_PARENT_CONTEXT; } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java index 52fd5a8321..baaf63d903 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java @@ -514,7 +514,7 @@ void testStartSpanAfterTransactionHasEnded() { try (Scope transactionScope = transaction.activateInScope()) { assertThat(tracerImpl.getActive()).isEqualTo(transaction); - final Span span = tracerImpl.startSpan(TraceContext.fromActive(), tracerImpl, Baggage.EMPTY); + final Span span = tracerImpl.startSpan(transaction, Baggage.EMPTY, -1); assertThat(span).isNotNull(); try (Scope scope = span.activateInScope()) { assertThat(tracerImpl.currentTransaction()).isNotNull();