8000 Refactor ScopeManager for future scoped context improvements (#4710) · DataDog/dd-trace-java@f2f7dd0 · GitHub
[go: up one dir, main page]
Skip to content

Commit f2f7dd0

Browse files
PerfectSlayerPerfectSlayer
authored
Refactor ScopeManager for future scoped context improvements (#4710)
Extract scope manager single and concurrent continuation, scope, scope stack Add scope close event to HealthMetrics to remove dep to StatsD Clean up AgentScopeManager API Clean up HealthMetrics
1 parent dfa0e4a commit f2f7dd0

File tree

23 files changed

+617
-594
lines changed

23 files changed

+617
-594
lines changed

‎dd-java-agent/instrumentation/opentelemetry/src/test/groovy/TypeConverterTest.groovy

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.api.DDSpanId
33
import datadog.trace.api.DDTraceId
4-
import datadog.trace.api.StatsDClient
54
import datadog.trace.api.sampling.PrioritySampling
65
import datadog.trace.bootstrap.instrumentation.api.AgentTracer
76
import datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopPathwayContext
87
import datadog.trace.bootstrap.instrumentation.api.ScopeSource
98
import datadog.trace.core.DDSpan
109
import datadog.trace.core.DDSpanContext
1110
import datadog.trace.core.PendingTrace
12-
import datadog.trace.core.monitor.HealthMetrics
1311
import datadog.trace.core.propagation.PropagationTags
1412
import datadog.trace.core.scopemanager.ContinuableScopeManager
1513
import datadog.trace.instrumentation.opentelemetry.TypeConverter
@@ -47,7 +45,7 @@ class TypeConverterTest extends AgentTestRunner {
4745
}
4846

4947
def "should avoid extra allocation for a scope wrapper"() {
50-
def scopeManager = new ContinuableScopeManager(0, StatsDClient.NO_OP, false, true, HealthMetrics.NO_OP)
48+
def scopeManager = new ContinuableScopeManager(0, false, true)
5149
def context = createTestSpanContext()
5250
def span1 = new DDSpan(0, context)
5351
def span2 = new DDSpan(0, context)

‎dd-java-agent/instrumentation/opentracing/api-0.31/src/test/groovy/TypeConverterTest.groovy

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.api.DDSpanId
33
import datadog.trace.api.DDTraceId
4-
import datadog.trace.api.StatsDClient
54
import datadog.trace.api.sampling.PrioritySampling
65
import datadog.trace.bootstrap.instrumentation.api.AgentTracer
76
import datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopPathwayContext
87
import datadog.trace.bootstrap.instrumentation.api.ScopeSource
98
import datadog.trace.core.DDSpan
109
import datadog.trace.core.DDSpanContext
1110
import datadog.trace.core.PendingTrace
12-
import datadog.trace.core.monitor.HealthMetrics
1311
import datadog.trace.core.propagation.PropagationTags
1412
import datadog.trace.core.scopemanager.ContinuableScopeManager
1513
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
@@ -52,7 +50,7 @@ class TypeConverterTest extends AgentTestRunner {
5250
}
5351

5452
def "should avoid extra allocation for a scope wrapper"() {
55-
def scopeManager = new ContinuableScopeManager(0, StatsDClient.NO_OP, false, true, HealthMetrics.NO_OP)
53+
def scopeManager = new ContinuableScopeManager(0, false, true)
5654
def context = createTestSpanContext()
5755
def span1 = new DDSpan(0, context)
5856
def span2 = new DDSpan(0, context)

‎dd-java-agent/instrumentation/opentracing/api-0.32/src/test/groovy/TypeConverterTest.groovy

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.api.DDSpanId
33
import datadog.trace.api.DDTraceId
4-
import datadog.trace.api.StatsDClient
54
import datadog.trace.api.sampling.PrioritySampling
65
import datadog.trace.bootstrap.instrumentation.api.AgentTracer
76
import datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopPathwayContext
87
import datadog.trace.bootstrap.instrumentation.api.ScopeSource
98
import datadog.trace.core.DDSpan
109
import datadog.trace.core.DDSpanContext
1110
import datadog.trace.core.PendingTrace
12-
import datadog.trace.core.monitor.HealthMetrics
1311
import datadog.trace.core.propagation.PropagationTags
1412
import datadog.trace.core.scopemanager.ContinuableScopeManager
1513
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
@@ -52,7 +50,7 @@ class TypeConverterTest extends AgentTestRunner {
5250
}
5351

5452
def "should avoid extra allocation for a scope wrapper"() {
55-
def scopeManager = new ContinuableScopeManager(0, StatsDClient.NO_OP, false, true, HealthMetrics.NO_OP)
53+
def scopeManager = new ContinuableScopeManager(0, false, true)
5654
def context = createTestSpanContext()
5755
def span1 = new DDSpan(0, context)
5856
def span2 = new DDSpan(0, context)

‎dd-trace-core/build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ excludedClassesCoverage += [
2020
'datadog.trace.core.PendingTraceBuffer.DelayingPendingTraceBuffer.FlushElement',
2121
'datadog.trace.core.StatusLogger',
2222
'datadog.trace.core.scopemanager.ContinuableScopeManager.ScopeStackThreadLocal',
23-
'datadog.trace.core.scopemanager.ContinuableScopeManager.SingleContinuation',
23+
'datadog.trace.core.scopemanager.SingleContinuation',
2424
'datadog.trace.core.SpanCorrelationImpl',
2525
'datadog.trace.core.Base64Encoder',
2626
'datadog.trace.core.CoreTracer.1',

‎dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -500,16 +500,13 @@ private CoreTracer(
500500

501501
this.traceWriteTimer = performanceMonitoring.newThreadLocalTimer("trace.write");
502502
if (scopeManager == null) {
503-
ContinuableScopeManager csm =
503+
this.scopeManager =
504504
new ContinuableScopeManager(
505505
config.getScopeDepthLimit(),
506-
this.statsDClient,
507506
config.isScopeStrictMode(),
508507
config.isScopeInheritAsyncPropagation(),
509508
profilingContextIntegration,
510509
this.healthMetrics);
511-
this.scopeManager = csm;
512-
513510
} else {
514511
this.scopeManager = scopeManager;
515512
}
@@ -702,8 +699,8 @@ public AgentScope activateSpan(AgentSpan span, ScopeSource source, boolean isAsy
702699
}
703700

704701
@Override
705-
public AgentScope.Continuation captureSpan(final AgentSpan span, ScopeSource source) {
706-
return scopeManager.captureSpan(span, source);
702+
public AgentScope.Continuation captureSpan(final AgentSpan span) {
703+
return scopeManager.captureSpan(span);
707704
}
708705

709706
@Override

‎dd-trace-core/src/main/java/datadog/trace/core/monitor/HealthMetrics.java

Lines changed: 22 additions & 20 deletions
F438
Original file line numberDiff line numberDiff line change
@@ -17,46 +17,48 @@
1717
* </ul>
1818
*/
1919
public abstract class HealthMetrics implements AutoCloseable {
20-
public static HealthMetrics NO_OP = new NoOpHealthMetrics();
20+
public static HealthMetrics NO_OP = new HealthMetrics() {};
2121

22-
public void start() {};
22+
public void start() {}
2323

24-
public void onStart(final int queueCapacity) {};
24+
public void onStart(final int queueCapacity) {}
2525

26-
public void onShutdown(final boolean flushSuccess) {};
26+
public void onShutdown(final boolean flushSuccess) {}
2727

28-
public void onPublish(final List<DDSpan> trace, final int samplingPriority) {};
28+
public void onPublish(final List<DDSpan> trace, final int samplingPriority) {}
2929

30-
public void onFailedPublish(final int samplingPriority) {};
30+
public void onFailedPublish(final int samplingPriority) {}
3131

32-
public void onPartialPublish(final int numberOfDroppedSpans) {};
32+
public void onPartialPublish(final int numberOfDroppedSpans) {}
3333

34-
public void onScheduleFlush(final boolean previousIncomplete) {};
34+
public void onScheduleFlush(final boolean previousIncomplete) {}
3535

36-
public void onFlush(final boolean early) {};
36+
public void onFlush(final boolean early) {}
3737

38-
public void onPartialFlush(final int sizeInBytes) {};
38+
public void onPartialFlush(final int sizeInBytes) {}
3939

40-
public void onSerialize(final int serializedSizeInBytes) {};
40+
public void onSerialize(final int serializedSizeInBytes) {}
4141

42-
public void onFailedSerialize(final List<DDSpan> trace, final Throwable optionalCause) {};
42+
public void onFailedSerialize(final List<DDSpan> trace, final Throwable optionalCause) {}
4343

44-
public void onCreateSpan() {};
44+
public void onCreateSpan() {}
4545

46-
public void onCreateTrace() {};
46+
public void onCreateTrace() {}
4747

48-
public void onCreateManualTrace() {};
48+
public void onCreateManualTrace() {}
4949

50-
public void onCancelContinuation() {};
50+
public void onScopeCloseError(int scopeSource) {}
5151

52-
public void onFinishContinuation() {};
52+
public void onCancelContinuation() {}
53+
54+
public void onFinishContinuation() {}
5355

5456
public void onSend(
55-
final int traceCount, final int sizeInBytes, final RemoteApi.Response response) {};
57+
final int traceCount, final int sizeInBytes, final RemoteApi.Response response) {}
5658

5759
public void onFailedSend(
58-
final int traceCount, final int sizeInBytes, final RemoteApi.Response response) {};
60+
final int traceCount, final int sizeInBytes, final RemoteApi.Response response) {}
5961

6062
@Override
61-
public void close() {};
63+
public void close() {}
6264
}

‎dd-trace-core/src/main/java/datadog/trace/core/monitor/NoOpHealthMetrics.java

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎dd-trace-core/src/main/java/datadog/trace/core/monitor/TracerHealthMetrics.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import datadog.trace.api.StatsDClient;
1313
import datadog.trace.api.cache.RadixTreeCache;
14+
import datadog.trace.bootstrap.instrumentation.api.ScopeSource;
1415
import datadog.trace.common.writer.RemoteApi;
1516
import datadog.trace.core.DDSpan;
1617
import datadog.trace.util.AgentTaskScheduler;
@@ -210,6 +211,14 @@ public void onCreateManualTrace() {
210211
manualTraces.inc();
211212
}
212213

214+
@Override
215+
public void onScopeCloseError(int scopeSource) {
216+
statsd.incrementCounter("scope.close.error", NO_TAGS);
217+
if (scopeSource == ScopeSource.MANUAL.id()) {
218+
statsd.incrementCounter("scope.user.close.error", NO_TAGS);
219+
}
220+
}
221+
213222
@Override
214223
public void onCancelContinuation() {
215224
cancelledContinuations.inc();

‎dd-trace-core/src/main/java/datadog/trace/core/scopemanager/AbstractContinuation.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package datadog.trace.core.scopemanager;
2+
3+
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
4+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
5+
import datadog.trace.bootstrap.instrumentation.api.AgentTrace;
6+
7+
/**
8+
* This class must not be a nested class of ContinuableScope to avoid an unconstrained chain of
9+
* references (using too much memory).
10+
*/
11+
abstract class AbstractContinuation implements AgentScope.Continuation {
12+
13+
final ContinuableScopeManager scopeManager;
14+
final AgentSpan spanUnderScope;
15+
final byte source;
16+
final AgentTrace trace;
17+
18+
public AbstractContinuation(
19+
ContinuableScopeManager scopeManager, AgentSpan spanUnderScope, byte source) {
20+
this.scopeManager = scopeManager;
21+
this.spanUnderScope = spanUnderScope;
22+
this.source = source;
23+
this.trace = spanUnderScope.context().getTrace();
24+
}
25+
26+
AbstractContinuation register() {
27+
trace.registerContinuation(this);
28+
return this;
29+
}
30+
31+
// Called by ContinuableScopeManager when a continued scope is closed
32+
// Can't use cancel() for SingleContinuation because of the "used" check
33+
abstract void cancelFromContinuedScopeClose();
34+
}

‎dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ConcurrentContinuation.java

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package datadog.trace.core.scopemanager;
2+
3+
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
4+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
5+
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
6+
7+
/**
8+
* This class must not be a nested class of ContinuableScope to avoid an unconstrained chain of
9+
* references (using too much memory).
10+
*
11+
* <p>This {@link AbstractContinuation} differs from the {@link SingleContinuation} in that if it is
12+
* activated, it needs to be canceled in addition to the returned {@link AgentScope} being closed.
13+
* This is to allow multiple concurrent threads that activate the continuation to race in a safe
14+
* way, and close the scopes without fear of closing the related {@link AgentSpan} prematurely.
15+
*/
16+
final class ConcurrentContinuation extends AbstractContinuation {
17+
private static final int START = 1;
18+
private static final int CLOSED = Integer.MIN_VALUE >> 1;
19+
private static final int BARRIER = Integer.MIN_VALUE >> 2;
20+
private volatile int count = START;
21+
22+
private static final AtomicIntegerFieldUpdater<ConcurrentContinuation> COUNT =
23+
AtomicIntegerFieldUpdater.newUpdater(ConcurrentContinuation.class, "count");
24+
25+
public ConcurrentContinuation(
26+
ContinuableScopeManager scopeManager, AgentSpan spanUnderScope, byte source) {
27+
super(scopeManager, spanUnderScope, source);
28+
}
29+
30+
private boolean tryActivate() {
31+
int current = COUNT.incrementAndGet(this);
32+
if (current < START) {
33+
COUNT.decrementAndGet(this);
34+
}
35+
return current > START;
36+
}
37+
38+
private boolean tryClose() {
39+
int current = COUNT.get(this);
40+
if (current < BARRIER) {
41+
return false;
42+
}
43+
// Now decrement the counter
44+
current = COUNT.decrementAndGet(this);
45+
// Try to close this if we are between START and BARRIER
46+
while (current < START && current > BARRIER) {
47+
if (COUNT.compareAndSet(this, current, CLOSED)) {
48+
return true;
49+
}
50+
current = COUNT.get(this);
51+
}
52+
return false;
53+
}
54+
55+
@Override
56+
public AgentScope activate() {
57+
if (tryActivate()) {
58+
return scopeManager.continueSpan(this, spanUnderScope, source);
59+
} else {
60+
return null;
61+
}
62+
}
63+
64+
@Override
65+
public void cancel() {
66+
if (tryClose()) {
67+
trace.cancelContinuation(this);
68+
}
69+
ContinuableScopeManager.log.debug(
70+
"t_id={} -> canceling continuation {}", spanUnderScope.getTraceId(), this);
71+
}
72+
73+
@Override
74+
public AgentSpan getSpan() {
75+
return spanUnderScope;
76+
}
77+
78+
@Override
79+
void cancelFromContinuedScopeClose() {
80+
cancel();
81+
}
82+
83+
@Override
84+
public String toString() {
85+
int c = COUNT.get(this);
86+
String s = c < BARRIER ? "CANCELED" : String.valueOf(c);
87+
return getClass().getSimpleName()
88+
+ "@"
89+
+ Integer.toHexString(hashCode())
90+
+ "("
91+
+ s
92+
+ ")->"
93+
+ spanUnderScope;
94+
}
95+
}

0 commit comments

Comments
 (0)
0