diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 13d51bb4c8..ffc85ece5f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,8 +20,8 @@ Other source files (Groovy, Scala, etc) should ideally be formatted by Intellij Suggested plugins and settings: * Editor > Code Style > Java/Groovy > Imports - * Class count to use import with '*': `10` (some number sufficiently large that is unlikely to matter) - * Names count to use static import with '*': `10` + * Class count to use import with '*': `50` (some number sufficiently large that is unlikely to matter) + * Names count to use static import with '*': `50` * With java use the following import layout (groovy should still use the default) to ensure consistency with google-java-format: ![import layout](https://user-images.githubusercontent.com/734411/43430811-28442636-94ae-11e8-86f1-f270ddcba023.png) * [Google Java Format](https://plugins.jetbrains.com/plugin/8527-google-java-format) 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 9992f041f6..e5afb50b43 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 @@ -1,5 +1,7 @@ package datadog.trace.api; +import java.net.InetAddress; +import java.net.UnknownHostException; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -51,6 +53,7 @@ public class Config { public static final String TRACE_ANNOTATIONS = "trace.annotations"; public static final String TRACE_METHODS = "trace.methods"; public static final String TRACE_CLASSES_EXCLUDE = "trace.classes.exclude"; + public static final String TRACE_REPORT_HOSTNAME = "trace.report-hostname"; public static final String HEADER_TAGS = "trace.header.tags"; public static final String HTTP_SERVER_ERROR_STATUSES = "http.server.error.statuses"; public static final String HTTP_CLIENT_ERROR_STATUSES = "http.client.error.statuses"; @@ -105,11 +108,16 @@ public class Config { private static final String SPLIT_BY_SPACE_OR_COMMA_REGEX = "[,\\s]+"; + private static final boolean DEFAULT_TRACE_REPORT_HOSTNAME = false; + public enum PropagationStyle { DATADOG, B3 } + /** A tag intended for internal use only, hence not added to the public api DDTags class. */ + private static final String INTERNAL_HOST_NAME = "_dd.hostname"; + /** * this is a random UUID that gets generated on JVM start up and is attached to every root span * and every JMX metric that is sent out. @@ -147,6 +155,8 @@ public class Config { @Getter private final boolean logsInjectionEnabled; + @Getter private final boolean reportHostName; + // Read order: System Properties -> Env Variables, [-> default value] // Visible for testing Config() { @@ -221,6 +231,9 @@ public class Config { logsInjectionEnabled = getBooleanSettingFromEnvironment(LOGS_INJECTION_ENABLED, DEFAULT_LOGS_INJECTION_ENABLED); + reportHostName = + getBooleanSettingFromEnvironment(TRACE_REPORT_HOSTNAME, DEFAULT_TRACE_REPORT_HOSTNAME); + log.debug("New instance: {}", this); } @@ -301,9 +314,24 @@ public class Config { logsInjectionEnabled = getBooleanSettingFromEnvironment(LOGS_INJECTION_ENABLED, DEFAULT_LOGS_INJECTION_ENABLED); + reportHostName = + getPropertyBooleanValue(properties, TRACE_REPORT_HOSTNAME, parent.reportHostName); + log.debug("New instance: {}", this); } + /** @return A map of tags to be applied only to the local application root span. */ + public Map getLocalRootSpanTags() { + final Map runtimeTags = getRuntimeTags(); + final Map result = + newHashMap(reportHostName ? (runtimeTags.size() + 1) : runtimeTags.size()); + result.putAll(runtimeTags); + if (reportHostName) { + result.put(INTERNAL_HOST_NAME, getHostname()); + } + return Collections.unmodifiableMap(result); + } + public Map getMergedSpanTags() { // DO not include runtimeId into span tags: we only want that added to the root span final Map result = newHashMap(globalTags.size() + spanTags.size()); @@ -336,7 +364,7 @@ public class Config { * * @return A map of tag-name -> tag-value */ - public Map getRuntimeTags() { + private Map getRuntimeTags() { final Map result = newHashMap(2); result.put(RUNTIME_ID_TAG, runtimeId); result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); @@ -636,6 +664,20 @@ public class Config { return Collections.unmodifiableSet(result); } + /** + * Returns the detected hostname. This operation is time consuming so if the usage changes and + * this method will be called several times then we should implement some sort of caching. + */ + private String getHostname() { + try { + return InetAddress.getLocalHost().getHostName(); + } catch (UnknownHostException e) { + // If we are not able to detect the hostname we do not throw an exception. + } + + return null; + } + // This has to be placed after all other static fields to give them a chance to initialize private static final Config INSTANCE = new Config(); diff --git a/dd-trace-api/src/main/java/datadog/trace/api/interceptor/MutableSpan.java b/dd-trace-api/src/main/java/datadog/trace/api/interceptor/MutableSpan.java index 7ec35377f6..75d71d264a 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/interceptor/MutableSpan.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/interceptor/MutableSpan.java @@ -50,5 +50,16 @@ public interface MutableSpan { MutableSpan setError(boolean value); + /** @deprecated Use {@link #getLocalRootSpan()} instead. */ + @Deprecated MutableSpan getRootSpan(); + + /** + * Returns the root span for current the trace fragment. In the context of distributed tracing + * this method returns the root span only for the fragment generated by the currently traced + * application. + * + * @return The root span for the current trace fragment. + */ + MutableSpan getLocalRootSpan(); } 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 1ca064b5e1..d62eb9d47d 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 @@ -35,6 +35,7 @@ import static datadog.trace.api.Config.SERVICE_MAPPING import static datadog.trace.api.Config.SERVICE_NAME import static datadog.trace.api.Config.SPAN_TAGS import static datadog.trace.api.Config.TRACE_AGENT_PORT +import static datadog.trace.api.Config.TRACE_REPORT_HOSTNAME import static datadog.trace.api.Config.TRACE_ENABLED import static datadog.trace.api.Config.TRACE_RESOLVER_ENABLED import static datadog.trace.api.Config.WRITER_TYPE @@ -56,6 +57,7 @@ class ConfigTest extends Specification { private static final DD_JMXFETCH_METRICS_CONFIGS_ENV = "DD_JMXFETCH_METRICS_CONFIGS" private static final DD_TRACE_AGENT_PORT_ENV = "DD_TRACE_AGENT_PORT" private static final DD_AGENT_PORT_LEGACY_ENV = "DD_AGENT_PORT" + private static final DD_TRACE_REPORT_HOSTNAME = "DD_TRACE_REPORT_HOSTNAME" def "verify defaults"() { when: @@ -78,6 +80,7 @@ class ConfigTest extends Specification { config.httpClientErrorStatuses == (400..499).toSet() config.httpClientSplitByDomain == false config.partialFlushMinSpans == 1000 + config.reportHostName == false config.runtimeContextFieldInjection == true config.propagationStylesToExtract.toList() == [Config.PropagationStyle.DATADOG] config.propagationStylesToInject.toList() == [Config.PropagationStyle.DATADOG] @@ -118,6 +121,7 @@ class ConfigTest extends Specification { prop.setProperty(HTTP_CLIENT_ERROR_STATUSES, "111") prop.setProperty(HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "true") prop.setProperty(PARTIAL_FLUSH_MIN_SPANS, "15") + prop.setProperty(TRACE_REPORT_HOSTNAME, "true") prop.setProperty(RUNTIME_CONTEXT_FIELD_INJECTION, "false") prop.setProperty(PROPAGATION_STYLE_EXTRACT, "Datadog, B3") prop.setProperty(PROPAGATION_STYLE_INJECT, "B3, Datadog") @@ -148,6 +152,7 @@ class ConfigTest extends Specification { config.httpClientErrorStatuses == (111..111).toSet() config.httpClientSplitByDomain == true config.partialFlushMinSpans == 15 + config.reportHostName == true config.runtimeContextFieldInjection == false config.propagationStylesToExtract.toList() == [Config.PropagationStyle.DATADOG, Config.PropagationStyle.B3] config.propagationStylesToInject.toList() == [Config.PropagationStyle.B3, Config.PropagationStyle.DATADOG] @@ -179,6 +184,7 @@ class ConfigTest extends Specification { System.setProperty(PREFIX + HTTP_CLIENT_ERROR_STATUSES, "111") System.setProperty(PREFIX + HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "true") System.setProperty(PREFIX + PARTIAL_FLUSH_MIN_SPANS, "25") + System.setProperty(PREFIX + TRACE_REPORT_HOSTNAME, "true") System.setProperty(PREFIX + RUNTIME_CONTEXT_FIELD_INJECTION, "false") System.setProperty(PREFIX + PROPAGATION_STYLE_EXTRACT, "Datadog, B3") System.setProperty(PREFIX + PROPAGATION_STYLE_INJECT, "B3, Datadog") @@ -209,6 +215,7 @@ class ConfigTest extends Specification { config.httpClientErrorStatuses == (111..111).toSet() config.httpClientSplitByDomain == true config.partialFlushMinSpans == 25 + config.reportHostName == true config.runtimeContextFieldInjection == false config.propagationStylesToExtract.toList() == [Config.PropagationStyle.DATADOG, Config.PropagationStyle.B3] config.propagationStylesToInject.toList() == [Config.PropagationStyle.B3, Config.PropagationStyle.DATADOG] @@ -228,6 +235,7 @@ class ConfigTest extends Specification { environmentVariables.set(DD_PROPAGATION_STYLE_EXTRACT, "B3 Datadog") environmentVariables.set(DD_PROPAGATION_STYLE_INJECT, "Datadog B3") environmentVariables.set(DD_JMXFETCH_METRICS_CONFIGS_ENV, "some/file") + environmentVariables.set(DD_TRACE_REPORT_HOSTNAME, "true") when: def config = new Config() @@ -239,6 +247,7 @@ class ConfigTest extends Specification { config.propagationStylesToExtract.toList() == [Config.PropagationStyle.B3, Config.PropagationStyle.DATADOG] config.propagationStylesToInject.toList() == [Config.PropagationStyle.DATADOG, Config.PropagationStyle.B3] config.jmxFetchMetricsConfigs == ["some/file"] + config.reportHostName == true } def "sys props override env vars"() { @@ -649,4 +658,27 @@ class ConfigTest extends Specification { listString | list "" | [] } + + def "verify hostname not added to root span tags by default"() { + setup: + Properties properties = new Properties() + + when: + def config = Config.get(properties) + + then: + !config.localRootSpanTags.containsKey('_dd.hostname') + } + + def "verify configuration to add hostname to root span tags"() { + setup: + Properties properties = new Properties() + properties.setProperty(TRACE_REPORT_HOSTNAME, 'true') + + when: + def config = Config.get(properties) + + then: + config.localRootSpanTags.get('_dd.hostname') == InetAddress.localHost.hostName + } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java index d95a159e8b..c7602b2955 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java @@ -110,8 +110,16 @@ public class DDSpan implements Span, MutableSpan { finishAndAddToTrace(TimeUnit.MICROSECONDS.toNanos(stoptimeMicros - startTimeMicro)); } + @Override + public DDSpan setError(final boolean error) { + context.setErrorFlag(true); + return this; + } + /** - * Check if the span is the root parent. It means that the traceId is the same as the spanId + * Check if the span is the root parent. It means that the traceId is the same as the spanId. In + * the context of distributed tracing this will return true if an only if this is the application + * initializing the trace. * * @return true if root, false otherwise */ @@ -121,13 +129,15 @@ public class DDSpan implements Span, MutableSpan { } @Override - public DDSpan setError(final boolean error) { - context.setErrorFlag(true); - return this; + @Deprecated + @JsonIgnore + public MutableSpan getRootSpan() { + return getLocalRootSpan(); } @Override - public MutableSpan getRootSpan() { + @JsonIgnore + public MutableSpan getLocalRootSpan() { return context().getTrace().getRootSpan(); } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index 571a55c4e8..d09e0d1ca6 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -56,8 +56,8 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace /** Scope manager is in charge of managing the scopes from which spans are created */ final ContextualScopeManager scopeManager = new ContextualScopeManager(); - /** Tags required to link apm traces to runtime metrics */ - final Map runtimeTags; + /** A set of tags that are added only to the application's root span */ + private final Map localRootSpanTags; /** A set of tags that are added to every span */ private final Map defaultSpanTags; /** A configured mapping of service names to update with new values */ @@ -107,7 +107,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace // This constructor is already used in the wild, so we have to keep it inside this API for now. public DDTracer(final String serviceName, final Writer writer, final Sampler sampler) { - this(serviceName, writer, sampler, Config.get().getRuntimeTags()); + this(serviceName, writer, sampler, Config.get().getLocalRootSpanTags()); } private DDTracer(final String serviceName, final Config config) { @@ -115,7 +115,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace serviceName, Writer.Builder.forConfig(config), Sampler.Builder.forConfig(config), - config.getRuntimeTags(), + config.getLocalRootSpanTags(), config.getMergedSpanTags(), config.getServiceMapping(), config.getHeaderTags(), @@ -149,7 +149,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace config.getServiceName(), writer, Sampler.Builder.forConfig(config), - config.getRuntimeTags(), + config.getLocalRootSpanTags(), config.getMergedSpanTags(), config.getServiceMapping(), config.getHeaderTags(), @@ -165,6 +165,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace final Writer writer, final Sampler sampler, final String runtimeId, + final Map localRootSpanTags, final Map defaultSpanTags, final Map serviceNameMappings, final Map taggedHeaders) { @@ -172,7 +173,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace serviceName, writer, sampler, - customRuntimeTags(runtimeId), + customRuntimeTags(runtimeId, localRootSpanTags), defaultSpanTags, serviceNameMappings, taggedHeaders, @@ -187,7 +188,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace final String serviceName, final Writer writer, final Sampler sampler, - final Map runtimeTags, + final Map localRootSpanTags, final Map defaultSpanTags, final Map serviceNameMappings, final Map taggedHeaders) { @@ -195,7 +196,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace serviceName, writer, sampler, - runtimeTags, + localRootSpanTags, defaultSpanTags, serviceNameMappings, taggedHeaders, @@ -206,12 +207,12 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace final String serviceName, final Writer writer, final Sampler sampler, - final Map runtimeTags, + final Map localRootSpanTags, final Map defaultSpanTags, final Map serviceNameMappings, final Map taggedHeaders, final int partialFlushMinSpans) { - assert runtimeTags != null; + assert localRootSpanTags != null; assert defaultSpanTags != null; assert serviceNameMappings != null; assert taggedHeaders != null; @@ -220,8 +221,8 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace this.writer = writer; this.writer.start(); this.sampler = sampler; + this.localRootSpanTags = localRootSpanTags; this.defaultSpanTags = defaultSpanTags; - this.runtimeTags = runtimeTags; this.serviceNameMappings = serviceNameMappings; this.partialFlushMinSpans = partialFlushMinSpans; @@ -439,9 +440,9 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace } @Deprecated - private static Map customRuntimeTags(final String runtimeId) { - final Map runtimeTags = new HashMap<>(); - runtimeTags.putAll(Config.get().getRuntimeTags()); + private static Map customRuntimeTags( + final String runtimeId, Map applicationRootSpanTags) { + final Map runtimeTags = new HashMap<>(applicationRootSpanTags); runtimeTags.put(Config.RUNTIME_ID_TAG, runtimeId); return Collections.unmodifiableMap(runtimeTags); } @@ -623,7 +624,9 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace } } - // Propagate internal trace + // Propagate internal trace. + // Note: if we are not in the context of distributed tracing and we are starting the first + // root span, parentContext will be null at this point. if (parentContext instanceof DDSpanContext) { final DDSpanContext ddsc = (DDSpanContext) parentContext; traceId = ddsc.getTraceId(); @@ -660,10 +663,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace origin = null; } - // add runtime tags to the root span - for (final Map.Entry runtimeTag : runtimeTags.entrySet()) { - tags.put(runtimeTag.getKey(), runtimeTag.getValue()); - } + tags.putAll(localRootSpanTags); parentTrace = new PendingTrace(DDTracer.this, traceId, serviceNameMappings); } diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanTest.groovy index d287f0eb09..288245247d 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanTest.groovy @@ -5,6 +5,7 @@ import datadog.opentracing.propagation.TagContext import datadog.trace.api.sampling.PrioritySampling import datadog.trace.common.sampling.RateByServiceSampler import datadog.trace.common.writer.ListWriter +import io.opentracing.SpanContext import spock.lang.Shared import spock.lang.Specification @@ -207,18 +208,45 @@ class DDSpanTest extends Specification { new ExtractedContext("1", "2", 0, "some-origin", [:], [:]) | _ } - def "getRootSpan returns the root span"() { + def "isRootSpan() in and not in the context of distributed tracing"() { setup: - def root = tracer.buildSpan("root").start() + def root = tracer.buildSpan("root").asChildOf((SpanContext)extractedContext).start() def child = tracer.buildSpan("child").asChildOf(root).start() expect: - root.getRootSpan() == root - child.getRootSpan() == root + root.isRootSpan() == isTraceRootSpan + !child.isRootSpan() cleanup: child.finish() root.finish() + + where: + extractedContext | isTraceRootSpan + null | true + new ExtractedContext("123", "456", 1, "789", [:], [:]) | false + } + + def "getApplicationRootSpan() in and not in the context of distributed tracing"() { + setup: + def root = tracer.buildSpan("root").asChildOf((SpanContext)extractedContext).start() + def child = tracer.buildSpan("child").asChildOf(root).start() + + expect: + root.localRootSpan == root + child.localRootSpan == root + // Checking for backward compatibility method names + root.rootSpan == root + child.rootSpan == root + + cleanup: + child.finish() + root.finish() + + where: + extractedContext | isTraceRootSpan + null | true + new ExtractedContext("123", "456", 1, "789", [:], [:]) | false } def "setting forced tracing via tag"() { diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy index fcd40ea063..53ce76f23a 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy @@ -49,6 +49,7 @@ class SpanDecoratorTest extends Specification { new AllSampler(), "some-runtime-id", emptyMap(), + emptyMap(), mapping, emptyMap() ) @@ -80,6 +81,7 @@ class SpanDecoratorTest extends Specification { new AllSampler(), "some-runtime-id", emptyMap(), + emptyMap(), mapping, emptyMap() ) @@ -126,6 +128,7 @@ class SpanDecoratorTest extends Specification { new AllSampler(), "some-runtime-id", emptyMap(), + emptyMap(), mapping, emptyMap() ) diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/DDTracerTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/DDTracerTest.groovy index 43bcf3aab9..aabe1bb6db 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/DDTracerTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/DDTracerTest.groovy @@ -31,7 +31,7 @@ class DDTracerTest extends Specification { // assert that a trace agent isn't running locally as that messes up the test. try { (new Socket("localhost", 8126)).close() - throw new IllegalStateException("Trace Agent unexpectedly running locally.") + throw new IllegalStateException("An agent is already running locally on port 8126. Please stop it if you want to run tests locally.") } catch (final ConnectException ioe) { // trace agent is not running locally. } @@ -126,8 +126,8 @@ class DDTracerTest extends Specification { tracer.serviceName == DEFAULT_SERVICE_NAME tracer.sampler == sampler tracer.writer == writer - tracer.runtimeTags[Config.RUNTIME_ID_TAG].size() > 0 // not null or empty - tracer.runtimeTags[Config.LANGUAGE_TAG_KEY] == Config.LANGUAGE_TAG_VALUE + tracer.localRootSpanTags[Config.RUNTIME_ID_TAG].size() > 0 // not null or empty + tracer.localRootSpanTags[Config.LANGUAGE_TAG_KEY] == Config.LANGUAGE_TAG_VALUE } def "Shares TraceCount with DDApi with #key = #value"() { @@ -144,4 +144,19 @@ class DDTracerTest extends Specification { PRIORITY_SAMPLING | "true" PRIORITY_SAMPLING | "false" } + + def "root tags are applied only to root spans"() { + setup: + def tracer = new DDTracer('my_service', new ListWriter(), new AllSampler(), '', ['only_root': 'value'], [:], [:], [:]) + def root = tracer.buildSpan('my_root').start() + def child = tracer.buildSpan('my_child').asChildOf(root).start() + + expect: + root.context().tags.containsKey('only_root') + !child.context().tags.containsKey('only_root') + + cleanup: + child.finish() + root.finish() + } }