Skip to content

Commit 284eae7

Browse files
dougqhclaude
andcommitted
Apply SpanPrototype in decorator afterStart; migrate afterStart tests off mocks
Have BaseDecorator/ServerDecorator/ClientDecorator build a lazily-cached SpanPrototype (extension chain mirroring the decorator hierarchy) and apply it in afterStart via span.setSpanType/setAllTags/setIntegrationName, replacing the per-Entry setTag calls. Behavior-identical: setAllTags runs the same constant tags through the same interceptor path the per-tag calls used. Migrate the four afterStart specs from Spock mock-interaction assertions to a state-based harness (RecordingSpan/RecordingSpanContext accumulate applied state; ExpectedSpanState asserts the whole state at once), with three leniency modes matching Spock's polymorphic feature-method inheritance across the decorator hierarchy. Other specs (onPeerConnection/onConnection/onStatement/ beforeFinish) are unchanged. Also drop the born-dead SpanPrototype.Builder.initInstrumentationNames(String[]) overload (no caller); initInstrumentationName covers the single-name case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b8aa652 commit 284eae7

11 files changed

Lines changed: 597 additions & 131 deletions

File tree

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
1313
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1414
import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities;
15+
import datadog.trace.bootstrap.instrumentation.api.SpanPrototype;
1516
import datadog.trace.bootstrap.instrumentation.api.Tags;
1617
import java.lang.reflect.Method;
1718
import java.net.Inet4Address;
@@ -45,8 +46,9 @@ public String apply(Class<?> clazz) {
4546

4647
private final TagMap.Entry traceAnalyticsEntry;
4748

48-
// Deliberately not volatile, reading null and repeating the calculation is safe
49-
private TagMap.Entry cachedComponentEntry = null;
49+
// Deliberately not volatile: reading a stale null and rebuilding is safe. SpanPrototype is
50+
// frozen, so a benign race produces two equivalent prototypes and either is fine.
51+
private SpanPrototype cachedSpanPrototype = null;
5052

5153
protected BaseDecorator() {
5254
final Config config = Config.get();
@@ -72,35 +74,60 @@ protected BaseDecorator() {
7274

7375
protected abstract CharSequence component();
7476

75-
/** Caches the component TagMap.Entry, so it isn't recreated for every trace */
76-
protected final TagMap.Entry componentEntry() {
77-
// DQH = Tried calling component() in the constructor, but that had issues with static
78-
// field ordering. That was caught be an integration test, but I didn't want to risk
79-
// breaking other integrations where the test is not as thorough.
80-
81-
// This approach while more complicated doesn't have any field initialization ordering issues.
82-
TagMap.Entry componentEntry = cachedComponentEntry;
83-
if (componentEntry == null) {
84-
cachedComponentEntry = componentEntry = TagMap.Entry.create(Tags.COMPONENT, component());
77+
/**
78+
* The baked-once {@link SpanPrototype} carrying this decorator's constant identity and tags: span
79+
* type, component, integration name, and — via the {@link ServerDecorator} / {@link
80+
* ClientDecorator} extensions — span kind and language.
81+
*
82+
* <p>Built lazily on first access, not in the constructor: {@link #component()}, {@link
83+
* #spanType()}, and (in {@link ClientDecorator}) {@code spanKind()} are overridable and may
84+
* reference statics that are not yet initialized while the decorator singleton is under
85+
* construction. Deferring the build sidesteps that field-initialization-ordering hazard (the same
86+
* one the old per-{@link TagMap.Entry} caches guarded against) while collapsing those several
87+
* caches into a single object. Not volatile: {@link SpanPrototype} is frozen, so a benign race
88+
* rebuilds an equivalent prototype.
89+
*/
90+
protected final SpanPrototype spanPrototype() {
91+
SpanPrototype prototype = cachedSpanPrototype;
92+
if (prototype == null) {
93+
cachedSpanPrototype = prototype = buildSpanPrototype();
8594
}
86-
return componentEntry;
95+
return prototype;
96+
}
97+
98+
/**
99+
* Builds this decorator's {@link SpanPrototype}. Subclasses extend the chain with {@link
100+
* SpanPrototype.Builder#extends_} to add their level's constants (see {@link ServerDecorator} /
101+
* {@link ClientDecorator}), mirroring the decorator class hierarchy. Called once per decorator,
102+
* lazily — see {@link #spanPrototype()}.
103+
*/
104+
protected SpanPrototype buildSpanPrototype() {
105+
return SpanPrototype.builder()
106+
.initSpanType(spanType())
107+
.initComponentAndIntegration(component())
108+
.build();
87109
}
88110

89111
protected boolean traceAnalyticsDefault() {
90112
return false;
91113
}
92114

93115
public void afterStart(final AgentSpan span) {
94-
if (spanType() != null) {
95-
span.setSpanType(spanType());
116+
final SpanPrototype prototype = spanPrototype();
117+
118+
final CharSequence spanType = prototype.spanType();
119+
if (spanType != null) {
120+
span.setSpanType(spanType);
96121
}
97122

98-
span.setTag(componentEntry());
123+
// Reuses the prototype's frozen entries (setAllTags(TagMap) shares them) and still routes each
124+
// tag through the interceptor -- same as the per-Entry setTag calls this replaces.
125+
span.setAllTags(prototype.tags());
99126

100-
// DQH - Could retrieve the value from componentEntry and cast to avoid the virtual call,
101-
// unclear which option is better here
102-
final CharSequence component = component();
103-
span.spanContext().setIntegrationName(component);
127+
final CharSequence integrationName = prototype.integrationName();
128+
if (integrationName != null) {
129+
span.spanContext().setIntegrationName(integrationName);
130+
}
104131

105132
// null handled by setMetric
106133
span.setMetric(traceAnalyticsEntry);

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ClientDecorator.java

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,34 @@
11
package datadog.trace.bootstrap.instrumentation.decorator;
22

3-
import datadog.trace.api.TagMap;
43
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
4+
import datadog.trace.bootstrap.instrumentation.api.SpanPrototype;
55
import datadog.trace.bootstrap.instrumentation.api.Tags;
66

77
public abstract class ClientDecorator extends BaseDecorator {
8-
// Deliberately not volatile, reading a stale null and creating an extra Entry is safe
9-
private TagMap.Entry cachedSpanKindEntry = null;
108

119
protected abstract String service();
1210

13-
/** Caches span kind entry to reduce allocation */
14-
private final TagMap.Entry spanKindEntry() {
15-
// DQH - I considered moving the creation of the TagMap.Entry into a ClientDecorator
16-
// constructor, but that introduces a subtle ordering requirement.
17-
18-
// If the spanKind method refers to a static that isn't yet initialized,
19-
// then spanKind will return null when the Decorator singleton is being constructed.
20-
21-
// Such an ordering problem did occur with similar changes in BaseDecorator, so I've
22-
// decided to be cautious here, too.
23-
TagMap.Entry kindEntry = cachedSpanKindEntry;
24-
if (kindEntry == null) {
25-
cachedSpanKindEntry = kindEntry = TagMap.Entry.create(Tags.SPAN_KIND, spanKind());
26-
}
27-
return kindEntry;
28-
}
29-
3011
protected String spanKind() {
3112
return Tags.SPAN_KIND_CLIENT;
3213
}
3314

15+
@Override
16+
protected SpanPrototype buildSpanPrototype() {
17+
// Extend the base prototype with the client-level span.kind. spanKind() is overridable and may
18+
// read a not-yet-initialized static during singleton construction -- building lazily (via
19+
// spanPrototype()) preserves the ordering safety the old cached spanKindEntry provided.
20+
return SpanPrototype.builder()
21+
.extends_(super.buildSpanPrototype())
22+
.initKind(spanKind())
23+
.build();
24+
}
25+
3426
@Override
3527
public void afterStart(final AgentSpan span) {
3628
final String service = service();
3729
if (service != null) {
3830
span.setServiceName(service, component());
3931
}
40-
span.setTag(spanKindEntry());
4132

4233
// Generate metrics for all client spans.
4334
span.setMeasured(true);
Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,19 @@
11
package datadog.trace.bootstrap.instrumentation.decorator;
22

33
import datadog.trace.api.DDTags;
4-
import datadog.trace.api.TagMap;
5-
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
4+
import datadog.trace.bootstrap.instrumentation.api.SpanPrototype;
65
import datadog.trace.bootstrap.instrumentation.api.Tags;
76

87
public abstract class ServerDecorator extends BaseDecorator {
9-
private static final TagMap.Entry SPAN_KIND_ENTRY =
10-
TagMap.Entry.create(Tags.SPAN_KIND, Tags.SPAN_KIND_SERVER);
11-
private static final TagMap.Entry LANG_ENTRY =
12-
TagMap.Entry.create(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE);
138

149
@Override
15-
public void afterStart(final AgentSpan span) {
16-
span.setTag(SPAN_KIND_ENTRY);
17-
span.setTag(LANG_ENTRY);
18-
19-
super.afterStart(span);
10+
protected SpanPrototype buildSpanPrototype() {
11+
// Extend the base prototype with the server-level constants (span.kind=server, language). The
12+
// prototype chain mirrors the decorator class hierarchy; base afterStart applies the whole set.
13+
return SpanPrototype.builder()
14+
.extends_(super.buildSpanPrototype())
15+
.initKind(Tags.SPAN_KIND_SERVER)
16+
.initTag(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE)
17+
.build();
2018
}
2119
}

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package datadog.trace.bootstrap.instrumentation.decorator
22

3-
import datadog.trace.api.TagMap
43
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
54
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext
65
import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities
@@ -25,25 +24,20 @@ class BaseDecoratorTest extends DDSpecification {
2524
def spanContext = Mock(AgentSpanContext)
2625

2726
def "test afterStart"() {
27+
setup:
28+
def recordingSpan = new RecordingSpan()
29+
2830
when:
29-
decorator.afterStart(span)
31+
decorator.afterStart(recordingSpan)
3032

3133
then:
32-
1 * span.setSpanType(decorator.spanType())
33-
1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component"))
34-
1 * span.spanContext() >> spanContext
35-
1 * spanContext.setIntegrationName("test-component")
36-
_ * span.setTag(_)
37-
_ * span.setTag(_, _) // Want to allow other calls from child implementations.
38-
_ * span.setTag(_)
39-
_ * span.setMeasured(true)
40-
_ * span.setMetric(_)
41-
_ * span.setMetric(_, _)
42-
_ * span.setMetric(_)
43-
_ * span.setServiceName(_, _)
44-
_ * span.setOperationName(_)
45-
_ * span.setSamplingPriority(_)
46-
0 * _
34+
// The base spec runs polymorphically against every subclass decorator, so it only asserts the
35+
// baseline identity every decorator applies, tolerating the tags subclasses layer on. Each
36+
// level's exact tag set is asserted by its own afterStart spec.
37+
ExpectedSpanState.expected()
38+
.spanType(decorator.spanType())
39+
.component("test-component")
40+
.assertIdentityAppliedTo(recordingSpan)
4741
}
4842

4943
def "test onPeerConnection"() {

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
package datadog.trace.bootstrap.instrumentation.decorator
22

3-
import datadog.trace.api.DDTags
4-
import datadog.trace.api.TagMap
53
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
6-
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext
7-
import datadog.trace.bootstrap.instrumentation.api.Tags
84

95
class ClientDecoratorTest extends BaseDecoratorTest {
106

@@ -13,28 +9,24 @@ class ClientDecoratorTest extends BaseDecoratorTest {
139
def "test afterStart"() {
1410
setup:
1511
def decorator = newDecorator((String) serviceName)
16-
def spanContext = Mock(AgentSpanContext)
12+
def recordingSpan = new RecordingSpan()
1713

1814
when:
19-
decorator.afterStart(span)
15+
decorator.afterStart(recordingSpan)
2016

2117
then:
18+
def expected = ExpectedSpanState.expected()
19+
.spanType(decorator.spanType())
20+
.component("test-component")
21+
.spanKind("client")
22+
.measured(true)
23+
.analyticsSampleRate(1.0d)
2224
if (serviceName != null) {
23-
1 * span.setServiceName(serviceName, "test-component")
25+
expected.serviceName(serviceName, "test-component")
2426
}
25-
1 * span.setMeasured(true)
26-
1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component"))
27-
1 * span.spanContext() >> spanContext
28-
1 * spanContext.setIntegrationName("test-component")
29-
1 * span.setTag(TagMap.Entry.create(Tags.SPAN_KIND, "client"))
30-
1 * span.setSpanType(decorator.spanType())
31-
1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0))
32-
_ * span.setTag(_)
33-
_ * span.setTag(_, _) // Want to allow other calls from child implementations.
34-
_ * span.setTag(_)
35-
_ * span.setServiceName(_)
36-
_ * span.setOperationName(_)
37-
0 * _
27+
// Polymorphic parent spec: subclass decorators (e.g. DB-type processing) layer on extra tags in
28+
// afterStart, so tolerate additional tags while asserting the client-level scalars exactly.
29+
expected.assertAppliedAllowingExtraTags(recordingSpan)
3830

3931
where:
4032
serviceName << ["test-service", "other-service", null]

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
package datadog.trace.bootstrap.instrumentation.decorator
22

3-
import datadog.trace.api.DDTags
4-
import datadog.trace.api.TagMap
53
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
6-
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext
74
import datadog.trace.bootstrap.instrumentation.api.Tags
85

96
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_HOST
@@ -17,23 +14,22 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest {
1714
def "test afterStart"() {
1815
setup:
1916
def decorator = newDecorator((String) serviceName)
20-
def spanContext = Mock(AgentSpanContext)
17+
def recordingSpan = new RecordingSpan()
2118

2219
when:
23-
decorator.afterStart(span)
20+
decorator.afterStart(recordingSpan)
2421

2522
then:
23+
def expected = ExpectedSpanState.expected()
24+
.spanType("test-type")
25+
.component("test-component")
26+
.spanKind("client")
27+
.measured(true)
28+
.analyticsSampleRate(1.0d)
2629
if (serviceName != null) {
27-
1 * span.setServiceName(serviceName, "test-component")
30+
expected.serviceName(serviceName, "test-component")
2831
}
29-
1 * span.setMeasured(true)
30-
1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component"))
31-
1 * span.spanContext() >> spanContext
32-
1 * spanContext.setIntegrationName("test-component")
33-
1 * span.setTag(TagMap.Entry.create(Tags.SPAN_KIND, "client"))
34-
1 * span.setSpanType("test-type")
35-
1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0))
36-
0 * _
32+
expected.assertAppliedTo(recordingSpan)
3733

3834
where:
3935
serviceName << ["test-service", "other-service", null]

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ServerDecoratorTest.groovy

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,27 @@
11
package datadog.trace.bootstrap.instrumentation.decorator
22

3-
import datadog.trace.api.TagMap
43
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
5-
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext
6-
7-
import static datadog.trace.api.DDTags.ANALYTICS_SAMPLE_RATE
8-
import static datadog.trace.api.DDTags.LANGUAGE_TAG_KEY
9-
import static datadog.trace.api.DDTags.LANGUAGE_TAG_VALUE
10-
import static datadog.trace.bootstrap.instrumentation.api.Tags.COMPONENT
11-
import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND
124

135
class ServerDecoratorTest extends BaseDecoratorTest {
146

157
def span = Mock(AgentSpan)
168

179
def "test afterStart"() {
10+
setup:
1811
def decorator = newDecorator()
19-
def spanContext = Mock(AgentSpanContext)
12+
def recordingSpan = new RecordingSpan()
2013

2114
when:
22-
decorator.afterStart(span)
15+
decorator.afterStart(recordingSpan)
2316

2417
then:
25-
1 * span.setTag(TagMap.Entry.create(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE))
26-
1 * span.setTag(TagMap.Entry.create(COMPONENT, "test-component"))
27-
1 * span.spanContext() >> spanContext
28-
1 * spanContext.setIntegrationName("test-component")
29-
1 * span.setTag(TagMap.Entry.create(SPAN_KIND, "server"))
30-
1 * span.setSpanType(decorator.spanType())
31-
if (decorator.traceAnalyticsEnabled) {
32-
1 * span.setMetric(TagMap.Entry.create(ANALYTICS_SAMPLE_RATE, 1.0))
33-
} else {
34-
1 * span.setMetric(null)
35-
}
36-
0 * _
18+
ExpectedSpanState.expected()
19+
.spanType(decorator.spanType())
20+
.component("test-component")
21+
.spanKind("server")
22+
.language()
23+
.analyticsSampleRate(decorator.traceAnalyticsEnabled ? 1.0d : null)
24+
.assertAppliedTo(recordingSpan)
3725
}
3826

3927
def "test beforeFinish"() {

0 commit comments

Comments
 (0)