Skip to content

Commit efe97c9

Browse files
videnkzfelixbarny
videnkz
and
felixbarny
authored
renamed span_frames_min_duration to span_stack_trace_min_duration. Ch… (elastic#2220)
* renamed span_frames_min_duration to span_stack_trace_min_duration. Changed logic when reporting. * added entry to changelog * fixed according to comments * fixed according to comments * added StacktraceConfigurationTest * added headers * updated logic and tests * Update apm-agent-core/src/main/java/co/elastic/apm/agent/impl/stacktrace/StacktraceConfiguration.java deleted aliasKey Co-authored-by: Felix Barnsteiner <[email protected]> * updated changelog * Update apm-agent-core/src/main/java/co/elastic/apm/agent/impl/stacktrace/StacktraceConfiguration.java Co-authored-by: Felix Barnsteiner <[email protected]> * Update CHANGELOG.asciidoc Co-authored-by: Felix Barnsteiner <[email protected]> * Update apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java Co-authored-by: Felix Barnsteiner <[email protected]> * added logic with check default value of spanFramesMinDurationMsValue * fixed remaining references to config-span-frames-min-duration * fixed reference in CoreConfiguration Co-authored-by: Felix Barnsteiner <[email protected]> Co-authored-by: Felix Barnsteiner <[email protected]>
1 parent 87dc99a commit efe97c9

File tree

17 files changed

+267
-24
lines changed

17 files changed

+267
-24
lines changed

‎CHANGELOG.asciidoc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ by the agent may be different. This was done in order to improve the integration
4545
* Add support for Spring AMQP batch API - {pull}1716[#1716]
4646
* Add the (current) transaction name to the error (when using APM Server 8.0) - {pull}2235[#2235]
4747
* The JVM/JMX metrics are reported for each service name individually (when using APM Server 8.0) - {pull}2233[#2233]
48+
* Added <<config-span-stack-trace-min-duration,`span_stack_trace_min_duration`>> option.
49+
This replaces the now deprecated `span_frames_min_duration` option.
50+
The difference is that the new option has more intuitive semantics for negative values (never collect stack trace) and zero (always collect stack trace). - {pull}2220[#2220]
4851
4952
[float]
5053
===== Performance improvements

‎apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/CoreConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ public class CoreConfiguration extends ConfigurationOptionProvider {
466466
"Also note that there is a maximum amount of spans per transaction (see <<config-transaction-max-spans, `transaction_max_spans`>>).\n" +
467467
"\n" +
468468
"NOTE: The agent will create stack traces for spans which took longer than\n" +
469-
"<<config-span-frames-min-duration, `span_frames_min_duration`>>.\n" +
469+
"<<config-span-stack-trace-min-duration, `span_stack_trace_min_duration`>>.\n" +
470470
"When tracing a large number of methods (for example by using wildcards),\n" +
471471
"this may lead to high overhead.\n" +
472472
"Consider increasing the threshold or disabling stack trace collection altogether.\n\n" +

‎apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -407,9 +407,10 @@ private void reportSpan(Span span) {
407407
}
408408
// makes sure that parents are also non-discardable
409409
span.setNonDiscardable();
410-
long spanFramesMinDurationMs = stacktraceConfiguration.getSpanFramesMinDurationMs();
411-
if (spanFramesMinDurationMs != 0 && span.isSampled() && span.getStackFrames() == null) {
412-
if (span.getDurationMs() >= spanFramesMinDurationMs) {
410+
411+
long spanStackTraceMinDurationMs = stacktraceConfiguration.getSpanStackTraceMinDurationMs();
412+
if (spanStackTraceMinDurationMs >= 0 && span.isSampled() && span.getStackFrames() == null) {
413+
if (span.getDurationMs() >= spanStackTraceMinDurationMs) {
413414
span.withStacktrace(new Throwable());
414415
}
415416
}

‎apm-agent-core/src/main/java/co/elastic/apm/agent/impl/stacktrace/StacktraceConfiguration.java

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public class StacktraceConfiguration extends ConfigurationOptionProvider {
6767
private final ConfigurationOption<TimeDuration> spanFramesMinDurationMs = TimeDurationValueConverter.durationOption("ms")
6868
.key("span_frames_min_duration")
6969
.aliasKeys("span_frames_min_duration_ms")
70-
.tags("performance")
70+
.tags("internal", "deprecated")
7171
.configurationCategory(STACKTRACE_CATEGORY)
7272
.description("While this is very helpful to find the exact place in your code that causes the span, " +
7373
"collecting this stack trace does have some overhead. " +
@@ -80,6 +80,21 @@ public class StacktraceConfiguration extends ConfigurationOptionProvider {
8080
.dynamic(true)
8181
.buildWithDefault(TimeDuration.of("5ms"));
8282

83+
private final ConfigurationOption<TimeDuration> spanStackTraceMinDurationMs = TimeDurationValueConverter.durationOption("ms")
84+
.key("span_stack_trace_min_duration")
85+
.tags("performance")
86+
.configurationCategory(STACKTRACE_CATEGORY)
87+
.description("While this is very helpful to find the exact place in your code that causes the span, " +
88+
"collecting this stack trace does have some overhead. " +
89+
"\n" +
90+
"When setting this option to value `0ms`, stack traces will be collected for all spans. " +
91+
"Setting it to a positive value, e.g. `5ms`, will limit stack trace collection to spans " +
92+
"with durations equal to or longer than the given value, e.g. 5 milliseconds.\n" +
93+
"\n" +
94+
"To disable stack trace collection for spans completely, set the value to `-1ms`.")
95+
.dynamic(true)
96+
.buildWithDefault(TimeDuration.of("5ms"));
97+
8398
public Collection<String> getApplicationPackages() {
8499
return applicationPackages.get();
85100
}
@@ -88,7 +103,17 @@ public int getStackTraceLimit() {
88103
return stackTraceLimit.get();
89104
}
90105

91-
public long getSpanFramesMinDurationMs() {
92-
return spanFramesMinDurationMs.getValue().getMillis();
106+
public long getSpanStackTraceMinDurationMs() {
107+
if (spanStackTraceMinDurationMs.isDefault() && !spanFramesMinDurationMs.isDefault()) {
108+
long spanFramesMinDurationMsValue = spanFramesMinDurationMs.getValue().getMillis();
109+
if (spanFramesMinDurationMsValue == 0) {
110+
return -1;
111+
} else if (spanFramesMinDurationMsValue < 0) {
112+
return 0;
113+
} else {
114+
return spanFramesMinDurationMsValue;
115+
}
116+
}
117+
return spanStackTraceMinDurationMs.getValue().getMillis();
93118
}
94119
}

‎apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,8 @@ void testNestedSpan() {
119119

120120
@Test
121121
void testDisableStacktraces() {
122-
when(tracerImpl.getConfig(StacktraceConfiguration.class).getSpanFramesMinDurationMs()).thenReturn(0L);
122+
when(tracerImpl.getConfig(StacktraceConfiguration.class).getSpanStackTraceMinDurationMs()).thenReturn(-1L);
123+
123124
Transaction transaction = startTestRootTransaction();
124125
try (Scope scope = transaction.activateInScope()) {
125126
Span span = tracerImpl.getActive().createSpan();
@@ -133,7 +134,7 @@ void testDisableStacktraces() {
133134

134135
@Test
135136
void testEnableStacktraces() throws InterruptedException {
136-
when(tracerImpl.getConfig(StacktraceConfiguration.class).getSpanFramesMinDurationMs()).thenReturn(-1L);
137+
when(tracerImpl.getConfig(StacktraceConfiguration.class).getSpanStackTraceMinDurationMs()).thenReturn(0L);
137138
Transaction transaction = startTestRootTransaction();
138139
try (Scope scope = transaction.activateInScope()) {
139140
Span span = tracerImpl.getActive().createSpan();
@@ -148,7 +149,7 @@ void testEnableStacktraces() throws InterruptedException {
148149

149150
@Test
150151
void testDisableStacktracesForFastSpans() {
151-
when(tracerImpl.getConfig(StacktraceConfiguration.class).getSpanFramesMinDurationMs()).thenReturn(100L);
152+
when(tracerImpl.getConfig(StacktraceConfiguration.class).getSpanStackTraceMinDurationMs()).thenReturn(100L);
152153
Transaction transaction = startTestRootTransaction();
153154
try (Scope scope = transaction.activateInScope()) {
154155
Span span = tracerImpl.getActive().createSpan();
@@ -163,7 +164,7 @@ void testDisableStacktracesForFastSpans() {
163164

164165
@Test
165166
void testEnableStacktracesForSlowSpans() throws InterruptedException {
166-
when(tracerImpl.getConfig(StacktraceConfiguration.class).getSpanFramesMinDurationMs()).thenReturn(1L);
167+
when(tracerImpl.getConfig(StacktraceConfiguration.class).getSpanStackTraceMinDurationMs()).thenReturn(1L);
167168
Transaction transaction = startTestRootTransaction();
168169
try (Scope scope = transaction.activateInScope()) {
169170
Span span = tracerImpl.getActive().createSpan();

‎apm-agent-core/src/test/java/co/elastic/apm/agent/impl/stacktrace/StacktraceConfigurationTest.java

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package co.elastic.apm.agent.impl.stacktrace;
20+
21+
import co.elastic.apm.agent.configuration.SpyConfiguration;
22+
import org.junit.jupiter.api.Test;
23+
import org.stagemonitor.configuration.ConfigurationRegistry;
24+
import org.stagemonitor.configuration.source.SimpleSource;
25+
26+
import static org.assertj.core.api.Assertions.assertThat;
27+
28+
class StacktraceConfigurationTest {
29+
30+
@Test
31+
void testGetSpanStackTraceMinDurationMs_whenSpanStackTraceMinDurationNonDefault_thenGetValueFromSpanStackTraceMinDuration() {
32+
ConfigurationRegistry configRegistry = SpyConfiguration.createSpyConfig(SimpleSource.forTest("span_frames_min_duration", "10ms").add("span_stack_trace_min_duration", "15ms"));
33+
34+
long actual = configRegistry.getConfig(StacktraceConfiguration.class).getSpanStackTraceMinDurationMs();
35+
assertThat(actual).isEqualTo(15);
36+
}
37+
38+
@Test
39+
void testGetSpanStackTraceMinDurationMs_whenSpanStackTraceDefault_whenSpanFramesNonDefault_thenGetValueFromSpanFramesMinDuration() {
40+
ConfigurationRegistry configRegistry = SpyConfiguration.createSpyConfig(SimpleSource.forTest("span_frames_min_duration", "15ms"));
41+
42+
long actual = configRegistry.getConfig(StacktraceConfiguration.class).getSpanStackTraceMinDurationMs();
43+
assertThat(actual).isEqualTo(15);
44+
}
45+
46+
@Test
47+
void testGetSpanStackTraceMinDurationMs_whenSpanStackTraceDefault_whenSpanFramesMinDurationIsZero_thenGetSwappedValueOfSpanFramesMinDuration() {
48+
ConfigurationRegistry configRegistry = SpyConfiguration.createSpyConfig(SimpleSource.forTest("span_frames_min_duration", "0ms"));
49+
50+
long actual = configRegistry.getConfig(StacktraceConfiguration.class).getSpanStackTraceMinDurationMs();
51+
assertThat(actual).isEqualTo(-1);
52+
}
53+
54+
@Test
55+
void testGetSpanStackTraceMinDurationMs_whenSpanStackTraceDefault_whenSpanFramesMinDurationIsNegative_thenGetSwappedValueOfSpanFramesMinDuration() {
56+
ConfigurationRegistry configRegistry = SpyConfiguration.createSpyConfig(SimpleSource.forTest("span_frames_min_duration", "-1ms"));
57+
58+
long actual = configRegistry.getConfig(StacktraceConfiguration.class).getSpanStackTraceMinDurationMs();
59+
assertThat(actual).isEqualTo(0);
60+
}
61+
62+
@Test
63+
void testGetSpanStackTraceMinDurationMs_defaultValue() {
64+
ConfigurationRegistry configRegistry = SpyConfiguration.createSpyConfig();
65+
66+
long actual = configRegistry.getConfig(StacktraceConfiguration.class).getSpanStackTraceMinDurationMs();
67+
assertThat(actual).isEqualTo(5);
68+
}
69+
}

‎apm-agent-core/src/test/java/org/example/stacktrace/StacktraceSerializationTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void setUp() throws IOException {
5858
tracer = MockTracer.createRealTracer();
5959
stacktraceConfiguration = tracer.getConfig(StacktraceConfiguration.class);
6060
// always enable
61-
when(stacktraceConfiguration.getSpanFramesMinDurationMs()).thenReturn(-1L);
61+
when(stacktraceConfiguration.getSpanStackTraceMinDurationMs()).thenReturn(0L);
6262
serializer = new DslJsonSerializer(stacktraceConfiguration, mock(ApmServerClient.class), tracer.getMetaData());
6363
objectMapper = new ObjectMapper();
6464
stacktrace = getStackTrace();

‎apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/main/java/co/elastic/apm/agent/rabbitmq/AmqpConstants.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
119
package co.elastic.apm.agent.rabbitmq;
220

321
public class AmqpConstants {

‎apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/main/java/co/elastic/apm/agent/rabbitmq/MessageBatchHelper.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
119
package co.elastic.apm.agent.rabbitmq;
220

321
import co.elastic.apm.agent.impl.ElasticApmTracer;

‎apm-agent-plugins/apm-rabbitmq/apm-rabbitmq-spring/src/main/java/co/elastic/apm/agent/rabbitmq/MessageBatchIteratorWrapper.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
119
package co.elastic.apm.agent.rabbitmq;
220

321

0 commit comments

Comments
 (0)