Skip to content

Commit 48bc030

Browse files
mccullsdevflow.devflow-routing-intake
andauthored
Rework shim between ContextManager and the legacy scope manager (#11938)
Rework shim between ContextManager and legacy scope manager to allow for proper swapping over to ThreadLocalContextManager. Install legacy approach to managing contexts everywhere to begin with (as the migration proceeds we'll make it toggleable, then change the default depending on the Java version) Rename internal method for readability Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 47a760b commit 48bc030

11 files changed

Lines changed: 220 additions & 59 deletions

File tree

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,8 @@ public static void start(
337337
StaticEventLogger.end("crashtracking");
338338
}
339339

340+
AgentTracer.maybeInstallLegacyContextManager();
341+
340342
startDatadogAgent(initTelemetry, inst);
341343

342344
final EnumSet<Library> libraries = detectLibraries(log);

dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/nativeimage/TracerActivation.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import datadog.trace.agent.tooling.MeterInstaller;
99
import datadog.trace.agent.tooling.ProfilerInstaller;
1010
import datadog.trace.agent.tooling.TracerInstaller;
11+
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
1112
import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration;
1213
import org.slf4j.Logger;
1314
import org.slf4j.LoggerFactory;
@@ -18,6 +19,7 @@ public final class TracerActivation {
1819

1920
public static void activate() {
2021
try {
22+
AgentTracer.maybeInstallLegacyContextManager();
2123
// Initialize meter
2224
MeterInstaller.installMeter();
2325
// Initialize tracer

dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.CODE_ORIGIN_FO
1010
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.closePrevious
1111
import static datadog.trace.util.AgentThreadFactory.AgentThread.TASK_SCHEDULER
1212

13-
1413
import ch.qos.logback.classic.Level
1514
import ch.qos.logback.classic.util.ContextInitializer
1615
import com.datadog.debugger.agent.ClassesToRetransformFinder
@@ -351,18 +350,20 @@ abstract class InstrumentationSpecification extends DDSpecification implements A
351350
void setupSpec() {
352351
InstrumentationErrors.resetErrors()
353352

354-
AgentMeter.registerIfAbsent(
355-
STATS_D_CLIENT,
356-
new MonitoringImpl(STATS_D_CLIENT, 10, TimeUnit.SECONDS),
357-
DDSketchHistograms.FACTORY
358-
)
359-
360353
// If this fails, it's likely the result of another test loading Config before it can be
361354
// injected into the bootstrap classpath. If one test extends AgentTestRunner in a module, all tests must extend
362355
assert Config.getClassLoader() == null: "Config must load on the bootstrap classpath."
363356

364357
configurePreAgent()
365358

359+
AgentTracer.maybeInstallLegacyContextManager()
360+
361+
AgentMeter.registerIfAbsent(
362+
STATS_D_CLIENT,
363+
new MonitoringImpl(STATS_D_CLIENT, 10, TimeUnit.SECONDS),
364+
DDSketchHistograms.FACTORY
365+
)
366+
366367
TEST_DATA_STREAMS_WRITER = new RecordingDatastreamsPayloadWriter()
367368
DDAgentFeaturesDiscovery features = new MockFeaturesDiscovery(true)
368369

dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import datadog.trace.core.TraceCollector;
2424
import datadog.trace.junit.utils.config.WithConfig;
2525
import datadog.trace.junit.utils.context.AllowContextTestingExtension;
26+
import datadog.trace.junit.utils.context.LegacyContextTestingExtension;
2627
import java.lang.instrument.ClassFileTransformer;
2728
import java.lang.instrument.Instrumentation;
2829
import java.util.List;
@@ -54,7 +55,11 @@
5455
* </ul>
5556
*/
5657
@WithConfig(key = "detailed.instrumentation.errors", value = "true")
57-
@ExtendWith({TestClassShadowingExtension.class, AllowContextTestingExtension.class})
58+
@ExtendWith({
59+
TestClassShadowingExtension.class,
60+
AllowContextTestingExtension.class,
61+
LegacyContextTestingExtension.class
62+
})
5863
public abstract class AbstractInstrumentationTest {
5964
static final Instrumentation INSTRUMENTATION = ByteBuddyAgent.getInstrumentation();
6065

dd-trace-core/src/jmh/java/datadog/context/ContextManagerBenchmark.java

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,34 @@ public void setup() {
107107
}
108108

109109
static ContextManager createManager(String type) {
110-
return "Continuable".equals(type)
111-
? new ContinuableScopeManager(0, false)
112-
: ThreadLocalContextManager.INSTANCE;
110+
if ("Continuable".equals(type)) {
111+
ContinuableScopeManager csm = new ContinuableScopeManager(0, false);
112+
return new ContextManager() {
113+
@Override
114+
public Context current() {
115+
return csm.currentContext();
116+
}
117+
118+
@Override
119+
public ContextScope attach(Context ctx) {
120+
return csm.attach(ctx);
121+
}
122+
123+
@Override
124+
public Context swap(Context ctx) {
125+
return csm.swap(ctx);
126+
}
127+
128+
@Override
129+
public ContextContinuation capture(Context ctx) {
130+
return csm.capture(ctx);
131+
}
132+
133+
@Override
134+
public void addListener(ContextListener l) {}
135+
};
136+
}
137+
return ThreadLocalContextManager.INSTANCE;
113138
}
114139

115140
static Context[] createContexts() {

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
import datadog.communication.ddagent.DDAgentFeaturesDiscovery;
2323
import datadog.communication.ddagent.ExternalAgentLauncher;
2424
import datadog.communication.ddagent.SharedCommunicationObjects;
25+
import datadog.context.Context;
26+
import datadog.context.ContextContinuation;
27+
import datadog.context.ContextScope;
2528
import datadog.context.propagation.Propagators;
2629
import datadog.environment.ThreadSupport;
2730
import datadog.logging.RatelimitedLogger;
@@ -2465,4 +2468,24 @@ static TagMap withTracerTags(
24652468
}
24662469
return result.freeze();
24672470
}
2471+
2472+
@Override
2473+
public Context currentContext() {
2474+
return scopeManager.currentContext();
2475+
}
2476+
2477+
@Override
2478+
public ContextScope attach(Context context) {
2479+
return scopeManager.attach(context);
2480+
}
2481+
2482+
@Override
2483+
public Context swap(Context context) {
2484+
return scopeManager.swap(context);
2485+
}
2486+
2487+
@Override
2488+
public ContextContinuation capture(Context context) {
2489+
return scopeManager.capture(context);
2490+
}
24682491
}

dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414

1515
import datadog.context.Context;
1616
import datadog.context.ContextContinuation;
17-
import datadog.context.ContextListener;
18-
import datadog.context.ContextManager;
1917
import datadog.context.ContextScope;
2018
import datadog.logging.RatelimitedLogger;
2119
import datadog.trace.api.Config;
@@ -47,7 +45,7 @@
4745
* from being reported even if all related spans are finished. It also delegates to other
4846
* ScopeInterceptors to provide additional functionality.
4947
*/
50-
public final class ContinuableScopeManager implements ContextManager {
48+
public final class ContinuableScopeManager {
5149

5250
static final Logger log = LoggerFactory.getLogger(ContinuableScopeManager.class);
5351
static final RatelimitedLogger ratelimitedLog = new RatelimitedLogger(log, 1, MINUTES);
@@ -95,8 +93,6 @@ public ContinuableScopeManager(
9593
this.profilingContextIntegration = profilingContextIntegration;
9694
this.profilingEnabled =
9795
!(profilingContextIntegration instanceof ProfilingContextIntegration.NoOp);
98-
99-
ContextManager.register(this);
10096
}
10197

10298
public AgentScope activateSpan(final AgentSpan span) {
@@ -370,18 +366,15 @@ ScopeStack scopeStack() {
370366
return this.tlsScopeStack.get();
371367
}
372368

373-
@Override
374-
public Context current() {
369+
public Context currentContext() {
375370
final ContinuableScope active = scopeStack().active();
376371
return active == null ? Context.root() : active.context;
377372
}
378373

379-
@Override
380374
public ContextScope attach(@NonNull Context context) {
381375
return activate(context);
382376
}
383377

384-
@Override
385378
public Context swap(@NonNull Context context) {
386379
ScopeStack oldStack = tlsScopeStack.get();
387380
ContinuableScope oldScope = oldStack.top;
@@ -412,7 +405,6 @@ public Context swap(@NonNull Context context) {
412405
return new ScopeContext(oldStack);
413406
}
414407

415-
@Override
416408
public ContextContinuation capture(@NonNull Context context) {
417409
// respect async propagation flag for Context.current().capture()
418410
ContinuableScope activeScope = scopeStack().active();
@@ -431,12 +423,6 @@ public ContextContinuation capture(@NonNull Context context) {
431423
return new ScopeContinuation(this, context, CONTEXT, traceCollector).register();
432424
}
433425

434-
@Override
435-
public void addListener(@NonNull ContextListener unused) {
436-
// this new API is not expected to be used in legacy mode...
437-
log.warn("Unexpected call to ContextManager.addListener(...)");
438-
}
439-
440426
static final class ScopeStackThreadLocal extends ThreadLocal<ScopeStack> {
441427

442428
private final ProfilingContextIntegration profilingContextIntegration;

0 commit comments

Comments
 (0)