Fix issues from code reviews.
MODIFY
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java
@@ -83,10 +83,6 @@ public abstract class HttpClientDecorator<REQUEST, RESPONSE> extends ClientDecor
final Integer status = status(response);
if (status != null) {
span.setTag(Tags.HTTP_STATUS, status);
-
- if (Config.get().getHttpClientErrorStatuses().contains(status)) {
- span.setError(true);
- }
}
}
return span;

MODIFY
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java
@@ -108,10 +108,6 @@ public abstract class HttpServerDecorator<REQUEST, CONNECTION, RESPONSE> extends
final Integer status = status(response);
if (status != null) {
span.setTag(Tags.HTTP_STATUS, status);
-
- if (Config.get().getHttpServerErrorStatuses().contains(status)) {
- span.setError(true);
- }
}
}
return span;

MODIFY
dd-trace-api/src/main/java/datadog/trace/api/Config.java
dd-trace-api/src/main/java/datadog/trace/api/Config.java
@@ -830,7 +830,7 @@ public class Config {
return jmxFetchIntegrationEnabled(integrationNames, defaultEnabled);
}
- public boolean isDecoratorEnabled(final String name) {
+ public boolean isRuleEnabled(final String name) {
return getBooleanSettingFromEnvironment("trace." + name + ".enabled", true)
&& getBooleanSettingFromEnvironment("trace." + name.toLowerCase() + ".enabled", true);
}

MODIFY
dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java
dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java
@@ -23,7 +23,7 @@ public class DDDecoratorsFactory {
new ServiceNameDecorator("service", false),
new ServletContextDecorator())) {
- if (Config.get().isDecoratorEnabled(decorator.getClass().getSimpleName())) {
+ if (Config.get().isRuleEnabled(decorator.getClass().getSimpleName())) {
decorators.add(decorator);
} else {
log.debug("{} disabled", decorator.getClass().getSimpleName());

MODIFY
dd-trace-ot/src/main/java/datadog/trace/common/processor/TraceProcessor.java
dd-trace-ot/src/main/java/datadog/trace/common/processor/TraceProcessor.java
@@ -5,10 +5,10 @@ import datadog.trace.api.Config;
import datadog.trace.common.processor.rule.AnalyticsSampleRateRule;
import datadog.trace.common.processor.rule.DBStatementRule;
import datadog.trace.common.processor.rule.ErrorRule;
+import datadog.trace.common.processor.rule.HttpStatusErrorRule;
import datadog.trace.common.processor.rule.ResourceNameRule;
import datadog.trace.common.processor.rule.SpanTypeRule;
import datadog.trace.common.processor.rule.Status404Rule;
-import datadog.trace.common.processor.rule.Status5XXRule;
import datadog.trace.common.processor.rule.URLAsResourceNameRule;
import java.util.ArrayList;
import java.util.Collection;
@@ -24,7 +24,7 @@ public class TraceProcessor {
new DBStatementRule(),
new ResourceNameRule(),
new SpanTypeRule(),
- new Status5XXRule(),
+ new HttpStatusErrorRule(),
new ErrorRule(),
new URLAsResourceNameRule(),
new Status404Rule(),
@@ -44,9 +44,9 @@ public class TraceProcessor {
}
private static boolean isEnabled(final Rule rule) {
- boolean enabled = Config.get().isDecoratorEnabled(rule.getClass().getSimpleName());
+ boolean enabled = Config.get().isRuleEnabled(rule.getClass().getSimpleName());
for (final String alias : rule.aliases()) {
- enabled &= Config.get().isDecoratorEnabled(alias);
+ enabled &= Config.get().isRuleEnabled(alias);
}
if (!enabled) {
log.debug("{} disabled", rule.getClass().getSimpleName());

MODIFY
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/AnalyticsSampleRateRule.java
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/AnalyticsSampleRateRule.java
@@ -16,18 +16,19 @@ public class AnalyticsSampleRateRule implements TraceProcessor.Rule {
@Override
public void processSpan(
final DDSpan span, final Map<String, Object> tags, final Collection<DDSpan> trace) {
- if (tags.containsKey(DDTags.ANALYTICS_SAMPLE_RATE)) {
- final Object value = tags.get(DDTags.ANALYTICS_SAMPLE_RATE);
- if (value instanceof Number) {
- span.context().setMetric(DDTags.ANALYTICS_SAMPLE_RATE, (Number) value);
- } else if (value instanceof String) {
+ final Object sampleRateValue = tags.get(DDTags.ANALYTICS_SAMPLE_RATE);
+ if (sampleRateValue instanceof Number) {
+ span.context().setMetric(DDTags.ANALYTICS_SAMPLE_RATE, (Number) sampleRateValue);
+ } else if (sampleRateValue instanceof String) {
try {
span.context()
- .setMetric(DDTags.ANALYTICS_SAMPLE_RATE, Double.parseDouble((String) value));
+ .setMetric(DDTags.ANALYTICS_SAMPLE_RATE, Double.parseDouble((String) sampleRateValue));
} catch (final NumberFormatException ex) {
// ignore
}
}
+
+ if (tags.containsKey(DDTags.ANALYTICS_SAMPLE_RATE)) {
span.setTag(DDTags.ANALYTICS_SAMPLE_RATE, (String) null); // Remove the tag
}
}

MODIFY
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/DBStatementRule.java
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/DBStatementRule.java
@@ -19,18 +19,22 @@ public class DBStatementRule implements TraceProcessor.Rule {
@Override
public void processSpan(
final DDSpan span, final Map<String, Object> tags, final Collection<DDSpan> trace) {
- if (tags.containsKey(Tags.DB_STATEMENT.getKey())) {
+ final Object dbStatementValue = tags.get(Tags.DB_STATEMENT.getKey());
+
+ if (dbStatementValue instanceof String) {
// Special case: Mongo
// Skip the decorators
if (tags.containsKey(Tags.COMPONENT.getKey())
&& "java-mongo".equals(tags.get(Tags.COMPONENT.getKey()))) {
return;
}
-
- final String statement = tags.get(Tags.DB_STATEMENT.getKey()).toString();
+ final String statement = (String) dbStatementValue;
if (!statement.isEmpty()) {
span.setResourceName(statement);
}
+ }
+
+ if (tags.containsKey(Tags.DB_STATEMENT.getKey())) {
span.setTag(Tags.DB_STATEMENT.getKey(), (String) null); // Remove the tag
}
}

MODIFY
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/ErrorRule.java
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/ErrorRule.java
@@ -16,13 +16,14 @@ public class ErrorRule implements TraceProcessor.Rule {
@Override
public void processSpan(
final DDSpan span, final Map<String, Object> tags, final Collection<DDSpan> trace) {
- if (tags.containsKey(Tags.ERROR.getKey())) {
final Object value = tags.get(Tags.ERROR.getKey());
if (value instanceof Boolean) {
span.setError((Boolean) value);
- } else {
+ } else if (value != null) {
span.setError(Boolean.parseBoolean(value.toString()));
}
+
+ if (tags.containsKey(Tags.ERROR.getKey())) {
span.setTag(Tags.ERROR, null); // Remove the tag
}
}

RENAME
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/Status5XXRule.java
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/HttpStatusErrorRule.java
package datadog.trace.common.processor.rule;
import datadog.opentracing.DDSpan;
+import datadog.trace.api.Config;
+import datadog.trace.api.DDSpanTypes;
import datadog.trace.common.processor.TraceProcessor;
import io.opentracing.tag.Tags;
import java.util.Collection;
import java.util.Map;
-/** Mark all 5xx status codes as an error */
-public class Status5XXRule implements TraceProcessor.Rule {
+public class HttpStatusErrorRule implements TraceProcessor.Rule {
@Override
public String[] aliases() {
- return new String[] {"Status5XXDecorator"};
+ return new String[] {};
}
@Override
@@ -19,9 +20,17 @@ public class Status5XXRule implements TraceProcessor.Rule {
if (!span.context().getErrorFlag() && tags.containsKey(Tags.HTTP_STATUS.getKey())) {
final Object value = tags.get(Tags.HTTP_STATUS.getKey());
try {
- final int responseCode =
+ final int status =
value instanceof Integer ? (int) value : Integer.parseInt(value.toString());
- span.setError(500 <= responseCode && responseCode < 600);
+ if (span.getType().equals(DDSpanTypes.HTTP_SERVER)) {
+ if (Config.get().getHttpServerErrorStatuses().contains(status)) {
+ span.setError(true);
+ }
+ } else if (span.getType().equals(DDSpanTypes.HTTP_CLIENT)) {
+ if (Config.get().getHttpClientErrorStatuses().contains(status)) {
+ span.setError(true);
+ }
+ }
} catch (final NumberFormatException ex) {
// If using Tags.HTTP_STATUS, value should always be an Integer,
// but lets catch NumberFormatException just to be safe.

MODIFY
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/ResourceNameRule.java
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/ResourceNameRule.java
@@ -16,8 +16,12 @@ public class ResourceNameRule implements TraceProcessor.Rule {
@Override
public void processSpan(
final DDSpan span, final Map<String, Object> tags, final Collection<DDSpan> trace) {
+ final Object name = tags.get(DDTags.RESOURCE_NAME);
+ if (name != null) {
+ span.setResourceName(name.toString());
+ }
+
if (tags.containsKey(DDTags.RESOURCE_NAME)) {
- span.setResourceName(tags.get(DDTags.RESOURCE_NAME).toString());
span.setTag(DDTags.RESOURCE_NAME, (String) null); // Remove the tag
}
}

MODIFY
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/SpanTypeRule.java
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/SpanTypeRule.java
@@ -16,8 +16,12 @@ public class SpanTypeRule implements TraceProcessor.Rule {
@Override
public void processSpan(
final DDSpan span, final Map<String, Object> tags, final Collection<DDSpan> trace) {
+ final Object type = tags.get(DDTags.SPAN_TYPE);
+ if (type != null) {
+ span.setSpanType(type.toString());
+ }
+
if (tags.containsKey(DDTags.SPAN_TYPE)) {
- span.setSpanType(tags.get(DDTags.SPAN_TYPE).toString());
span.setTag(DDTags.SPAN_TYPE, (String) null); // Remove the tag
}
}

MODIFY
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/URLAsResourceNameRule.java
dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/URLAsResourceNameRule.java
@@ -89,10 +89,13 @@ public class URLAsResourceNameRule implements TraceProcessor.Rule {
}
private String addMethodIfAvailable(final Map<String, Object> meta, String path) {
- // if the verb (GET, POST ...) is present, add it
- final Object verb = meta.get(Tags.HTTP_METHOD.getKey());
- if (verb != null && !verb.toString().isEmpty()) {
- path = verb.toString().toUpperCase() + " " + path;
+ // if the method (GET, POST ...) is present, add it
+ final Object method = meta.get(Tags.HTTP_METHOD.getKey());
+ if (method != null) {
+ final String verb = method.toString().toUpperCase().trim();
+ if (!verb.isEmpty()) {
+ path = verb + " " + path;
+ }
}
return path;
}

Fix issues from code reviews.