Skip to content

Commit 039404d

Browse files
mccullsmcculls
authored
Remove activeScope() use in OpenTracing shim (#8478)
This was only used to implement the deprecated 'active()' OpenTracing method. We take a best effort approach of using a 'fake' scope which lets the caller close the active scope if it matches the expected span, and otherwise logs a warning (or throws an exception in strict mode.)
1 parent c779bc9 commit 039404d

File tree

13 files changed

+157
-18
lines changed

13 files changed

+157
-18
lines changed

‎dd-java-agent/instrumentation/opentracing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/GlobalTracerInstrumentation.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public String[] helperClassNames() {
4444
packageName + ".OTTextMapSetter",
4545
packageName + ".OTScopeManager",
4646
packageName + ".OTScopeManager$OTScope",
47+
packageName + ".OTScopeManager$FakeScope",
4748
packageName + ".TypeConverter",
4849
packageName + ".OTSpan",
4950
packageName + ".OTSpanContext",

‎dd-java-agent/instrumentation/opentracing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/OTScopeManager.java

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package datadog.trace.instrumentation.opentracing31;
22

3+
import datadog.trace.api.Config;
34
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
45
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
56
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -8,8 +9,12 @@
89
import io.opentracing.Scope;
910
import io.opentracing.ScopeManager;
1011
import io.opentracing.Span;
12+
import org.slf4j.Logger;
13+
import org.slf4j.LoggerFactory;
1114

1215
public class OTScopeManager implements ScopeManager {
16+
static final Logger log = LoggerFactory.getLogger(OTScopeManager.class);
17+
1318
private final TypeConverter converter;
1419
private final AgentTracer.TracerAPI tracer;
1520

@@ -33,8 +38,12 @@ public Scope activate(final Span span, final boolean finishSpanOnClose) {
3338
@Deprecated
3439
@Override
3540
public Scope active() {
41+
AgentSpan agentSpan = tracer.activeSpan();
42+
if (null == agentSpan) {
43+
return null;
44+
}
3645
// WARNING... Making an assumption about finishSpanOnClose
37-
return converter.toScope(tracer.activeScope(), false);
46+
return new OTScope(new FakeScope(agentSpan), false, converter);
3847
}
3948

4049
static class OTScope implements Scope, TraceScope {
@@ -67,4 +76,33 @@ public boolean isFinishSpanOnClose() {
6776
return finishSpanOnClose;
6877
}
6978
}
79+
80+
private final class FakeScope implements AgentScope {
81+
private final AgentSpan agentSpan;
82+
83+
FakeScope(AgentSpan agentSpan) {
84+
this.agentSpan = agentSpan;
85+
}
86+
87+
@Override
88+
public AgentSpan span() {
89+
return agentSpan;
90+
}
91+
92+
@Override
93+
public byte source() {
94+
return ScopeSource.MANUAL.id();
95+
}
96+
97+
@Override
98+
public void close() {
99+
if (agentSpan == tracer.activeSpan()) {
100+
tracer.closeActive();
101+
} else if (Config.get().isScopeStrictMode()) {
102+
throw new RuntimeException("Tried to close " + agentSpan + " scope when not on top");
103+
} else {
104+
log.warn("Tried to close {} scope when not on top", agentSpan);
105+
}
106+
}
107+
}
70108
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ class OpenTracing31Test extends AgentTestRunner {
258258
firstScope.close()
259259

260260
then:
261-
tracer.scopeManager().active().delegate == secondScope.delegate
261+
tracer.scopeManager().active().span() == secondScope.span()
262262
_ * TEST_PROFILING_CONTEXT_INTEGRATION._
263263
0 * _
264264

‎dd-java-agent/instrumentation/opentracing/api-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/GlobalTracerInstrumentation.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public String[] helperClassNames() {
3535
packageName + ".OTTextMapInjectSetter",
3636
packageName + ".OTScopeManager",
3737
packageName + ".OTScopeManager$OTScope",
38+
packageName + ".OTScopeManager$FakeScope",
3839
packageName + ".TypeConverter",
3940
packageName + ".OTSpan",
4041
packageName + ".OTSpanContext",

‎dd-java-agent/instrumentation/opentracing/api-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/OTScopeManager.java

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package datadog.trace.instrumentation.opentracing32;
22

3+
import datadog.trace.api.Config;
34
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
45
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
56
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -8,8 +9,12 @@
89
import io.opentracing.Scope;
910
import io.opentracing.ScopeManager;
1011
import io.opentracing.Span;
12+
import org.slf4j.Logger;
13+
import org.slf4j.LoggerFactory;
1114

1215
public class OTScopeManager implements ScopeManager {
16+
static final Logger log = LoggerFactory.getLogger(OTScopeManager.class);
17+
1318
private final TypeConverter converter;
1419
private final AgentTracer.TracerAPI tracer;
1520

@@ -38,8 +43,12 @@ public Scope activate(final Span span, final boolean finishSpanOnClose) {
3843
@Deprecated
3944
@Override
4045
public Scope active() {
46+
AgentSpan agentSpan = tracer.activeSpan();
47+
if (null == agentSpan) {
48+
return null;
49+
}
4150
// WARNING... Making an assumption about finishSpanOnClose
42-
return converter.toScope(tracer.activeScope(), false);
51+
return new OTScope(new FakeScope(agentSpan), false, converter);
4352
}
4453

4554
@Override
@@ -77,4 +86,33 @@ public boolean isFinishSpanOnClose() {
7786
return finishSpanOnClose;
7887
}
7988
}
89+
90+
private final class FakeScope implements AgentScope {
91+
private final AgentSpan agentSpan;
92+
93+
FakeScope(AgentSpan agentSpan) {
94+
this.agentSpan = agentSpan;
95+
}
96+
97+
@Override
98+
public AgentSpan span() {
99+
return agentSpan;
100+
}
101+
102+
@Override
103+
public byte source() {
104+
return ScopeSource.MANUAL.id();
105+
}
106+
107+
@Override
108+
public void close() {
109+
if (agentSpan == tracer.activeSpan()) {
110+
tracer.closeActive();
111+
} else if (Config.get().isScopeStrictMode()) {
112+
throw new RuntimeException("Tried to close " + agentSpan + " scope when not on top");
113+
} else {
114+
log.warn("Tried to close {} scope when not on top", agentSpan);
115+
}
116+
}
117+
}
80118
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ class OpenTracing32Test extends AgentTestRunner {
273273
firstScope.close()
274274

275275
then:
276-
tracer.scopeManager().active().delegate == secondScope.delegate
276+
tracer.scopeManager().active().span() == secondScope.span()
277277
_ * TEST_PROFILING_CONTEXT_INTEGRATION._
278278
0 * _
279279

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,14 @@ public AgentScope activeScope() {
977977
return scopeManager.active();
978978
}
979979

980+
@Override
981+
public void closeActive() {
982+
AgentScope activeScope = this.scopeManager.active();
983+
if (activeScope != null) {
984+
activeScope.close();
985+
}
986+
}
987+
980988
@Override
981989
public AgentSpanContext notifyExtensionStart(Object event) {
982990
return LambdaHandler.notifyStartInvocation(this, event);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public final void close() {
5555
byte source = source();
5656
scopeManager.healthMetrics.onScopeCloseError(source);
5757
if (source == ScopeSource.MANUAL.id() && scopeManager.strictMode) {
58-
throw new RuntimeException("Tried to close scope when not on top");
58+
throw new RuntimeException("Tried to close " + span + " scope when not on top");
5959
}
6060
}
6161

‎dd-trace-ot/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ minimumInstructionCoverage = 0.5
1515
excludedClassesCoverage += [
1616
// This is mainly equals() and hashCode()
1717
"datadog.opentracing.OTScopeManager.OTScope",
18+
"datadog.opentracing.OTScopeManager.FakeScope",
1819
"datadog.opentracing.OTSpan",
1920
"datadog.opentracing.OTSpanContext",
2021
"datadog.opentracing.CustomScopeManagerWrapper.CustomScopeState",

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

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package datadog.opentracing;
22

3+
import datadog.trace.api.Config;
34
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
45
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
56
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -8,9 +9,13 @@
89
import io.opentracing.Scope;
910
import io.opentracing.ScopeManager;
1011
import io.opentracing.Span;
12+
import org.slf4j.Logger;
13+
import org.slf4j.LoggerFactory;
1114

1215
/** One of the two possible scope managers. See CustomScopeManagerWrapper */
1316
class OTScopeManager implements ScopeManager {
17+
static final Logger log = LoggerFactory.getLogger(OTScopeManager.class);
18+
1419
private final TypeConverter converter;
1520
private final AgentTracer.TracerAPI tracer;
1621

@@ -39,8 +44,12 @@ public Scope activate(final Span span, final boolean finishSpanOnClose) {
3944
@Deprecated
4045
@Override
4146
public Scope active() {
47+
AgentSpan agentSpan = tracer.activeSpan();
48+
if (null == agentSpan) {
49+
return null;
50+
}
4251
// WARNING... Making an assumption about finishSpanOnClose
43-
return converter.toScope(tracer.activeScope(), false);
52+
return new OTScope(new FakeScope(agentSpan), false, converter);
4453
}
4554

4655
@Override
@@ -83,16 +92,45 @@ public boolean equals(final Object o) {
8392
return false;
8493
}
8594
final OTScope otScope = (OTScope) o;
86-
return delegate.equals(otScope.delegate);
95+
return delegate.span().equals(otScope.delegate.span());
8796
}
8897

8998
@Override
9099
public int hashCode() {
91-
return delegate.hashCode();
100+
return delegate.span().hashCode();
92101
}
93102

94103
boolean isFinishSpanOnClose() {
95104
return finishSpanOnClose;
96105
}
97106
}
107+
108+
private final class FakeScope implements AgentScope {
109+
private final AgentSpan agentSpan;
110+
111+
FakeScope(AgentSpan agentSpan) {
112+
this.agentSpan = agentSpan;
113+
}
114+
115+
@Override
116+
public AgentSpan span() {
117+
return agentSpan;
118+
}
119+
120+
@Override
121+
public byte source() {
122+
return ScopeSource.MANUAL.id();
123+
}
124+
125+
@Override
126+
public void close() {
127+
if (agentSpan == tracer.activeSpan()) {
128+
tracer.closeActive();
129+
} else if (Config.get().isScopeStrictMode()) {
130+
throw new RuntimeException("Tried to close " + agentSpan + " scope when not on top");
131+
} else {
132+
log.warn("Tried to close {} scope when not on top", agentSpan);
133+
}
134+
}
135+
}
98136
}

0 commit comments

Comments
 (0)