diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/JDBCMaps.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/JDBCMaps.java index 6752ac019e2..062597276dd 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/JDBCMaps.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/JDBCMaps.java @@ -17,13 +17,26 @@ public class JDBCMaps { Collections.synchronizedMap(new WeakHashMap()); public static final Map preparedStatements = Collections.synchronizedMap(new WeakHashMap()); - public static final String UNKNOWN_QUERY = "Unknown Query"; + + private static final boolean RENAME_UNKNOWN = + Boolean.valueOf(getPropOrEnv("dd.trace.rename.unknown")); + + public static final String DB_QUERY = RENAME_UNKNOWN ? "DB Query" : "Unknown Query"; @Data public static class DBInfo { - public static DBInfo UNKNOWN = new DBInfo("null", "unknown", null); + public static DBInfo DEFAULT = + new DBInfo("null", RENAME_UNKNOWN ? "database" : "unknown", null); private final String url; private final String type; private final String user; } + + private static String getPropOrEnv(final String name) { + return System.getProperty(name, System.getenv(propToEnvName(name))); + } + + private static String propToEnvName(final String name) { + return name.toUpperCase().replace(".", "_"); + } } diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java index da1fbde9846..e4487e5ae79 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -66,7 +66,7 @@ public static Scope startSpan(@Advice.This final PreparedStatement statement) { JDBCMaps.DBInfo dbInfo = JDBCMaps.connectionInfo.get(connection); if (dbInfo == null) { - dbInfo = JDBCMaps.DBInfo.UNKNOWN; + dbInfo = JDBCMaps.DBInfo.DEFAULT; } final Scope scope = GlobalTracer.get().buildSpan(dbInfo.getType() + ".query").startActive(true); @@ -77,7 +77,7 @@ public static Scope startSpan(@Advice.This final PreparedStatement statement) { Tags.COMPONENT.set(span, "java-jdbc-prepared_statement"); span.setTag(DDTags.SERVICE_NAME, dbInfo.getType()); - span.setTag(DDTags.RESOURCE_NAME, sql == null ? JDBCMaps.UNKNOWN_QUERY : sql); + span.setTag(DDTags.RESOURCE_NAME, sql == null ? JDBCMaps.DB_QUERY : sql); span.setTag(DDTags.SPAN_TYPE, "sql"); span.setTag("span.origin.type", statement.getClass().getName()); span.setTag("db.jdbc.url", dbInfo.getUrl()); diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java index 4457c76f764..9a20e27b8dc 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java @@ -66,7 +66,7 @@ public static Scope startSpan( JDBCMaps.DBInfo dbInfo = JDBCMaps.connectionInfo.get(connection); if (dbInfo == null) { - dbInfo = JDBCMaps.DBInfo.UNKNOWN; + dbInfo = JDBCMaps.DBInfo.DEFAULT; } final Scope scope = diff --git a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaConsumerInstrumentation.java b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaConsumerInstrumentation.java index a6b28aabe39..d1967c8ba95 100644 --- a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaConsumerInstrumentation.java +++ b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaConsumerInstrumentation.java @@ -95,7 +95,7 @@ public static class ConsumeScopeAction @Override public void decorate(final Tracer.SpanBuilder spanBuilder, final ConsumerRecord record) { - final String topic = record.topic() == null ? "unknown" : record.topic(); + final String topic = record.topic() == null ? "kafka" : record.topic(); final SpanContext spanContext = GlobalTracer.get() .extract(Format.Builtin.TEXT_MAP, new TextMapExtractAdapter(record.headers())); diff --git a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaProducerInstrumentation.java b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaProducerInstrumentation.java index 0315afcb008..3d7bc68c1e9 100644 --- a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaProducerInstrumentation.java +++ b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaProducerInstrumentation.java @@ -73,7 +73,7 @@ public static Scope startSpan( callback = new ProducerCallback(callback, scope); final Span span = scope.span(); - final String topic = record.topic() == null ? "unknown" : record.topic(); + final String topic = record.topic() == null ? "kafka" : record.topic(); if (record.partition() != null) { span.setTag("kafka.partition", record.partition()); } diff --git a/dd-trace-ot/dd-trace-ot.gradle b/dd-trace-ot/dd-trace-ot.gradle index 1eae3b359d4..e418a109916 100644 --- a/dd-trace-ot/dd-trace-ot.gradle +++ b/dd-trace-ot/dd-trace-ot.gradle @@ -10,7 +10,6 @@ apply from: "${rootDir}/gradle/publish.gradle" minimumBranchCoverage = 0.5 minimumInstructionCoverage = 0.6 whitelistedInstructionClasses += whitelistedBranchClasses += [ - 'datadog.opentracing.decorators.*', 'datadog.trace.common.writer.ListWriter', 'datadog.trace.common.sampling.PrioritySampling' ] diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java index 0fce5bd0579..85c745d3ff8 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java @@ -4,7 +4,6 @@ import datadog.opentracing.decorators.AbstractDecorator; import datadog.trace.api.DDTags; import datadog.trace.api.sampling.PrioritySampling; -import io.opentracing.tag.Tags; import java.util.Collections; import java.util.List; import java.util.Map; @@ -238,25 +237,14 @@ public synchronized void setTag(final String tag, final Object value) { return; } - if (tag.equals(DDTags.SERVICE_NAME)) { - setServiceName(value.toString()); - return; - } else if (tag.equals(DDTags.RESOURCE_NAME)) { - setResourceName(value.toString()); - return; - } else if (tag.equals(DDTags.SPAN_TYPE)) { - setSpanType(value.toString()); - return; - } - - this.tags.put(tag, value); + boolean addTag = true; // Call decorators final List decorators = tracer.getSpanContextDecorators(tag); if (decorators != null && value != null) { for (final AbstractDecorator decorator : decorators) { try { - decorator.afterSetTag(this, tag, value); + addTag &= decorator.shouldSetTag(this, tag, value); } catch (final Throwable ex) { log.debug( "Could not decorate the span decorator={}: {}", @@ -265,10 +253,9 @@ public synchronized void setTag(final String tag, final Object value) { } } } - // Error management - if (Tags.ERROR.getKey().equals(tag) - && Boolean.TRUE.equals(value instanceof String ? Boolean.valueOf((String) value) : value)) { - this.errorFlag = true; + + if (addTag) { + this.tags.put(tag, value); } } 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 4eccf4b94dd..8883d2a91c1 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -56,7 +56,7 @@ public class DDTracer implements io.opentracing.Tracer { final ContextualScopeManager scopeManager = new ContextualScopeManager(); /** A set of tags that are added to every span */ - private final Map spanTags; + private final Map spanTags; /** Span context decorators */ private final Map> spanContextDecorators = @@ -88,24 +88,31 @@ public DDTracer(final Properties config) { config.getProperty(DDTraceConfig.SERVICE_NAME), Writer.Builder.forConfig(config), Sampler.Builder.forConfig(config), - DDTraceConfig.parseMap(config.getProperty(DDTraceConfig.SPAN_TAGS))); + DDTraceConfig.parseMap(config.getProperty(DDTraceConfig.SPAN_TAGS)), + DDTraceConfig.parseMap(config.getProperty(DDTraceConfig.SERVICE_MAPPING))); log.debug("Using config: {}", config); } public DDTracer(final String serviceName, final Writer writer, final Sampler sampler) { - this(serviceName, writer, sampler, Collections.emptyMap()); + this( + serviceName, + writer, + sampler, + Collections.emptyMap(), + Collections.emptyMap()); } public DDTracer( final String serviceName, final Writer writer, final Sampler sampler, - final Map spanTags) { + final Map defaultSpanTags, + final Map serviceNameMappings) { this.serviceName = serviceName; this.writer = writer; this.writer.start(); this.sampler = sampler; - this.spanTags = spanTags; + this.spanTags = defaultSpanTags; registry = new CodecRegistry(); registry.register(Format.Builtin.HTTP_HEADERS, new HTTPCodec()); @@ -118,7 +125,8 @@ public DDTracer( registerClassLoader(ClassLoader.getSystemClassLoader()); - final List decorators = DDDecoratorsFactory.createBuiltinDecorators(); + final List decorators = + DDDecoratorsFactory.createBuiltinDecorators(serviceNameMappings); for (final AbstractDecorator decorator : decorators) { log.debug("Loading decorator: {}", decorator.getClass().getSimpleName()); addDecorator(decorator); @@ -132,7 +140,8 @@ public DDTracer(final Writer writer) { UNASSIGNED_DEFAULT_SERVICE_NAME, writer, new AllSampler(), - DDTraceConfig.parseMap(new DDTraceConfig().getProperty(DDTraceConfig.SPAN_TAGS))); + DDTraceConfig.parseMap(new DDTraceConfig().getProperty(DDTraceConfig.SPAN_TAGS)), + DDTraceConfig.parseMap(new DDTraceConfig().getProperty(DDTraceConfig.SERVICE_MAPPING))); } /** @@ -302,7 +311,9 @@ public class DDSpanBuilder implements SpanBuilder { // Builder attributes private Map tags = - spanTags.isEmpty() ? Collections.emptyMap() : new HashMap<>(spanTags); + spanTags.isEmpty() + ? Collections.emptyMap() + : new HashMap(spanTags); private long timestampMicro; private SpanContext parent; private String serviceName; diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/AbstractDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/AbstractDecorator.java index 08804d204b5..51b6ec7edfe 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/AbstractDecorator.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/AbstractDecorator.java @@ -1,7 +1,6 @@ package datadog.opentracing.decorators; import datadog.opentracing.DDSpanContext; -import datadog.trace.api.DDTags; /** * Span decorators are called when new tags are written and proceed to various remappings and @@ -13,27 +12,20 @@ public abstract class AbstractDecorator { private Object matchingValue; - private String setTag; + private String replacementTag; - private String setValue; + private String replacementValue; - public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) { + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { if (this.getMatchingValue() == null || this.getMatchingValue().equals(value)) { - final String targetTag = getSetTag() == null ? tag : getSetTag(); - final String targetValue = getSetValue() == null ? String.valueOf(value) : getSetValue(); - - if (targetTag.equals(DDTags.SERVICE_NAME)) { - context.setServiceName(targetValue); - } else if (targetTag.equals(DDTags.RESOURCE_NAME)) { - context.setResourceName(targetValue); - } else if (targetTag.equals(DDTags.SPAN_TYPE)) { - context.setSpanType(targetValue); - } else { - context.setTag(targetTag, targetValue); - } - return true; - } else { + final String targetTag = getReplacementTag() == null ? tag : getReplacementTag(); + final String targetValue = + getReplacementValue() == null ? String.valueOf(value) : getReplacementValue(); + + context.setTag(targetTag, targetValue); return false; + } else { + return true; } } @@ -53,19 +45,19 @@ public void setMatchingValue(final Object value) { this.matchingValue = value; } - public String getSetTag() { - return setTag; + public String getReplacementTag() { + return replacementTag; } - public void setSetTag(final String targetTag) { - this.setTag = targetTag; + public void setReplacementTag(final String targetTag) { + this.replacementTag = targetTag; } - public String getSetValue() { - return setValue; + public String getReplacementValue() { + return replacementValue; } - public void setSetValue(final String targetValue) { - this.setValue = targetValue; + public void setReplacementValue(final String targetValue) { + this.replacementValue = targetValue; } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DBStatementAsResourceName.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DBStatementAsResourceName.java index e90d419f21f..d3b23bd5485 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DBStatementAsResourceName.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DBStatementAsResourceName.java @@ -9,11 +9,11 @@ public class DBStatementAsResourceName extends AbstractDecorator { public DBStatementAsResourceName() { super(); this.setMatchingTag(Tags.DB_STATEMENT.getKey()); - this.setSetTag(DDTags.RESOURCE_NAME); + this.setReplacementTag(DDTags.RESOURCE_NAME); } @Override - public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) { + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { // Special case: Mongo // Skip the decorators @@ -23,15 +23,15 @@ public boolean afterSetTag(final DDSpanContext context, final String tag, final } // Assign service name - if (super.afterSetTag(context, tag, value)) { + if (!super.shouldSetTag(context, tag, value)) { // TODO: remove properly the tag (immutable at this time) // the `db.statement` tag must be removed because it will be set // by the Datadog Trace Agent as `sql.query`; here we're removing // a duplicate that will not be obfuscated with the current Datadog // Trace Agent version. context.setTag(Tags.DB_STATEMENT.getKey(), null); - return true; + return false; } - return false; + return true; } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DBTypeDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DBTypeDecorator.java index d64a961a8f9..637a58091ab 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DBTypeDecorator.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DBTypeDecorator.java @@ -13,14 +13,14 @@ public class DBTypeDecorator extends AbstractDecorator { public DBTypeDecorator() { super(); this.setMatchingTag(Tags.DB_TYPE.getKey()); - this.setSetTag(DDTags.SERVICE_NAME); + this.setReplacementTag(DDTags.SERVICE_NAME); } @Override - public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) { + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { // Assign service name - if (super.afterSetTag(context, tag, value)) { + if (!super.shouldSetTag(context, tag, value)) { // Assign span type to DB // Special case: Mongo, set to mongodb if ("mongo".equals(value)) { @@ -33,8 +33,7 @@ public boolean afterSetTag(final DDSpanContext context, final String tag, final } // Works for: mongo, cassandra, jdbc context.setOperationName(String.valueOf(value) + ".query"); - return true; } - return false; + return true; } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java index 0786cd8c481..24216a63c95 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java @@ -1,32 +1,33 @@ package datadog.opentracing.decorators; -import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.Map; /** Create DDSpanDecorators */ public class DDDecoratorsFactory { - public static List createBuiltinDecorators() { - List builtin = new ArrayList(8); - { - final HTTPComponent httpDecorator1 = new HTTPComponent(); - httpDecorator1.setMatchingTag("component"); - httpDecorator1.setMatchingValue("okhttp"); - builtin.add(httpDecorator1); - } - { - final HTTPComponent httpDecorator2 = new HTTPComponent(); - httpDecorator2.setMatchingTag("component"); - httpDecorator2.setMatchingValue("java-aws-sdk"); - builtin.add(httpDecorator2); - } - builtin.add(new ErrorFlag()); - builtin.add(new DBTypeDecorator()); - builtin.add(new DBStatementAsResourceName()); - builtin.add(new OperationDecorator()); - builtin.add(new Status404Decorator()); - builtin.add(new URLAsResourceName()); - builtin.add(new Status5XXDecorator()); + public static List createBuiltinDecorators( + final Map mappings) { + final HTTPComponent httpDecorator1 = new HTTPComponent(); + httpDecorator1.setMatchingTag("component"); + httpDecorator1.setMatchingValue("okhttp"); - return builtin; + final HTTPComponent httpDecorator2 = new HTTPComponent(); + httpDecorator2.setMatchingTag("component"); + httpDecorator2.setMatchingValue("java-aws-sdk"); + + return Arrays.asList( + new DBStatementAsResourceName(), + new DBTypeDecorator(), + new ErrorFlag(), + httpDecorator1, + httpDecorator2, + new OperationDecorator(), + new ResourceNameDecorator(), + new ServiceNameDecorator(mappings), + new SpanTypeDecorator(), + new Status5XXDecorator(), + new Status404Decorator(), + new URLAsResourceName()); } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ErrorFlag.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ErrorFlag.java index 333cb8c307e..18ea0febbf1 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ErrorFlag.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ErrorFlag.java @@ -10,13 +10,14 @@ public ErrorFlag() { } @Override - public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) { + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { // Assign resource name try { context.setErrorFlag(Boolean.parseBoolean(String.valueOf(value))); } catch (final Throwable t) { // DO NOTHING } + // TODO: Do we really want an error tag if the error flag is already set? return true; } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/HTTPComponent.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/HTTPComponent.java index 378b17d2c29..612e9cb29b0 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/HTTPComponent.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/HTTPComponent.java @@ -13,18 +13,17 @@ public class HTTPComponent extends AbstractDecorator { public HTTPComponent() { super(); this.setMatchingTag(Tags.COMPONENT.getKey()); - this.setSetTag(DDTags.SERVICE_NAME); + this.setReplacementTag(DDTags.SERVICE_NAME); } @Override - public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) { - // Assign service name - if (super.afterSetTag(context, tag, value)) { + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { + if (getMatchingValue().equals(value)) { + // Assign service name + super.shouldSetTag(context, tag, value); // Assign span type to WEB context.setSpanType("web"); - return true; - } else { - return false; } + return true; } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java index 0245fa65056..439103e499d 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java @@ -32,12 +32,10 @@ public OperationDecorator() { } @Override - public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) { - + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { if (MAPPINGS.containsKey(String.valueOf(value))) { context.setOperationName(MAPPINGS.get(String.valueOf(value))); - return true; } - return false; + return true; } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ResourceNameDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ResourceNameDecorator.java new file mode 100644 index 00000000000..e42fd1bf6ba --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ResourceNameDecorator.java @@ -0,0 +1,18 @@ +package datadog.opentracing.decorators; + +import datadog.opentracing.DDSpanContext; +import datadog.trace.api.DDTags; + +public class ResourceNameDecorator extends AbstractDecorator { + + public ResourceNameDecorator() { + super(); + this.setMatchingTag(DDTags.RESOURCE_NAME); + } + + @Override + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { + context.setResourceName(String.valueOf(value)); + return false; + } +} diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ServiceNameDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ServiceNameDecorator.java new file mode 100644 index 00000000000..0e893904083 --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ServiceNameDecorator.java @@ -0,0 +1,26 @@ +package datadog.opentracing.decorators; + +import datadog.opentracing.DDSpanContext; +import datadog.trace.api.DDTags; +import java.util.Map; + +public class ServiceNameDecorator extends AbstractDecorator { + + private final Map mappings; + + public ServiceNameDecorator(final Map mappings) { + super(); + this.setMatchingTag(DDTags.SERVICE_NAME); + this.mappings = mappings; + } + + @Override + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { + if (mappings.containsKey(String.valueOf(value))) { + context.setServiceName(mappings.get(String.valueOf(value))); + } else { + context.setServiceName(String.valueOf(value)); + } + return false; + } +} diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/SpanTypeDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/SpanTypeDecorator.java new file mode 100644 index 00000000000..19129c7b80a --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/SpanTypeDecorator.java @@ -0,0 +1,19 @@ +package datadog.opentracing.decorators; + +import datadog.opentracing.DDSpanContext; +import datadog.trace.api.DDTags; + +public class SpanTypeDecorator extends AbstractDecorator { + + public SpanTypeDecorator() { + super(); + this.setMatchingTag(DDTags.SPAN_TYPE); + } + + @Override + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { + context.setSpanType(String.valueOf(value)); + // TODO: Do we really want a span type tag since it already exists on the span? + return false; + } +} diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status404Decorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status404Decorator.java index 12281ffed35..35a5e54e322 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status404Decorator.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status404Decorator.java @@ -1,5 +1,6 @@ package datadog.opentracing.decorators; +import datadog.opentracing.DDSpanContext; import datadog.trace.api.DDTags; import io.opentracing.tag.Tags; @@ -10,7 +11,13 @@ public Status404Decorator() { super(); this.setMatchingTag(Tags.HTTP_STATUS.getKey()); this.setMatchingValue(404); - this.setSetTag(DDTags.RESOURCE_NAME); - this.setSetValue("404"); + this.setReplacementTag(DDTags.RESOURCE_NAME); + this.setReplacementValue("404"); + } + + @Override + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { + super.shouldSetTag(context, tag, value); + return true; } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status5XXDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status5XXDecorator.java index 6a094e80c46..368d0dbd5ba 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status5XXDecorator.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status5XXDecorator.java @@ -11,14 +11,11 @@ public Status5XXDecorator() { } @Override - public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) { - if (Tags.HTTP_STATUS.getKey().equals(tag)) { - final int responseCode = Integer.parseInt(value.toString()); - if (500 <= responseCode && responseCode < 600) { - context.setTag(Tags.ERROR.getKey(), true); - return true; - } + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { + final int responseCode = Integer.parseInt(value.toString()); + if (500 <= responseCode && responseCode < 600) { + context.setTag(Tags.ERROR.getKey(), true); } - return false; + return true; } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java index 73e02922dd2..06f25a1e96e 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java @@ -18,40 +18,37 @@ public class URLAsResourceName extends AbstractDecorator { public URLAsResourceName() { super(); this.setMatchingTag(Tags.HTTP_URL.getKey()); - this.setSetTag(DDTags.RESOURCE_NAME); + this.setReplacementTag(DDTags.RESOURCE_NAME); } @Override - public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) { - try { - final String statusCode = String.valueOf(context.getTags().get(Tags.HTTP_STATUS.getKey())); - // do nothing if the status code is already set and equals to 404. - // TODO: it assumes that Status404Decorator is active. If it's not, it will lead to unexpected behaviors - if (statusCode != null && statusCode.equals("404")) { - return true; - } - - // Get the path without host:port - String path = String.valueOf(value); + public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { + final String statusCode = String.valueOf(context.getTags().get(Tags.HTTP_STATUS.getKey())); + // do nothing if the status code is already set and equals to 404. + // TODO: it assumes that Status404Decorator is active. If it's not, it will lead to unexpected + // behaviors + if (statusCode != null && statusCode.equals("404")) { + return true; + } - try { - path = new java.net.URL(path).getPath(); - } catch (final MalformedURLException e) { - // do nothing, use the value instead of the path - } - // normalize the path - path = norm(path); + // Get the path without host:port + String path = String.valueOf(value); - // if the verb (GET, POST ...) is present, add it - final String verb = (String) context.getTags().get(Tags.HTTP_METHOD.getKey()); - if (verb != null && !verb.isEmpty()) { - path = verb + " " + path; - } + try { + path = new java.net.URL(path).getPath(); + } catch (final MalformedURLException e) { + // do nothing, use the value instead of the path + } + // normalize the path + path = norm(path); - context.setResourceName(path); - } catch (final Throwable e) { - return false; + // if the verb (GET, POST ...) is present, add it + final String verb = (String) context.getTags().get(Tags.HTTP_METHOD.getKey()); + if (verb != null && !verb.isEmpty()) { + path = verb + " " + path; } + + context.setResourceName(path); return true; } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/DDTraceConfig.java b/dd-trace-ot/src/main/java/datadog/trace/common/DDTraceConfig.java index f23f65d3216..c5a1fbce34a 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/DDTraceConfig.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/DDTraceConfig.java @@ -24,6 +24,7 @@ public class DDTraceConfig extends Properties { private static final String PREFIX = "dd."; public static final String SERVICE_NAME = "service.name"; + public static final String SERVICE_MAPPING = "service.mapping"; public static final String WRITER_TYPE = "writer.type"; public static final String AGENT_HOST = "agent.host"; public static final String AGENT_PORT = "agent.port"; @@ -31,6 +32,7 @@ public class DDTraceConfig extends Properties { public static final String SPAN_TAGS = "trace.span.tags"; private final String serviceName = getPropOrEnv(PREFIX + SERVICE_NAME); + private final String serviceMapping = getPropOrEnv(PREFIX + SERVICE_MAPPING); private final String writerType = getPropOrEnv(PREFIX + WRITER_TYPE); private final String agentHost = getPropOrEnv(PREFIX + AGENT_HOST); private final String agentPort = getPropOrEnv(PREFIX + AGENT_PORT); @@ -48,6 +50,7 @@ public DDTraceConfig() { super.defaults = defaults; setIfNotNull(SERVICE_NAME, serviceName); + setIfNotNull(SERVICE_MAPPING, serviceMapping); setIfNotNull(WRITER_TYPE, writerType); setIfNotNull(AGENT_HOST, agentHost); setIfNotNull(AGENT_PORT, agentPort); @@ -74,7 +77,7 @@ static String propToEnvName(final String name) { return name.toUpperCase().replace(".", "_"); } - public static Map parseMap(final String str) { + public static Map parseMap(final String str) { if (str == null || str.trim().isEmpty()) { return Collections.emptyMap(); } @@ -84,7 +87,7 @@ public static Map parseMap(final String str) { } final String[] tokens = str.split(",", -1); - final Map map = new HashMap<>(tokens.length + 1, 1f); + final Map map = new HashMap<>(tokens.length + 1, 1f); for (final String token : tokens) { final String[] keyValue = token.split(":", -1); 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 036325e1176..2d7e207de5b 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 @@ -3,46 +3,57 @@ package datadog.opentracing.decorators import datadog.opentracing.DDSpanContext import datadog.opentracing.DDTracer import datadog.opentracing.SpanFactory +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags import datadog.trace.common.writer.LoggingWriter import io.opentracing.tag.StringTag import io.opentracing.tag.Tags import spock.lang.Specification class SpanDecoratorTest extends Specification { + def tracer = new DDTracer(new LoggingWriter()) + def span = SpanFactory.newSpanOf(tracer) def "adding span personalisation using Decorators"() { setup: - def tracer = new DDTracer(new LoggingWriter()) def decorator = new AbstractDecorator() { @Override - boolean afterSetTag(DDSpanContext context, String tag, Object value) { - return super.afterSetTag(context, tag, value) + boolean shouldSetTag(DDSpanContext context, String tag, Object value) { + return super.shouldSetTag(context, tag, value) } - } decorator.setMatchingTag("foo") decorator.setMatchingValue("bar") - decorator.setSetTag("newFoo") - decorator.setSetValue("newBar") + decorator.setReplacementTag("newFoo") + decorator.setReplacementValue("newBar") tracer.addDecorator(decorator) - def span = SpanFactory.newSpanOf(tracer) new StringTag("foo").set(span, "bar") expect: span.getTags().containsKey("newFoo") span.getTags().get("newFoo") == "newBar" - - cleanup: - span.finish() } - def "override operation with OperationDecorator"() { + def "set service name"() { + setup: + tracer.addDecorator(new ServiceNameDecorator(mapping)) + + when: + span.setTag(DDTags.SERVICE_NAME, name) + + then: + span.getServiceName() == expected + where: + name | expected | mapping + "some-service" | "new-service" | ["some-service": "new-service"] + "other-service" | "other-service" | ["some-service": "new-service"] + } + + def "set operation name"() { setup: - def tracer = new DDTracer(new LoggingWriter()) - def span = SpanFactory.newSpanOf(tracer) tracer.addDecorator(new OperationDecorator()) when: @@ -51,19 +62,41 @@ class SpanDecoratorTest extends Specification { then: span.getOperationName() == operationName - cleanup: - span.finish() - where: component << OperationDecorator.MAPPINGS.keySet() operationName << OperationDecorator.MAPPINGS.values() } - def "override operation with DBTypeDecorator"() { + def "set resource name"() { + setup: + tracer.addDecorator(new ResourceNameDecorator()) + when: + span.setTag(DDTags.RESOURCE_NAME, name) + + then: + span.getResourceName() == name + + where: + name = "my resource name" + } + + def "set span type"() { + setup: + tracer.addDecorator(new SpanTypeDecorator()) + + when: + span.setTag(DDTags.SPAN_TYPE, type) + + then: + span.getSpanType() == type + + where: + type = DDSpanTypes.HTTP_CLIENT + } + + def "override operation with DBTypeDecorator"() { setup: - def tracer = new DDTracer(new LoggingWriter()) - def span = SpanFactory.newSpanOf(tracer) tracer.addDecorator(new DBTypeDecorator()) when: @@ -81,17 +114,12 @@ class SpanDecoratorTest extends Specification { span.getOperationName() == "mongo.query" span.context().getSpanType() == "mongodb" - cleanup: - span.finish() - where: type = "foo" } def "DBStatementAsResource should not interact on Mongo queries"() { setup: - def tracer = new DDTracer(new LoggingWriter()) - def span = SpanFactory.newSpanOf(tracer) tracer.addDecorator(new DBStatementAsResourceName()) when: @@ -111,17 +139,12 @@ class SpanDecoratorTest extends Specification { then: span.getResourceName() == something - cleanup: - span.finish() - where: something = "fake-query" } def "set 404 as a resource on a 404 issue"() { setup: - def tracer = new DDTracer(new LoggingWriter()) - def span = SpanFactory.newSpanOf(tracer) tracer.addDecorator(new Status404Decorator()) when: @@ -129,8 +152,42 @@ class SpanDecoratorTest extends Specification { then: span.getResourceName() == "404" + } + + def "set 5XX status code as an error"() { + setup: + tracer.addDecorator(new Status5XXDecorator()) + + when: + Tags.HTTP_STATUS.set(span, status) - cleanup: - span.finish() + then: + span.isError() == error + + where: + status | error + 400 | false + 404 | false + 499 | false + 500 | true + 550 | true + 599 | true + 600 | false + } + + def "set error flag when error tag reported"() { + setup: + tracer.addDecorator(new ErrorFlag()) + + when: + Tags.ERROR.set(span, error) + + then: + span.isError() == error + + where: + error | _ + true | _ + false | _ } } diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy index 063cb81a70a..ea76b81659a 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy @@ -102,7 +102,7 @@ class URLAsResourceNameTest extends Specification { tracer) then: - decorator.afterSetTag(context, Tags.HTTP_URL.getKey(), value) + decorator.shouldSetTag(context, Tags.HTTP_URL.getKey(), value) context.resourceName == resourceName where: diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy index 750326527c5..56830d67f19 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy @@ -1,6 +1,7 @@ package datadog.trace import datadog.opentracing.DDTracer +import datadog.opentracing.decorators.ServiceNameDecorator import datadog.trace.common.DDTraceConfig import datadog.trace.common.sampling.AllSampler import datadog.trace.common.writer.DDAgentWriter @@ -12,7 +13,14 @@ import org.junit.contrib.java.lang.system.RestoreSystemProperties import spock.lang.Specification import spock.lang.Unroll -import static datadog.trace.common.DDTraceConfig.* +import static datadog.trace.common.DDTraceConfig.AGENT_HOST +import static datadog.trace.common.DDTraceConfig.AGENT_PORT +import static datadog.trace.common.DDTraceConfig.PREFIX +import static datadog.trace.common.DDTraceConfig.SERVICE_MAPPING +import static datadog.trace.common.DDTraceConfig.SERVICE_NAME +import static datadog.trace.common.DDTraceConfig.SPAN_TAGS +import static datadog.trace.common.DDTraceConfig.WRITER_TYPE +import static datadog.trace.common.DDTraceConfig.propToEnvName class DDTraceConfigTest extends Specification { @Rule @@ -34,18 +42,22 @@ class DDTraceConfigTest extends Specification { then: config.getProperty(SERVICE_NAME) == "unnamed-java-app" + config.getProperty(SERVICE_MAPPING) == null config.getProperty(WRITER_TYPE) == "DDAgentWriter" config.getProperty(AGENT_HOST) == "localhost" config.getProperty(AGENT_PORT) == "8126" + config.getProperty(SPAN_TAGS) == null when: config = new DDTraceConfig("A different service name") then: config.getProperty(SERVICE_NAME) == "A different service name" + config.getProperty(SERVICE_MAPPING) == null config.getProperty(WRITER_TYPE) == "DDAgentWriter" config.getProperty(AGENT_HOST) == "localhost" config.getProperty(AGENT_PORT) == "8126" + config.getProperty(SPAN_TAGS) == null } def "specify overrides via system properties"() { @@ -96,7 +108,28 @@ class DDTraceConfigTest extends Specification { tracer.sampler instanceof AllSampler tracer.writer.toString() == "DDAgentWriter { api=DDApi { tracesEndpoint=http://localhost:8126/v0.3/traces } }" - tracer.spanContextDecorators.size() == 6 + tracer.spanContextDecorators.size() == 9 + } + + def "verify mapping configs on tracer"() { + setup: + System.setProperty(PREFIX + SERVICE_MAPPING, mapString) + System.setProperty(PREFIX + SPAN_TAGS, mapString) + + when: + def tracer = new DDTracer() + ServiceNameDecorator decorator = tracer.spanContextDecorators.values().flatten().find { + it instanceof ServiceNameDecorator + } + + then: + tracer.spanTags == map + decorator.mappings == map + + where: + mapString | map + "a:1, a:2, a:3" | [a: "3"] + "a:b,c:d" | [a: "b", c: "d"] } @Unroll @@ -132,19 +165,19 @@ class DDTraceConfigTest extends Specification { def "parsing an invalid string returns an empty map"() { expect: - DDTraceConfig.parseMap(str) == map + DDTraceConfig.parseMap(str) == [:] where: - str | map - null | [:] - "" | [:] - "1" | [:] - "a" | [:] - "a:" | [:] - "a,1" | [:] - "in:val:id" | [:] - "a:b:c:d" | [:] - "a:b,c,d" | [:] - "!a" | [:] + str | _ + null | _ + "" | _ + "1" | _ + "a" | _ + "a:" | _ + "a,1" | _ + "in:val:id" | _ + "a:b:c:d" | _ + "a:b,c,d" | _ + "!a" | _ } }