-
Notifications
You must be signed in to change notification settings - Fork 293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a built-in trace interceptor for keeping traces depending of their latency #8040
Changes from 4 commits
3fefc0d
c1a7978
ea65fd8
955e382
f127aa7
16496e9
a287c8d
d63fbf5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package datadog.trace.core.traceinterceptor; | ||
|
||
import datadog.trace.api.Config; | ||
import datadog.trace.api.DDTags; | ||
import datadog.trace.api.interceptor.AbstractTraceInterceptor; | ||
import datadog.trace.api.interceptor.MutableSpan; | ||
import datadog.trace.api.interceptor.TraceInterceptor; | ||
import java.util.Collection; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
// This trace latency interceptor is disabled by default. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: you can use |
||
// We can activate it by setting the value of dd.trace.latency.interceptor.value to a positive value | ||
// This value should be in milliseconds and this interceptor will retain any local trace who has a | ||
// root | ||
// span duration greater than this value. | ||
// The activation of this interceptor is ignored if partial flush is enabled in order to avoid | ||
// incomplete local trace (incomplete chunk of trace). | ||
// Note that since we're changing the sampling priority at the end of local trace, there is no | ||
// guarantee to get complete traces, | ||
// since the original sampling priority for this trace may have already been propagated. | ||
|
||
public class LatencyTraceInterceptor extends AbstractTraceInterceptor { | ||
private static final Logger log = LoggerFactory.getLogger(LatencyTraceInterceptor.class); | ||
// duration configured in ms, need to be converted in nano seconds | ||
private static final int LATENCY = Config.get().getTraceLatencyInterceptorValue() * 1000000; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the latency is in nanoseconds it must be a |
||
|
||
public static final TraceInterceptor INSTANCE = | ||
new LatencyTraceInterceptor(Priority.ROOT_SPAN_LATENCY); | ||
|
||
protected LatencyTraceInterceptor(Priority priority) { | ||
super(priority); | ||
} | ||
|
||
@Override | ||
public Collection<? extends MutableSpan> onTraceComplete( | ||
Collection<? extends MutableSpan> latencyTrace) { | ||
if (latencyTrace.isEmpty()) { | ||
return latencyTrace; | ||
} | ||
MutableSpan rootSpan = latencyTrace.iterator().next().getLocalRootSpan(); | ||
if (rootSpan != null && rootSpan.getDurationNano() > LATENCY) { | ||
rootSpan.setTag(DDTags.MANUAL_KEEP, true); | ||
} | ||
return latencyTrace; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
package datadog.trace.core.traceinterceptor | ||
|
||
import datadog.trace.api.DDTags | ||
import datadog.trace.common.writer.ListWriter | ||
|
||
import datadog.trace.core.test.DDCoreSpecification | ||
|
||
import spock.lang.Timeout | ||
|
||
@Timeout(10) | ||
class LatencyTraceInterceptorTest extends DDCoreSpecification { | ||
|
||
|
||
def "test set sampling priority according to latency"() { | ||
setup: | ||
|
||
injectSysConfig("trace.partial.flush.enabled", partialFlushEnabled) | ||
injectSysConfig("trace.latency.interceptor.value", latencyThreshold) | ||
|
||
when: | ||
def writer = new ListWriter() | ||
def tracer = tracerBuilder().writer(writer).build() | ||
|
||
def spanSetup = tracer.buildSpan("test","my_operation_name").withTag(priorityTag, true).start() | ||
sleep(minDuration) | ||
spanSetup.finish() | ||
|
||
then: | ||
def trace = writer.firstTrace() | ||
trace.size() == 1 | ||
def span = trace[0] | ||
span.context().getSamplingPriority() == expected | ||
|
||
cleanup: | ||
tracer.close() | ||
|
||
where: | ||
partialFlushEnabled | latencyThreshold | priorityTag | minDuration | expected | ||
"true" | "200" | DDTags.MANUAL_KEEP | 10 | 2 | ||
"true" | "200" | DDTags.MANUAL_DROP | 10 | -1 | ||
"true" | "200" | DDTags.MANUAL_KEEP | 300 | 2 | ||
"true" | "200" | DDTags.MANUAL_DROP | 300 | -1 | ||
"false" | "200" | DDTags.MANUAL_KEEP | 10 | 2 | ||
"false" | "200" | DDTags.MANUAL_DROP | 10 | -1 | ||
"false" | "200" | DDTags.MANUAL_KEEP | 300 | 2 | ||
"false" | "200" | DDTags.MANUAL_DROP | 300 | 2 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -176,6 +176,8 @@ public static String getHostName() { | |
private final boolean scopeStrictMode; | ||
private final int scopeIterationKeepAlive; | ||
private final int partialFlushMinSpans; | ||
private final int traceLatencyInterceptorValue; | ||
private final boolean traceLatencyInterceptorEnabled; | ||
private final boolean traceStrictWritesEnabled; | ||
private final boolean logExtractHeaderNames; | ||
private final Set<PropagationStyle> propagationStylesToExtract; | ||
|
@@ -860,6 +862,12 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins | |
? 0 | ||
: configProvider.getInteger(PARTIAL_FLUSH_MIN_SPANS, DEFAULT_PARTIAL_FLUSH_MIN_SPANS); | ||
|
||
traceLatencyInterceptorValue = | ||
configProvider.getInteger( | ||
TRACE_LATENCY_INTERCEPTOR_VALUE, DEFAULT_TRACE_LATENCY_INTERCEPTOR_VALUE); | ||
|
||
traceLatencyInterceptorEnabled = !partialFlushEnabled && (traceLatencyInterceptorValue >= 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You do not need to set a negative default. I also would have ensured the minimum is strictly above 0 |
||
|
||
traceStrictWritesEnabled = configProvider.getBoolean(TRACE_STRICT_WRITES_ENABLED, false); | ||
|
||
logExtractHeaderNames = | ||
|
@@ -2075,6 +2083,14 @@ public int getPartialFlushMinSpans() { | |
return partialFlushMinSpans; | ||
} | ||
|
||
public int getTraceLatencyInterceptorValue() { | ||
return traceLatencyInterceptorValue; | ||
} | ||
|
||
public boolean isTraceLatencyInterceptorEnabled() { | ||
return traceLatencyInterceptorEnabled; | ||
} | ||
|
||
public boolean isTraceStrictWritesEnabled() { | ||
return traceStrictWritesEnabled; | ||
} | ||
|
@@ -4158,6 +4174,10 @@ public String toString() { | |
+ scopeIterationKeepAlive | ||
+ ", partialFlushMinSpans=" | ||
+ partialFlushMinSpans | ||
+ ", traceLatencyInterceptorEnabled=" | ||
+ traceLatencyInterceptorEnabled | ||
+ ", traceLatencyInterceptorValue=" | ||
+ traceLatencyInterceptorValue | ||
+ ", traceStrictWritesEnabled=" | ||
+ traceStrictWritesEnabled | ||
+ ", tracePropagationStylesToExtract=" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd have used something like
trace.keep.latency.threshold.ms