From b20186e5d1eb4a440f22287fb1198a8792a01d8a Mon Sep 17 00:00:00 2001 From: Thomas Hardy Date: Fri, 19 Sep 2025 01:08:31 -0400 Subject: [PATCH 1/3] Don't set error status on otel spans for benign exceptions --- .../src/instrumentation.ts | 6 ++-- packages/test/src/activities/index.ts | 13 +++++-- packages/test/src/test-otel.ts | 35 +++++++++++++++++++ packages/test/src/workflows/index.ts | 1 + .../test/src/workflows/throw-maybe-benign.ts | 11 ++++++ 5 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 packages/test/src/workflows/throw-maybe-benign.ts diff --git a/packages/interceptors-opentelemetry/src/instrumentation.ts b/packages/interceptors-opentelemetry/src/instrumentation.ts index 1df417b34..760337ebc 100644 --- a/packages/interceptors-opentelemetry/src/instrumentation.ts +++ b/packages/interceptors-opentelemetry/src/instrumentation.ts @@ -3,7 +3,7 @@ * @module */ import * as otel from '@opentelemetry/api'; -import { Headers, defaultPayloadConverter } from '@temporalio/common'; +import { ApplicationFailure, ApplicationFailureCategory, Headers, defaultPayloadConverter } from '@temporalio/common'; /** Default trace header for opentelemetry interceptors */ export const TRACE_HEADER = '_tracer-data'; @@ -43,8 +43,10 @@ async function wrapWithSpan( span.setStatus({ code: otel.SpanStatusCode.OK }); return ret; } catch (err: any) { + const isBenignErr = err instanceof ApplicationFailure && err.category === ApplicationFailureCategory.BENIGN; if (acceptableErrors === undefined || !acceptableErrors(err)) { - span.setStatus({ code: otel.SpanStatusCode.ERROR, message: err instanceof Error ? err.message : String(err) }); + const statusCode = isBenignErr ? otel.SpanStatusCode.UNSET : otel.SpanStatusCode.ERROR + span.setStatus({ code: statusCode, message: err instanceof Error ? err.message : String(err) }); span.recordException(err); } else { span.setStatus({ code: otel.SpanStatusCode.OK }); diff --git a/packages/test/src/activities/index.ts b/packages/test/src/activities/index.ts index 8a62bd7ec..f7a5183d5 100644 --- a/packages/test/src/activities/index.ts +++ b/packages/test/src/activities/index.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-empty-function */ /* eslint-disable @typescript-eslint/explicit-module-boundary-types */ -import { Context } from '@temporalio/activity'; -import { ApplicationFailure } from '@temporalio/common'; +import { activityInfo, Context } from '@temporalio/activity'; +import { ApplicationFailure, ApplicationFailureCategory } from '@temporalio/common'; import { ProtoActivityInput, ProtoActivityResult } from '../../protos/root'; import { cancellableFetch as cancellableFetchInner } from './cancellable-fetch'; import { fakeProgress as fakeProgressInner } from './fake-progress'; @@ -93,3 +93,12 @@ export async function progressiveSleep(): Promise { export async function protoActivity(args: ProtoActivityInput): Promise { return ProtoActivityResult.create({ sentence: `${args.name} is ${args.age} years old.` }); } + +export async function throwMaybeBenign(): Promise { + if (activityInfo().attempt === 1) { + throw ApplicationFailure.create({ message: "not benign"}); + } + if (activityInfo().attempt === 2) { + throw ApplicationFailure.create({ message: "benign", category: ApplicationFailureCategory.BENIGN }); + } +} \ No newline at end of file diff --git a/packages/test/src/test-otel.ts b/packages/test/src/test-otel.ts index 3ae8bee3b..942e5d09a 100644 --- a/packages/test/src/test-otel.ts +++ b/packages/test/src/test-otel.ts @@ -470,4 +470,39 @@ if (RUN_INTEGRATION_TESTS) { const exceptionEvents = span.events.filter((event) => event.name === 'exception'); t.is(exceptionEvents.length, 1); }); + + test('Otel workflow omits ApplicationError with BENIGN category', async (t) => { + const memoryExporter = new InMemorySpanExporter(); + const provider = new BasicTracerProvider(); + provider.addSpanProcessor(new SimpleSpanProcessor(memoryExporter)); + provider.register(); + const tracer = provider.getTracer('test-error-tracer'); + + const worker = await Worker.create({ + workflowsPath: require.resolve('./workflows'), + activities, + taskQueue: 'test-otel-benign-err', + interceptors: { + activity: [(ctx) => { + return { inbound: new OpenTelemetryActivityInboundInterceptor(ctx, { tracer }) } + }], + } + }); + + const client = new WorkflowClient(); + + await worker.runUntil(client.execute(workflows.throwMaybeBenignErr, { + taskQueue: 'test-otel-benign-err', + workflowId: uuid4(), + retry: { maximumAttempts : 3 } + })) + + const spans = memoryExporter.getFinishedSpans(); + t.is(spans.length, 3); + t.is(spans[0].status.code, SpanStatusCode.ERROR); + t.is(spans[0].status.message, "not benign"); + t.is(spans[1].status.code, SpanStatusCode.UNSET); + t.is(spans[1].status.message, "benign"); + t.is(spans[2].status.code, SpanStatusCode.OK); + }); } diff --git a/packages/test/src/workflows/index.ts b/packages/test/src/workflows/index.ts index d4959ec2f..d86048b26 100644 --- a/packages/test/src/workflows/index.ts +++ b/packages/test/src/workflows/index.ts @@ -79,6 +79,7 @@ export * from './swc'; export * from './tasks-and-microtasks'; export * from './text-encoder-decoder'; export * from './throw-async'; +export * from './throw-maybe-benign'; export * from './trailing-timer'; export * from './try-to-continue-after-completion'; export * from './two-strings'; diff --git a/packages/test/src/workflows/throw-maybe-benign.ts b/packages/test/src/workflows/throw-maybe-benign.ts new file mode 100644 index 000000000..b12f1d43c --- /dev/null +++ b/packages/test/src/workflows/throw-maybe-benign.ts @@ -0,0 +1,11 @@ +import * as workflow from '@temporalio/workflow'; +import * as activities from "../activities"; + +const { throwMaybeBenign } = workflow.proxyActivities({ + startToCloseTimeout: '5s', + retry: { maximumAttempts: 3, backoffCoefficient: 1, initialInterval: 500}, + }); + +export async function throwMaybeBenignErr(): Promise { + await throwMaybeBenign(); +} \ No newline at end of file From 7ab08822fa4fe12137ac0fe56eba340fa3480a38 Mon Sep 17 00:00:00 2001 From: Thomas Hardy Date: Fri, 19 Sep 2025 01:34:13 -0400 Subject: [PATCH 2/3] Formatting --- .../src/instrumentation.ts | 2 +- packages/test/package.json | 3 ++- packages/test/src/activities/index.ts | 6 ++--- packages/test/src/test-otel.ts | 26 +++++++++++-------- .../test/src/workflows/throw-maybe-benign.ts | 12 ++++----- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/packages/interceptors-opentelemetry/src/instrumentation.ts b/packages/interceptors-opentelemetry/src/instrumentation.ts index 760337ebc..78a30797c 100644 --- a/packages/interceptors-opentelemetry/src/instrumentation.ts +++ b/packages/interceptors-opentelemetry/src/instrumentation.ts @@ -45,7 +45,7 @@ async function wrapWithSpan( } catch (err: any) { const isBenignErr = err instanceof ApplicationFailure && err.category === ApplicationFailureCategory.BENIGN; if (acceptableErrors === undefined || !acceptableErrors(err)) { - const statusCode = isBenignErr ? otel.SpanStatusCode.UNSET : otel.SpanStatusCode.ERROR + const statusCode = isBenignErr ? otel.SpanStatusCode.UNSET : otel.SpanStatusCode.ERROR; span.setStatus({ code: statusCode, message: err instanceof Error ? err.message : String(err) }); span.recordException(err); } else { diff --git a/packages/test/package.json b/packages/test/package.json index 8e2d4537a..954069b2b 100644 --- a/packages/test/package.json +++ b/packages/test/package.json @@ -8,7 +8,8 @@ "build:ts": "tsc --build", "build:protos": "node ./scripts/compile-proto.js", "test": "ava ./lib/test-*.js", - "test.watch": "ava --watch ./lib/test-*.js" + "test.watch": "ava --watch ./lib/test-*.js", + "test-otel": "ava ./lib/test-otel.js" }, "ava": { "timeout": "60s", diff --git a/packages/test/src/activities/index.ts b/packages/test/src/activities/index.ts index f7a5183d5..f6b981644 100644 --- a/packages/test/src/activities/index.ts +++ b/packages/test/src/activities/index.ts @@ -96,9 +96,9 @@ export async function protoActivity(args: ProtoActivityInput): Promise { if (activityInfo().attempt === 1) { - throw ApplicationFailure.create({ message: "not benign"}); + throw ApplicationFailure.create({ message: 'not benign' }); } if (activityInfo().attempt === 2) { - throw ApplicationFailure.create({ message: "benign", category: ApplicationFailureCategory.BENIGN }); + throw ApplicationFailure.create({ message: 'benign', category: ApplicationFailureCategory.BENIGN }); } -} \ No newline at end of file +} diff --git a/packages/test/src/test-otel.ts b/packages/test/src/test-otel.ts index 942e5d09a..a45a8951a 100644 --- a/packages/test/src/test-otel.ts +++ b/packages/test/src/test-otel.ts @@ -483,26 +483,30 @@ if (RUN_INTEGRATION_TESTS) { activities, taskQueue: 'test-otel-benign-err', interceptors: { - activity: [(ctx) => { - return { inbound: new OpenTelemetryActivityInboundInterceptor(ctx, { tracer }) } - }], - } + activity: [ + (ctx) => { + return { inbound: new OpenTelemetryActivityInboundInterceptor(ctx, { tracer }) }; + }, + ], + }, }); const client = new WorkflowClient(); - await worker.runUntil(client.execute(workflows.throwMaybeBenignErr, { - taskQueue: 'test-otel-benign-err', - workflowId: uuid4(), - retry: { maximumAttempts : 3 } - })) + await worker.runUntil( + client.execute(workflows.throwMaybeBenignErr, { + taskQueue: 'test-otel-benign-err', + workflowId: uuid4(), + retry: { maximumAttempts: 3 }, + }) + ); const spans = memoryExporter.getFinishedSpans(); t.is(spans.length, 3); t.is(spans[0].status.code, SpanStatusCode.ERROR); - t.is(spans[0].status.message, "not benign"); + t.is(spans[0].status.message, 'not benign'); t.is(spans[1].status.code, SpanStatusCode.UNSET); - t.is(spans[1].status.message, "benign"); + t.is(spans[1].status.message, 'benign'); t.is(spans[2].status.code, SpanStatusCode.OK); }); } diff --git a/packages/test/src/workflows/throw-maybe-benign.ts b/packages/test/src/workflows/throw-maybe-benign.ts index b12f1d43c..86d94e12c 100644 --- a/packages/test/src/workflows/throw-maybe-benign.ts +++ b/packages/test/src/workflows/throw-maybe-benign.ts @@ -1,11 +1,11 @@ import * as workflow from '@temporalio/workflow'; -import * as activities from "../activities"; +import * as activities from '../activities'; const { throwMaybeBenign } = workflow.proxyActivities({ - startToCloseTimeout: '5s', - retry: { maximumAttempts: 3, backoffCoefficient: 1, initialInterval: 500}, - }); + startToCloseTimeout: '5s', + retry: { maximumAttempts: 3, backoffCoefficient: 1, initialInterval: 500 }, +}); export async function throwMaybeBenignErr(): Promise { - await throwMaybeBenign(); -} \ No newline at end of file + await throwMaybeBenign(); +} From 682379e18c7de5b96753462e4c44f03e8341416d Mon Sep 17 00:00:00 2001 From: Thomas Hardy Date: Mon, 22 Sep 2025 15:06:22 -0400 Subject: [PATCH 3/3] review suggestions --- packages/interceptors-opentelemetry/src/instrumentation.ts | 2 +- packages/test/package.json | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/interceptors-opentelemetry/src/instrumentation.ts b/packages/interceptors-opentelemetry/src/instrumentation.ts index 78a30797c..433c0d1ed 100644 --- a/packages/interceptors-opentelemetry/src/instrumentation.ts +++ b/packages/interceptors-opentelemetry/src/instrumentation.ts @@ -46,7 +46,7 @@ async function wrapWithSpan( const isBenignErr = err instanceof ApplicationFailure && err.category === ApplicationFailureCategory.BENIGN; if (acceptableErrors === undefined || !acceptableErrors(err)) { const statusCode = isBenignErr ? otel.SpanStatusCode.UNSET : otel.SpanStatusCode.ERROR; - span.setStatus({ code: statusCode, message: err instanceof Error ? err.message : String(err) }); + span.setStatus({ code: statusCode, message: (err as Error).message ?? String(err) }); span.recordException(err); } else { span.setStatus({ code: otel.SpanStatusCode.OK }); diff --git a/packages/test/package.json b/packages/test/package.json index 954069b2b..8e2d4537a 100644 --- a/packages/test/package.json +++ b/packages/test/package.json @@ -8,8 +8,7 @@ "build:ts": "tsc --build", "build:protos": "node ./scripts/compile-proto.js", "test": "ava ./lib/test-*.js", - "test.watch": "ava --watch ./lib/test-*.js", - "test-otel": "ava ./lib/test-otel.js" + "test.watch": "ava --watch ./lib/test-*.js" }, "ava": { "timeout": "60s",