Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
===== Bug fixes
* Fixed edge case where inferred spans could cause cycles in the trace parent-child relationships, subsequently resulting in the UI crashing - {pull}3588[#3588]
* Fix NPE in dropped spans statistics - {pull}3590[#3590]
* Fix too small activation stack size for small `transaction_max_spans` values - {pull}3643[#3643]

[[release-notes-1.x]]
=== Java Agent version 1.x
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ public class ElasticApmTracer implements Tracer {
private static final Map<Class<?>, Class<? extends ConfigurationOptionProvider>> configs = new HashMap<>();

public static final Set<String> TRACE_HEADER_NAMES;
public static final int ACTIVATION_STACK_BASE_SIZE = 16;

static {
Set<String> headerNames = new HashSet<>();
Expand All @@ -138,7 +139,10 @@ public class ElasticApmTracer implements Tracer {
private final ThreadLocal<ActiveStack> activeStack = new ThreadLocal<ActiveStack>() {
@Override
protected ActiveStack initialValue() {
return new ActiveStack(transactionMaxSpans, emptyContext);
//We allow transactionMaxSpan activation plus a constant minimum of 16 to account for
// * the activation of the transaction itself
// * account for baggage updates, which also count towards the depth
return new ActiveStack(ACTIVATION_STACK_BASE_SIZE + transactionMaxSpans, emptyContext);
}
};

Expand Down Expand Up @@ -673,7 +677,7 @@ public synchronized void stop() {
public Reporter getReporter() {
return reporter;
}

public UniversalProfilingIntegration getProfilingIntegration() {
return profilingIntegration;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import co.elastic.apm.agent.common.util.WildcardMatcher;
import co.elastic.apm.agent.configuration.AutoDetectedServiceInfo;
import co.elastic.apm.agent.configuration.CoreConfiguration;
import co.elastic.apm.agent.impl.baggage.BaggageContext;
import co.elastic.apm.agent.tracer.service.ServiceInfo;
import co.elastic.apm.agent.configuration.SpyConfiguration;
import co.elastic.apm.agent.configuration.source.ConfigSources;
Expand Down Expand Up @@ -52,6 +53,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand All @@ -73,9 +75,15 @@ class ElasticApmTracerTest {

@BeforeEach
void setUp() {
doSetup(conf -> {
});
}

void doSetup(Consumer<ConfigurationRegistry> configCustomizer) {
objectPoolFactory = new TestObjectPoolFactory();
reporter = new MockReporter();
config = SpyConfiguration.createSpyConfig();
configCustomizer.accept(config);

apmServerClient = new ApmServerClient(config);
apmServerClient = mock(ApmServerClient.class, delegatesTo(apmServerClient));
Expand All @@ -88,6 +96,11 @@ void setUp() {
.buildAndStart();
}

void setupWithCustomConfig(Consumer<ConfigurationRegistry> configCustomizer) {
cleanupAndCheck(); //cleanup @BeforeEach
doSetup(configCustomizer);
}

@AfterEach
void cleanupAndCheck() {
reporter.assertRecycledAfterDecrementingReferences();
Expand Down Expand Up @@ -326,11 +339,14 @@ private ErrorCapture validateError(ErrorCapture error, AbstractSpan<?> span, boo

@Test
void testEnableDropSpans() {
doReturn(1).when(tracerImpl.getConfig(CoreConfiguration.class)).getTransactionMaxSpans();
setupWithCustomConfig(conf -> {
doReturn(1).when(conf.getConfig(CoreConfiguration.class)).getTransactionMaxSpans();
});
Transaction transaction = startTestRootTransaction();
try (Scope scope = transaction.activateInScope()) {
Span span = tracerImpl.getActive().createSpan();
try (Scope spanScope = span.activateInScope()) {
assertThat(tracerImpl.getActive()).isSameAs(span); //ensure ActiveStack limit is not reached
assertThat(span.isSampled()).isTrue();
span.end();
}
Expand Down Expand Up @@ -359,48 +375,61 @@ void testActivationStackOverflow() {
.withObjectPoolFactory(objectPoolFactory)
.buildAndStart();

Transaction transaction = tracer.startRootTransaction(getClass().getClassLoader());
assertThat(tracer.getActive()).isNull();
try (Scope scope = transaction.activateInScope()) {
assertThat(tracer.getActive()).isEqualTo(transaction);
Span child1 = transaction.createSpan();
try (Scope childScope = child1.activateInScope()) {
assertThat(tracer.getActive()).isEqualTo(child1);
Span grandchild1 = child1.createSpan();
try (Scope grandchildScope = grandchild1.activateInScope()) {
// latter activation should not be applied due to activation stack overflow
doWithNestedBaggageActivations(() -> {
Transaction transaction = tracer.startRootTransaction(getClass().getClassLoader());
assertThat(tracer.getActive()).isNull();
try (Scope scope = transaction.activateInScope()) {
assertThat(tracer.getActive()).isEqualTo(transaction);
Span child1 = transaction.createSpan();
try (Scope childScope = child1.activateInScope()) {
assertThat(tracer.getActive()).isEqualTo(child1);
Span ggc = grandchild1.createSpan();
try (Scope ggcScope = ggc.activateInScope()) {
Span grandchild1 = child1.createSpan();
try (Scope grandchildScope = grandchild1.activateInScope()) {
// latter activation should not be applied due to activation stack overflow
assertThat(tracer.getActive()).isEqualTo(child1);
ggc.end();
Span ggc = grandchild1.createSpan();
try (Scope ggcScope = ggc.activateInScope()) {
assertThat(tracer.getActive()).isEqualTo(child1);
ggc.end();
}
grandchild1.end();
}
grandchild1.end();
assertThat(tracer.getActive()).isEqualTo(child1);
child1.end();
}
assertThat(tracer.getActive()).isEqualTo(child1);
child1.end();
}
assertThat(tracer.getActive()).isEqualTo(transaction);
Span child2 = transaction.createSpan();
try (Scope childScope = child2.activateInScope()) {
assertThat(tracer.getActive()).isEqualTo(child2);
Span grandchild2 = child2.createSpan();
try (Scope grandchildScope = grandchild2.activateInScope()) {
// latter activation should not be applied due to activation stack overflow
assertThat(tracer.getActive()).isEqualTo(transaction);
Span child2 = transaction.createSpan();
try (Scope childScope = child2.activateInScope()) {
assertThat(tracer.getActive()).isEqualTo(child2);
Span grandchild2 = child2.createSpan();
try (Scope grandchildScope = grandchild2.activateInScope()) {
// latter activation should not be applied due to activation stack overflow
assertThat(tracer.getActive()).isEqualTo(child2);
grandchild2.end();
}
assertThat(tracer.getActive()).isEqualTo(child2);
grandchild2.end();
child2.end();
}
assertThat(tracer.getActive()).isEqualTo(child2);
child2.end();
assertThat(tracer.getActive()).isEqualTo(transaction);
transaction.end();
}
assertThat(tracer.getActive()).isEqualTo(transaction);
transaction.end();
}
}, tracer, ElasticApmTracer.ACTIVATION_STACK_BASE_SIZE);
assertThat(tracer.getActive()).isNull();
assertThat(reporter.getTransactions()).hasSize(1);
assertThat(reporter.getSpans()).hasSize(2);
}

private void doWithNestedBaggageActivations(Runnable r, Tracer tracer, int nestedCount) {
if (nestedCount == 0) {
r.run();
return;
}
BaggageContext baggageContext = tracer.currentContext().withUpdatedBaggage().buildContext();
try (Scope scope = baggageContext.activateInScope()) {
doWithNestedBaggageActivations(r, tracer, nestedCount - 1);
}
}

@Test
void testPause() {
tracerImpl.pause();
Expand Down