From 2028b100098777cdabc883d14e21e05af655c9f8 Mon Sep 17 00:00:00 2001
From: Tyler Benson <tyler.benson@datadoghq.com>
Date: Thu, 19 Mar 2020 17:23:49 -0700
Subject: [PATCH] Move ErrorFlag to TraceProcessor

---
 .../test/groovy/TraceAnnotationsTest.groovy   |  7 ------
 .../decorators/DDDecoratorsFactory.java       |  1 -
 .../opentracing/decorators/ErrorFlag.java     | 22 -----------------
 .../common/processor/TraceProcessor.java      |  3 ++-
 .../common/processor/rule/ErrorRule.java      | 24 +++++++++++++++++++
 5 files changed, 26 insertions(+), 31 deletions(-)
 delete mode 100644 dd-trace-ot/src/main/java/datadog/opentracing/decorators/ErrorFlag.java
 create mode 100644 dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/ErrorRule.java

diff --git a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy
index a2ffb96c05..82f44651f3 100644
--- a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy
+++ b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy
@@ -1,4 +1,3 @@
-import datadog.opentracing.decorators.ErrorFlag
 import datadog.trace.agent.test.AgentTestRunner
 import datadog.trace.agent.test.utils.ConfigUtils
 import datadog.trace.api.Trace
@@ -243,9 +242,6 @@ class TraceAnnotationsTest extends AgentTestRunner {
 
   def "test exception exit"() {
     setup:
-
-    TEST_TRACER.addDecorator(new ErrorFlag())
-
     Throwable error = null
     try {
       SayTracedHello.sayERROR()
@@ -272,9 +268,6 @@ class TraceAnnotationsTest extends AgentTestRunner {
 
   def "test exception exit with resource name"() {
     setup:
-
-    TEST_TRACER.addDecorator(new ErrorFlag())
-
     Throwable error = null
     try {
       SayTracedHello.sayERRORWithResource()
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 4dbc49c41b..909d533de5 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
@@ -18,7 +18,6 @@ public class DDDecoratorsFactory {
             new AnalyticsSampleRateDecorator(),
             new DBStatementAsResourceName(),
             new DBTypeDecorator(),
-            new ErrorFlag(),
             new ForceManualDropDecorator(),
             new ForceManualKeepDecorator(),
             new OperationDecorator(),
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
deleted file mode 100644
index 3b0c8c56be..0000000000
--- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/ErrorFlag.java
+++ /dev/null
@@ -1,22 +0,0 @@
-package datadog.opentracing.decorators;
-
-import datadog.opentracing.DDSpanContext;
-import io.opentracing.tag.Tags;
-
-public class ErrorFlag extends AbstractDecorator {
-  public ErrorFlag() {
-    super();
-    setMatchingTag(Tags.ERROR.getKey());
-  }
-
-  @Override
-  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
-    }
-    return false;
-  }
-}
diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/processor/TraceProcessor.java b/dd-trace-ot/src/main/java/datadog/trace/common/processor/TraceProcessor.java
index 8e1585fb93..4d47b7eeb2 100644
--- a/dd-trace-ot/src/main/java/datadog/trace/common/processor/TraceProcessor.java
+++ b/dd-trace-ot/src/main/java/datadog/trace/common/processor/TraceProcessor.java
@@ -2,6 +2,7 @@ package datadog.trace.common.processor;
 
 import datadog.opentracing.DDSpan;
 import datadog.trace.api.Config;
+import datadog.trace.common.processor.rule.ErrorRule;
 import datadog.trace.common.processor.rule.Status404Rule;
 import datadog.trace.common.processor.rule.Status5XXRule;
 import datadog.trace.common.processor.rule.URLAsResourceNameRule;
@@ -16,7 +17,7 @@ public class TraceProcessor {
   final Rule[] DEFAULT_RULES =
       new Rule[] {
         // Rules are applied in order.
-        new Status5XXRule(), new URLAsResourceNameRule(), new Status404Rule(),
+        new Status5XXRule(), new ErrorRule(), new URLAsResourceNameRule(), new Status404Rule(),
       };
 
   private final List<Rule> rules;
diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/ErrorRule.java b/dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/ErrorRule.java
new file mode 100644
index 0000000000..94f6ed5bb8
--- /dev/null
+++ b/dd-trace-ot/src/main/java/datadog/trace/common/processor/rule/ErrorRule.java
@@ -0,0 +1,24 @@
+package datadog.trace.common.processor.rule;
+
+import datadog.opentracing.DDSpan;
+import datadog.trace.common.processor.TraceProcessor;
+import io.opentracing.tag.Tags;
+import java.util.Collection;
+import java.util.Map;
+
+/** Converts error tag to error field */
+public class ErrorRule implements TraceProcessor.Rule {
+  @Override
+  public String[] aliases() {
+    return new String[] {"ErrorFlag"};
+  }
+
+  @Override
+  public void processSpan(
+      final DDSpan span, final Map<String, String> meta, final Collection<DDSpan> trace) {
+    if (meta.containsKey(Tags.ERROR.getKey())) {
+      span.setError(Boolean.parseBoolean(meta.get(Tags.ERROR.getKey())));
+      span.setTag(Tags.ERROR, null); // Remove the tag
+    }
+  }
+}