From 3e2ef885a5c02908e790e73a485b3a196fcbb7e3 Mon Sep 17 00:00:00 2001 From: dougqh Date: Tue, 3 Sep 2019 13:20:50 -0400 Subject: [PATCH] Ensure "client" spans do not set the language tag The core changes are in Config and ServerDecorator. Moved default tagging from Config::getRuntimeTags to Config::getLocalRootSpanTags. This changes the result of Config::getMergedJmxTags as well. To preserve language for servers changed ServerDecorator::afterStart. Other changes are in tests - the most complicated part is in TagsAssert::defaultTags. This now contains a bit too much conditional logic for my liking. --- .../trace/agent/decorator/ServerDecorator.java | 2 ++ .../trace/agent/decorator/ServerDecoratorTest.groovy | 2 ++ .../trace/agent/test/asserts/TagsAssert.groovy | 12 ++++++++++-- .../agent/test/server/http/TestHttpServer.groovy | 3 +++ .../src/main/java/datadog/trace/api/Config.java | 2 +- .../test/groovy/datadog/trace/api/ConfigTest.groovy | 8 ++++---- 6 files changed, 22 insertions(+), 7 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/ServerDecorator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/ServerDecorator.java index 653382e6e3..83366d94b3 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/ServerDecorator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/ServerDecorator.java @@ -2,6 +2,7 @@ package datadog.trace.agent.decorator; import io.opentracing.Span; import io.opentracing.tag.Tags; +import datadog.trace.api.Config; public abstract class ServerDecorator extends BaseDecorator { @@ -9,6 +10,7 @@ public abstract class ServerDecorator extends BaseDecorator { public Span afterStart(final Span span) { assert span != null; Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_SERVER); + span.setTag(Config.LANGUAGE_TAG_KEY, Config.LANGUAGE_TAG_VALUE); return super.afterStart(span); } } diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/ServerDecoratorTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/ServerDecoratorTest.groovy index cbae6a6757..4620151661 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/ServerDecoratorTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/ServerDecoratorTest.groovy @@ -1,5 +1,6 @@ package datadog.trace.agent.decorator +import datadog.trace.api.Config import datadog.trace.api.DDTags import io.opentracing.Span import io.opentracing.tag.Tags @@ -14,6 +15,7 @@ class ServerDecoratorTest extends BaseDecoratorTest { decorator.afterStart(span) then: + 1 * span.setTag(Config.LANGUAGE_TAG_KEY, Config.LANGUAGE_TAG_VALUE) 1 * span.setTag(Tags.COMPONENT.key, "test-component") 1 * span.setTag(Tags.SPAN_KIND.key, "server") 1 * span.setTag(DDTags.SPAN_TYPE, decorator.spanType()) diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy index 2052bead03..22bfe2ef51 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy @@ -4,6 +4,7 @@ import datadog.opentracing.DDSpan import datadog.trace.api.Config import groovy.transform.stc.ClosureParams import groovy.transform.stc.SimpleType +import io.opentracing.tag.Tags import java.util.regex.Pattern @@ -39,11 +40,18 @@ class TagsAssert { assert tags["thread.name"] != null assert tags["thread.id"] != null - if ("0" == spanParentId || distributedRootSpan) { + + boolean isRoot = ("0" == spanParentId) + if (isRoot || distributedRootSpan) { assert tags[Config.RUNTIME_ID_TAG] == Config.get().runtimeId - assert tags[Config.LANGUAGE_TAG_KEY] == Config.LANGUAGE_TAG_VALUE } else { assert tags[Config.RUNTIME_ID_TAG] == null + } + + boolean isServer = (tags[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_SERVER) + if (isRoot || distributedRootSpan || isServer) { + assert tags[Config.LANGUAGE_TAG_KEY] == Config.LANGUAGE_TAG_VALUE + } else { assert tags[Config.LANGUAGE_TAG_KEY] == null } } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/server/http/TestHttpServer.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/server/http/TestHttpServer.groovy index 0abfb6ecfc..f777af3b37 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/server/http/TestHttpServer.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/server/http/TestHttpServer.groovy @@ -5,6 +5,7 @@ import datadog.trace.agent.test.asserts.ListWriterAssert import io.opentracing.SpanContext import io.opentracing.Tracer import io.opentracing.propagation.Format +import io.opentracing.tag.Tags import io.opentracing.util.GlobalTracer import org.eclipse.jetty.http.HttpMethods import org.eclipse.jetty.server.Handler @@ -104,6 +105,7 @@ class TestHttpServer implements AutoCloseable { childOf(parentSpan) } tags { + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER defaultTags(parentSpan != null) } } @@ -235,6 +237,7 @@ class TestHttpServer implements AutoCloseable { tracer.extract(Format.Builtin.HTTP_HEADERS, new HttpServletRequestExtractAdapter(req)) def builder = tracer .buildSpan("test-http-server") + .withTag(Tags.SPAN_KIND.key, Tags.SPAN_KIND_SERVER) if (extractedContext != null) { builder.asChildOf(extractedContext) } diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index f309b50c6c..d19d2033df 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -424,6 +424,7 @@ public class Config { public Map getLocalRootSpanTags() { final Map runtimeTags = getRuntimeTags(); final Map result = new HashMap<>(runtimeTags); + result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); if (reportHostName) { final String hostName = getHostName(); @@ -484,7 +485,6 @@ public class Config { private Map getRuntimeTags() { final Map result = newHashMap(2); result.put(RUNTIME_ID_TAG, runtimeId); - result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); return Collections.unmodifiableMap(result); } diff --git a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 1bd60342b9..a6c68f07bd 100644 --- a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -76,7 +76,7 @@ class ConfigTest extends Specification { config.traceResolverEnabled == true config.serviceMapping == [:] config.mergedSpanTags == [:] - config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] + config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE): config.serviceName] config.headerTags == [:] config.httpServerErrorStatuses == (500..599).toSet() config.httpClientErrorStatuses == (400..499).toSet() @@ -150,7 +150,7 @@ class ConfigTest extends Specification { config.traceResolverEnabled == false config.serviceMapping == [a: "1"] config.mergedSpanTags == [b: "2", c: "3"] - config.mergedJmxTags == [b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] + config.mergedJmxTags == [b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE): config.serviceName] config.headerTags == [e: "5"] config.httpServerErrorStatuses == (122..457).toSet() config.httpClientErrorStatuses == (111..111).toSet() @@ -215,7 +215,7 @@ class ConfigTest extends Specification { config.traceResolverEnabled == false config.serviceMapping == [a: "1"] config.mergedSpanTags == [b: "2", c: "3"] - config.mergedJmxTags == [b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] + config.mergedJmxTags == [b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE): config.serviceName] config.headerTags == [e: "5"] config.httpServerErrorStatuses == (122..457).toSet() config.httpClientErrorStatuses == (111..111).toSet() @@ -405,7 +405,7 @@ class ConfigTest extends Specification { config.traceResolverEnabled == false config.serviceMapping == [a: "1"] config.mergedSpanTags == [b: "2", c: "3"] - config.mergedJmxTags == [b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] + config.mergedJmxTags == [b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE): config.serviceName] config.headerTags == [e: "5"] config.httpServerErrorStatuses == (122..457).toSet() config.httpClientErrorStatuses == (111..111).toSet()