Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities;
import datadog.trace.bootstrap.instrumentation.api.SpanPrototype;
import datadog.trace.bootstrap.instrumentation.api.Tags;
import java.lang.reflect.Method;
import java.net.Inet4Address;
Expand Down Expand Up @@ -53,8 +54,9 @@ public String apply(Class<?> clazz) {

private final TagMap.Entry traceAnalyticsEntry;

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

protected BaseDecorator() {
final Config config = Config.get();
Expand All @@ -80,18 +82,38 @@ protected BaseDecorator() {

protected abstract CharSequence component();

/** Caches the component TagMap.Entry, so it isn't recreated for every trace */
protected final TagMap.Entry componentEntry() {
// DQH = Tried calling component() in the constructor, but that had issues with static
// field ordering. That was caught be an integration test, but I didn't want to risk
// breaking other integrations where the test is not as thorough.

// This approach while more complicated doesn't have any field initialization ordering issues.
TagMap.Entry componentEntry = cachedComponentEntry;
if (componentEntry == null) {
cachedComponentEntry = componentEntry = TagMap.Entry.create(Tags.COMPONENT, component());
/**
* The baked-once {@link SpanPrototype} carrying this decorator's constant identity and tags: span
* type, component, integration name, and — via the {@link ServerDecorator} / {@link
* ClientDecorator} extensions — span kind and language.
*
* <p>Built lazily on first access, not in the constructor: {@link #component()}, {@link
* #spanType()}, and (in {@link ClientDecorator}) {@code spanKind()} are overridable and may
* reference statics that are not yet initialized while the decorator singleton is under
* construction. Deferring the build sidesteps that field-initialization-ordering hazard (the same
* one the old per-{@link TagMap.Entry} caches guarded against) while collapsing those several
* caches into a single object. Not volatile: {@link SpanPrototype} is frozen, so a benign race
* rebuilds an equivalent prototype.
*/
protected final SpanPrototype spanPrototype() {
SpanPrototype prototype = cachedSpanPrototype;
if (prototype == null) {
cachedSpanPrototype = prototype = buildSpanPrototype();
}
return componentEntry;
return prototype;
}

/**
* Builds this decorator's {@link SpanPrototype}. Subclasses extend the chain with {@link
* SpanPrototype.Builder#extends_} to add their level's constants (see {@link ServerDecorator} /
* {@link ClientDecorator}), mirroring the decorator class hierarchy. Called once per decorator,
* lazily — see {@link #spanPrototype()}.
*/
protected SpanPrototype buildSpanPrototype() {
return SpanPrototype.builder()
.initSpanType(spanType())
.initComponentAndIntegration(component())
.build();
}

protected boolean traceAnalyticsDefault() {
Expand All @@ -109,16 +131,10 @@ public final void afterStart(final AgentSpan span) {
}

protected void doAfterStart(final AgentSpan span) {
if (spanType() != null) {
span.setSpanType(spanType());
}

span.setTag(componentEntry());

// DQH - Could retrieve the value from componentEntry and cast to avoid the virtual call,
// unclear which option is better here
final CharSequence component = component();
span.spanContext().setIntegrationName(component);
// Stamps the prototype's constant span type, tags, and integration name as fallback defaults.
// apply is the single seam the construction-seeding path shares; because it never clobbers, it
// self-neutralizes once construction has already seeded the same prototype.
span.apply(spanPrototype());

// null handled by setMetric
span.setMetric(traceAnalyticsEntry);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,45 +1,36 @@
package datadog.trace.bootstrap.instrumentation.decorator;

import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.SpanPrototype;
import datadog.trace.bootstrap.instrumentation.api.Tags;
import javax.annotation.ParametersAreNonnullByDefault;

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

protected abstract String service();

/** Caches span kind entry to reduce allocation */
private final TagMap.Entry spanKindEntry() {
// DQH - I considered moving the creation of the TagMap.Entry into a ClientDecorator
// constructor, but that introduces a subtle ordering requirement.

// If the spanKind method refers to a static that isn't yet initialized,
// then spanKind will return null when the Decorator singleton is being constructed.

// Such an ordering problem did occur with similar changes in BaseDecorator, so I've
// decided to be cautious here, too.
TagMap.Entry kindEntry = cachedSpanKindEntry;
if (kindEntry == null) {
cachedSpanKindEntry = kindEntry = TagMap.Entry.create(Tags.SPAN_KIND, spanKind());
}
return kindEntry;
}

protected String spanKind() {
return Tags.SPAN_KIND_CLIENT;
}

@Override
protected SpanPrototype buildSpanPrototype() {
// Extend the base prototype with the client-level span.kind. spanKind() is overridable and may
// read a not-yet-initialized static during singleton construction -- building lazily (via
// spanPrototype()) preserves the ordering safety the old cached spanKindEntry provided.
return SpanPrototype.builder()
.extends_(super.buildSpanPrototype())
.initKind(spanKind())
.build();
}

@Override
protected void doAfterStart(final AgentSpan span) {
final String service = service();
if (service != null) {
span.setServiceName(service, component());
}
span.setTag(spanKindEntry());

// Generate metrics for all client spans.
span.setMeasured(true);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
package datadog.trace.bootstrap.instrumentation.decorator;

import datadog.trace.api.DDTags;
import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.SpanPrototype;
import datadog.trace.bootstrap.instrumentation.api.Tags;
import javax.annotation.ParametersAreNonnullByDefault;

@ParametersAreNonnullByDefault
public abstract class ServerDecorator extends BaseDecorator {
private static final TagMap.Entry SPAN_KIND_ENTRY =
TagMap.Entry.create(Tags.SPAN_KIND, Tags.SPAN_KIND_SERVER);
private static final TagMap.Entry LANG_ENTRY =
TagMap.Entry.create(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE);

@Override
protected void doAfterStart(final AgentSpan span) {
span.setTag(SPAN_KIND_ENTRY);
span.setTag(LANG_ENTRY);

super.doAfterStart(span);
protected SpanPrototype buildSpanPrototype() {
// Extend the base prototype with the server-level constants (span.kind=server, language). The
// prototype chain mirrors the decorator class hierarchy; base afterStart applies the whole set.
return SpanPrototype.builder()
.extends_(super.buildSpanPrototype())
.initKind(Tags.SPAN_KIND_SERVER)
.initTag(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,20 @@ class BaseDecoratorTest extends DDSpecification {
def spanContext = Mock(AgentSpanContext)

def "test afterStart"() {
setup:
def recordingSpan = new RecordingSpan()

when:
decorator.afterStart(span)
decorator.afterStart(recordingSpan)

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

def "test onPeerConnection"() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
package datadog.trace.bootstrap.instrumentation.decorator

import datadog.trace.api.DDTags
import datadog.trace.api.TagMap
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext
import datadog.trace.bootstrap.instrumentation.api.Tags

class ClientDecoratorTest extends BaseDecoratorTest {

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

when:
decorator.afterStart(span)
decorator.afterStart(recordingSpan)

then:
def expected = ExpectedSpanState.expected()
.spanType(decorator.spanType())
.component("test-component")
.spanKind("client")
.measured(true)
.analyticsSampleRate(1.0d)
if (serviceName != null) {
1 * span.setServiceName(serviceName, "test-component")
expected.serviceName(serviceName, "test-component")
}
1 * span.setMeasured(true)
1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component"))
1 * span.spanContext() >> spanContext
1 * spanContext.setIntegrationName("test-component")
1 * span.setTag(TagMap.Entry.create(Tags.SPAN_KIND, "client"))
1 * span.setSpanType(decorator.spanType())
1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0))
_ * span.setTag(_)
_ * span.setTag(_, _) // Want to allow other calls from child implementations.
_ * span.setTag(_)
_ * span.setServiceName(_)
_ * span.setOperationName(_)
0 * _
// Polymorphic parent spec: subclass decorators (e.g. DB-type processing) layer on extra tags in
// afterStart, so tolerate additional tags while asserting the client-level scalars exactly.
expected.assertAppliedAllowingExtraTags(recordingSpan)

where:
serviceName << ["test-service", "other-service", null]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package datadog.trace.bootstrap.instrumentation.decorator

import datadog.trace.api.DDTags
import datadog.trace.api.TagMap
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext
import datadog.trace.bootstrap.instrumentation.api.Tags

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

when:
decorator.afterStart(span)
decorator.afterStart(recordingSpan)

then:
def expected = ExpectedSpanState.expected()
.spanType("test-type")
.component("test-component")
.spanKind("client")
.measured(true)
.analyticsSampleRate(1.0d)
if (serviceName != null) {
1 * span.setServiceName(serviceName, "test-component")
expected.serviceName(serviceName, "test-component")
}
1 * span.setMeasured(true)
1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component"))
1 * span.spanContext() >> spanContext
1 * spanContext.setIntegrationName("test-component")
1 * span.setTag(TagMap.Entry.create(Tags.SPAN_KIND, "client"))
1 * span.setSpanType("test-type")
1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0))
0 * _
expected.assertAppliedTo(recordingSpan)

where:
serviceName << ["test-service", "other-service", null]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,39 +1,27 @@
package datadog.trace.bootstrap.instrumentation.decorator

import datadog.trace.api.TagMap
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext

import static datadog.trace.api.DDTags.ANALYTICS_SAMPLE_RATE
import static datadog.trace.api.DDTags.LANGUAGE_TAG_KEY
import static datadog.trace.api.DDTags.LANGUAGE_TAG_VALUE
import static datadog.trace.bootstrap.instrumentation.api.Tags.COMPONENT
import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND

class ServerDecoratorTest extends BaseDecoratorTest {

def span = Mock(AgentSpan)

def "test afterStart"() {
setup:
def decorator = newDecorator()
def spanContext = Mock(AgentSpanContext)
def recordingSpan = new RecordingSpan()

when:
decorator.afterStart(span)
decorator.afterStart(recordingSpan)

then:
1 * span.setTag(TagMap.Entry.create(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE))
1 * span.setTag(TagMap.Entry.create(COMPONENT, "test-component"))
1 * span.spanContext() >> spanContext
1 * spanContext.setIntegrationName("test-component")
1 * span.setTag(TagMap.Entry.create(SPAN_KIND, "server"))
1 * span.setSpanType(decorator.spanType())
if (decorator.traceAnalyticsEnabled) {
1 * span.setMetric(TagMap.Entry.create(ANALYTICS_SAMPLE_RATE, 1.0))
} else {
1 * span.setMetric(null)
}
0 * _
ExpectedSpanState.expected()
.spanType(decorator.spanType())
.component("test-component")
.spanKind("server")
.language()
.analyticsSampleRate(decorator.traceAnalyticsEnabled ? 1.0d : null)
.assertAppliedTo(recordingSpan)
}

def "test beforeFinish"() {
Expand Down
Loading