Skip to content

Commit bcf8182

Browse files
tylerbensontylerbenson
committed
Add limit to trace scope depth
When limit is exceeded, a NoopScope is returned. Allow custom ScopeManager to be provided, with the plan to remove `ScopeContext` customization in the future.
1 parent d8bf995 commit bcf8182

7 files changed

Lines changed: 102 additions & 10 deletions

File tree

‎dd-trace-api/src/main/java/datadog/trace/api/Config.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ public class Config {
7676
public static final String HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN = "trace.http.client.split-by-domain";
7777
public static final String DB_CLIENT_HOST_SPLIT_BY_INSTANCE = "trace.db.client.split-by-instance";
7878
public static final String SPLIT_BY_TAGS = "trace.split-by-tags";
79+
public static final String SCOPE_DEPTH_LIMIT = "trace.scope.depth.limit";
7980
public static final String PARTIAL_FLUSH_MIN_SPANS = "trace.partial.flush.min.spans";
8081
public static final String RUNTIME_CONTEXT_FIELD_INJECTION =
8182
"trace.runtime.context.field.injection";
@@ -128,6 +129,7 @@ public class Config {
128129
private static final boolean DEFAULT_HTTP_CLIENT_SPLIT_BY_DOMAIN = false;
129130
private static final boolean DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE = false;
130131
private static final String DEFAULT_SPLIT_BY_TAGS = "";
132+
private static final int DEFAULT_SCOPE_DEPTH_LIMIT = 100;
131133
private static final int DEFAULT_PARTIAL_FLUSH_MIN_SPANS = 1000;
132134
private static final String DEFAULT_PROPAGATION_STYLE_EXTRACT = PropagationStyle.DATADOG.name();
133135
private static final String DEFAULT_PROPAGATION_STYLE_INJECT = PropagationStyle.DATADOG.name();
@@ -188,6 +190,7 @@ public enum PropagationStyle {
188190
@Getter private final boolean httpClientSplitByDomain;
189191
@Getter private final boolean dbClientSplitByInstance;
190192
@Getter private final Set<String> splitByTags;
193+
@Getter private final Integer scopeDepthLimit;
191194
@Getter private final Integer partialFlushMinSpans;
192195
@Getter private final boolean runtimeContextFieldInjection;
193196
@Getter private final Set<PropagationStyle> propagationStylesToExtract;
@@ -290,6 +293,9 @@ public enum PropagationStyle {
290293
new LinkedHashSet<>(
291294
getListSettingFromEnvironment(SPLIT_BY_TAGS, DEFAULT_SPLIT_BY_TAGS)));
292295

296+
scopeDepthLimit =
297+
getIntegerSettingFromEnvironment(SCOPE_DEPTH_LIMIT, DEFAULT_SCOPE_DEPTH_LIMIT);
298+
293299
partialFlushMinSpans =
294300
getIntegerSettingFromEnvironment(PARTIAL_FLUSH_MIN_SPANS, DEFAULT_PARTIAL_FLUSH_MIN_SPANS);
295301

@@ -418,6 +424,9 @@ private Config(final Properties properties, final Config parent) {
418424
getPropertyListValue(
419425
properties, SPLIT_BY_TAGS, new ArrayList<>(parent.splitByTags))));
420426

427+
scopeDepthLimit =
428+
getPropertyIntegerValue(properties, SCOPE_DEPTH_LIMIT, parent.scopeDepthLimit);
429+
421430
partialFlushMinSpans =
422431
getPropertyIntegerValue(properties, PARTIAL_FLUSH_MIN_SPANS, parent.partialFlushMinSpans);
423432

‎dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace
6262
/** Sampler defines the sampling policy in order to reduce the number of traces for instance */
6363
final Sampler sampler;
6464
/** Scope manager is in charge of managing the scopes from which spans are created */
65-
final ContextualScopeManager scopeManager = new ContextualScopeManager();
65+
final ScopeManager scopeManager;
6666

6767
/** A set of tags that are added only to the application's root span */
6868
private final Map<String, String> localRootSpanTags;
@@ -114,6 +114,7 @@ public Builder config(final Config config) {
114114
sampler(Sampler.Builder.forConfig(config));
115115
injector(HttpCodec.createInjector(config));
116116
extractor(HttpCodec.createExtractor(config, config.getHeaderTags()));
117+
scopeManager(new ContextualScopeManager(config.getScopeDepthLimit()));
117118
localRootSpanTags(config.getLocalRootSpanTags());
118119
defaultSpanTags(config.getMergedSpanTags());
119120
serviceNameMappings(config.getServiceMapping());
@@ -258,6 +259,7 @@ public DDTracer(
258259
sampler,
259260
HttpCodec.createInjector(Config.get()),
260261
HttpCodec.createExtractor(Config.get(), taggedHeaders),
262+
new ContextualScopeManager(Config.get().getScopeDepthLimit()),
261263
localRootSpanTags,
262264
defaultSpanTags,
263265
serviceNameMappings,
@@ -274,6 +276,7 @@ private DDTracer(
274276
final Sampler sampler,
275277
final HttpCodec.Injector injector,
276278
final HttpCodec.Extractor extractor,
279+
final ScopeManager scopeManager,
277280
final Map<String, String> localRootSpanTags,
278281
final Map<String, String> defaultSpanTags,
279282
final Map<String, String> serviceNameMappings,
@@ -294,6 +297,7 @@ private DDTracer(
294297
this.sampler = sampler;
295298
this.injector = injector;
296299
this.extractor = extractor;
300+
this.scopeManager = scopeManager;
297301
this.localRootSpanTags = localRootSpanTags;
298302
this.defaultSpanTags = defaultSpanTags;
299303
this.serviceNameMappings = serviceNameMappings;
@@ -365,7 +369,9 @@ public void addDecorator(final AbstractDecorator decorator) {
365369

366370
@Deprecated
367371
public void addScopeContext(final ScopeContext context) {
368-
scopeManager.addScopeContext(context);
372+
if (scopeManager instanceof ContextualScopeManager) {
373+
((ContextualScopeManager) scopeManager).addScopeContext(context);
374+
}
369375
}
370376

371377
/**
@@ -386,7 +392,7 @@ public void registerClassLoader(final ClassLoader classLoader) {
386392
}
387393

388394
@Override
389-
public ContextualScopeManager scopeManager() {
395+
public ScopeManager scopeManager() {
390396
return scopeManager;
391397
}
392398

@@ -510,7 +516,9 @@ public boolean addTraceInterceptor(final TraceInterceptor interceptor) {
510516

511517
@Override
512518
public void addScopeListener(final ScopeListener listener) {
513-
scopeManager.addScopeListener(listener);
519+
if (scopeManager instanceof ContextualScopeManager) {
520+
((ContextualScopeManager) scopeManager).addScopeListener(listener);
521+
}
514522
}
515523

516524
@Override

‎dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContextualScopeManager.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,35 @@
55
import io.opentracing.Scope;
66
import io.opentracing.ScopeManager;
77
import io.opentracing.Span;
8+
import io.opentracing.noop.NoopScopeManager;
89
import java.util.Deque;
910
import java.util.List;
1011
import java.util.concurrent.ConcurrentLinkedDeque;
1112
import java.util.concurrent.CopyOnWriteArrayList;
13+
import lombok.extern.slf4j.Slf4j;
1214

15+
@Slf4j
1316
public class ContextualScopeManager implements ScopeManager {
1417
static final ThreadLocal<DDScope> tlsScope = new ThreadLocal<>();
1518
final Deque<ScopeContext> scopeContexts = new ConcurrentLinkedDeque<>();
1619
final List<ScopeListener> scopeListeners = new CopyOnWriteArrayList<>();
1720

21+
private final int depthLimit;
22+
23+
public ContextualScopeManager(final int depthLimit) {
24+
this.depthLimit = depthLimit;
25+
}
26+
1827
@Override
1928
public Scope activate(final Span span, final boolean finishOnClose) {
29+
final Scope active = active();
30+
if (active instanceof DDScope) {
31+
final int currentDepth = ((DDScope) active).depth();
32+
if (depthLimit <= currentDepth) {
33+
log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth);
34+
return NoopScopeManager.NoopScope.INSTANCE;
35+
}
36+
}
2037
for (final ScopeContext context : scopeContexts) {
2138
if (context.inContext()) {
2239
return context.activate(span, finishOnClose);

‎dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ public class ContinuableScope implements DDScope, TraceScope {
2929
private final Continuation continuation;
3030
/** Flag to propagate this scope across async boundaries. */
3131
private final AtomicBoolean isAsyncPropagating = new AtomicBoolean(false);
32+
/** depth of scope on thread */
33+
private final int depth;
3234

3335
ContinuableScope(
3436
final ContextualScopeManager scopeManager,
@@ -51,6 +53,7 @@ private ContinuableScope(
5153
this.finishOnClose = finishOnClose;
5254
toRestore = scopeManager.tlsScope.get();
5355
scopeManager.tlsScope.set(this);
56+
depth = toRestore == null ? 0 : toRestore.depth() + 1;
5457
for (final ScopeListener listener : scopeManager.scopeListeners) {
5558
listener.afterScopeActivated();
5659
}
@@ -90,6 +93,11 @@ public DDSpan span() {
9093
return spanUnderScope;
9194
}
9295

96+
@Override
97+
public int depth() {
98+
return depth;
99+
}
100+
93101
@Override
94102
public boolean isAsyncPropagating() {
95103
return isAsyncPropagating.get();

‎dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/DDScope.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,6 @@
77
interface DDScope extends Scope {
88
@Override
99
Span span();
10+
11+
int depth();
1012
}

‎dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ public class SimpleScope implements DDScope {
99
private final Span spanUnderScope;
1010
private final boolean finishOnClose;
1111
private final DDScope toRestore;
12+
private final int depth;
1213

1314
public SimpleScope(
1415
final ContextualScopeManager scopeManager,
@@ -20,6 +21,7 @@ public SimpleScope(
2021
this.finishOnClose = finishOnClose;
2122
toRestore = scopeManager.tlsScope.get();
2223
scopeManager.tlsScope.set(this);
24+
depth = toRestore == null ? 0 : toRestore.depth() + 1;
2325
for (final ScopeListener listener : scopeManager.scopeListeners) {
2426
listener.afterScopeActivated();
2527
}
@@ -48,4 +50,9 @@ public void close() {
4850
public Span span() {
4951
return spanUnderScope;
5052
}
53+
54+
@Override
55+
public int depth() {
56+
return depth;
57+
}
5158
}

‎dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@ import datadog.trace.context.ScopeListener
88
import datadog.trace.util.gc.GCUtils
99
import datadog.trace.util.test.DDSpecification
1010
import io.opentracing.Scope
11+
import io.opentracing.ScopeManager
1112
import io.opentracing.Span
13+
import io.opentracing.noop.NoopScopeManager
1214
import io.opentracing.noop.NoopSpan
15+
import spock.lang.Shared
1316
import spock.lang.Subject
1417
import spock.lang.Timeout
1518

@@ -22,27 +25,35 @@ import java.util.concurrent.atomic.AtomicReference
2225
import static java.util.concurrent.TimeUnit.SECONDS
2326

2427
class ScopeManagerTest extends DDSpecification {
25-
def latch
26-
def writer
27-
def tracer
2828

29+
@Shared
30+
CountDownLatch latch
31+
@Shared
32+
ListWriter writer
33+
@Shared
34+
DDTracer tracer
35+
36+
@Shared
2937
@Subject
30-
def scopeManager
38+
ScopeManager scopeManager
3139

32-
def setup() {
40+
def setupSpec() {
3341
latch = new CountDownLatch(1)
3442
final currentLatch = latch
3543
writer = new ListWriter() {
3644
void incrementTraceCount() {
3745
currentLatch.countDown()
3846
}
3947
}
40-
tracer = new DDTracer(writer)
48+
tracer = DDTracer.builder().writer(writer).build()
4149
scopeManager = tracer.scopeManager()
4250
}
4351

4452
def cleanup() {
4553
scopeManager.tlsScope.remove()
54+
scopeManager.scopeContexts.clear()
55+
scopeManager.scopeListeners.clear()
56+
writer.clear()
4657
}
4758

4859
def "non-ddspan activation results in a simple scope"() {
@@ -129,6 +140,36 @@ class ScopeManagerTest extends DDSpecification {
129140
false | true
130141
}
131142

143+
def "scopemanager returns noop scope if depth exceeded"() {
144+
when: "fill up the scope stack"
145+
Scope scope = null
146+
for (int i = 0; i <= depth; i++) {
147+
scope = scopeManager.activate(NoopSpan.INSTANCE, false)
148+
assert scope instanceof SimpleScope
149+
}
150+
151+
then: "last scope is still valid"
152+
(scope as SimpleScope).depth() == depth
153+
154+
when: "activate a scope over the limit"
155+
scope = scopeManager.activate(NoopSpan.INSTANCE, false)
156+
157+
then: "a noop instance is returned"
158+
scope instanceof NoopScopeManager.NoopScope
159+
160+
when: "try again for good measure"
161+
scope = scopeManager.activate(NoopSpan.INSTANCE, false)
162+
163+
then: "still have a noop instance"
164+
scope instanceof NoopScopeManager.NoopScope
165+
166+
and: "scope stack not effected."
167+
(scopeManager.active() as SimpleScope).depth() == depth
168+
169+
where:
170+
depth = scopeManager.depthLimit
171+
}
172+
132173
def "ContinuableScope only creates continuations when propagation is set"() {
133174
setup:
134175
def builder = tracer.buildSpan("test")

0 commit comments

Comments
 (0)