Fix invalid currentTransaction() logic (elastic#3294)

jackshirazi · Aug 25, 2023 · 3a2163e · 3a2163e
1 parent ca96d4e
commit 3a2163e
Show file tree

Hide file tree

Showing 5 changed files with 4 additions and 25 deletions.
diff --git 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
@@ -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<ActivationListener> 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
@@ -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
@@ -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<Tracer> FROM_ACTIVE = new ChildContextCreator<Tracer>() {
-        @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<Object> AS_ROOT = new ChildContextCreator<Object>() {
         @Override
         public boolean asChildOf(TraceContext child, Object ignore) {
@@ -292,10 +281,6 @@ public static <C> HeaderChildContextCreator<byte[], C> getFromTraceContextBinary
         return (HeaderChildContextCreator<byte[], C>) FROM_TRACE_CONTEXT_BINARY_HEADERS;
     }
 
-    public static ChildContextCreator<Tracer> fromActive() {
-        return FROM_ACTIVE;
-    }
-
     public static ChildContextCreator<TraceContext> 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
@@ -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();