Don't set error status on otel spans for benign exceptions

Author: THardy98

temporal-opentracing/src/main/java/io/temporal/opentracing/internal/SpanFactory.java

@@ -9,6 +9,7 @@
 import io.opentracing.Tracer.SpanBuilder;
 import io.opentracing.log.Fields;
 import io.opentracing.tag.Tags;
+import io.temporal.internal.common.FailureUtils;
 import io.temporal.opentracing.OpenTracingOptions;
 import io.temporal.opentracing.SpanCreationContext;
 import io.temporal.opentracing.SpanOperationType;
@@ -238,8 +239,9 @@ public Tracer.SpanBuilder createWorkflowHandleQuerySpan(
   @SuppressWarnings("deprecation")
   public void logFail(Span toSpan, Throwable failReason) {
     toSpan.setTag(StandardTagNames.FAILED, true);
-    toSpan.setTag(Tags.ERROR, options.getIsErrorPredicate().test(failReason));
-
+    if (!FailureUtils.isBenignApplicationFailure(failReason)) {
+      toSpan.setTag(Tags.ERROR, options.getIsErrorPredicate().test(failReason));
+    }
     Map<String, Object> logPayload = new HashMap<>();
     logPayload.put(Fields.EVENT, "error");
     logPayload.put(Fields.ERROR_KIND, failReason.getClass().getName());

temporal-opentracing/src/test/java/io/temporal/opentracing/ActivityFailureTest.java

@@ -7,13 +7,15 @@
 import io.opentracing.mock.MockTracer;
 import io.opentracing.tag.Tags;
 import io.opentracing.util.ThreadLocalScopeManager;
+import io.temporal.activity.Activity;
 import io.temporal.activity.ActivityInterface;
 import io.temporal.activity.ActivityMethod;
 import io.temporal.activity.ActivityOptions;
 import io.temporal.client.WorkflowClient;
 import io.temporal.client.WorkflowClientOptions;
 import io.temporal.client.WorkflowOptions;
 import io.temporal.common.RetryOptions;
+import io.temporal.failure.ApplicationErrorCategory;
 import io.temporal.failure.ApplicationFailure;
 import io.temporal.testing.internal.SDKTestWorkflowRule;
 import io.temporal.worker.WorkerFactoryOptions;
@@ -152,4 +154,108 @@ public void testActivityFailureSpanStructure() {
     assertEquals(activityStartSpan.context().spanId(), activitySuccessfulRunSpan.parentId());
     assertEquals("RunActivity:Activity", activitySuccessfulRunSpan.operationName());
   }
+
+  @Rule
+  public SDKTestWorkflowRule benignTestRule =
+      SDKTestWorkflowRule.newBuilder()
+          .setWorkerFactoryOptions(
+              WorkerFactoryOptions.newBuilder()
+                  .setWorkerInterceptors(new OpenTracingWorkerInterceptor(OT_OPTIONS))
+                  .validateAndBuildWithDefaults())
+          .setWorkflowTypes(BenignWorkflowImpl.class)
+          .setActivityImplementations(new BenignFailingActivityImpl())
+          .build();
+
+  @ActivityInterface
+  public interface BenignTestActivity {
+    @ActivityMethod
+    String throwMaybeBenign();
+  }
+
+  @WorkflowInterface
+  public interface BenignTestWorkflow {
+    @WorkflowMethod
+    String workflow();
+  }
+
+  public static class BenignFailingActivityImpl implements BenignTestActivity {
+    @Override
+    public String throwMaybeBenign() {
+      int attempt = Activity.getExecutionContext().getInfo().getAttempt();
+      if (attempt == 1) {
+        // First attempt: regular failure
+        throw ApplicationFailure.newFailure("not benign", "TestFailure");
+      } else if (attempt == 2) {
+        // Second attempt: benign failure
+        throw ApplicationFailure.newBuilder()
+            .setMessage("benign")
+            .setType("TestFailure")
+            .setCategory(ApplicationErrorCategory.BENIGN)
+            .build();
+      } else {
+        // Third attempt: success
+        return "success";
+      }
+    }
+  }
+
+  public static class BenignWorkflowImpl implements BenignTestWorkflow {
+    private final BenignTestActivity activity =
+        Workflow.newActivityStub(
+            BenignTestActivity.class,
+            ActivityOptions.newBuilder()
+                .setStartToCloseTimeout(Duration.ofMinutes(1))
+                .setRetryOptions(
+                    RetryOptions.newBuilder()
+                        .setMaximumAttempts(3)
+                        .setBackoffCoefficient(1)
+                        .setInitialInterval(Duration.ofMillis(100))
+                        .build())
+                .validateAndBuildWithDefaults());
+
+    @Override
+    public String workflow() {
+      return activity.throwMaybeBenign();
+    }
+  }
+
+  @Test
+  public void testBenignApplicationFailureSpanBehavior() {
+    MockSpan span = mockTracer.buildSpan("BenignTestFunction").start();
+
+    WorkflowClient client = benignTestRule.getWorkflowClient();
+    try (Scope scope = mockTracer.scopeManager().activate(span)) {
+      BenignTestWorkflow workflow =
+          client.newWorkflowStub(
+              BenignTestWorkflow.class,
+              WorkflowOptions.newBuilder()
+                  .setTaskQueue(benignTestRule.getTaskQueue())
+                  .validateBuildWithDefaults());
+      assertEquals("success", workflow.workflow());
+    } finally {
+      span.finish();
+    }
+
+    List<MockSpan> allSpans = mockTracer.finishedSpans();
+
+    // Filter to only activity execution spans (RunActivity spans created by worker interceptor)
+    List<MockSpan> activityRunSpans =
+        allSpans.stream()
+            .filter(s -> s.operationName().startsWith("RunActivity:"))
+            .collect(java.util.stream.Collectors.toList());
+
+    assertEquals(3, activityRunSpans.size());
+
+    // First attempt: regular failure - should have ERROR tag
+    MockSpan firstAttemptSpan = activityRunSpans.get(0);
+    assertEquals(true, firstAttemptSpan.tags().get(Tags.ERROR.getKey()));
+
+    // Second attempt: benign failure - should NOT have ERROR tag
+    MockSpan secondAttemptSpan = activityRunSpans.get(1);
+    assertEquals(null, secondAttemptSpan.tags().get(Tags.ERROR.getKey()));
+
+    // Third attempt: success - should not have ERROR tag
+    MockSpan thirdAttemptSpan = activityRunSpans.get(2);
+    assertEquals(null, thirdAttemptSpan.tags().get(Tags.ERROR.getKey()));
+  }
 }