From 356ddeefb599493837596cb710f2f93e3b279e29 Mon Sep 17 00:00:00 2001 From: h6o <48004352+newbee1939@users.noreply.github.com> Date: Fri, 6 Mar 2026 04:25:05 +0900 Subject: [PATCH 1/2] fix(span): enforce StatusCode precedence rules in setStatus per specification (#6461) Signed-off-by: h6o <48004352+newbee1939@users.noreply.github.com> --- CHANGELOG.md | 1 + api/src/trace/span.ts | 18 +- .../opentelemetry-sdk-trace-base/src/Span.ts | 19 +- .../test/common/Span.test.ts | 300 ++++++++++++++---- .../test/Shim.test.ts | 4 + 5 files changed, 267 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c49dbc7574..2ffa659cf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2 ### :bug: Bug Fixes +* fix(sdk-trace-base): enforce StatusCode precedence rules in `setStatus` per specification [#6461](https://github.com/open-telemetry/opentelemetry-js/pull/6461) @newbee1939 * fix(sdk-trace-web): propagate `optimised` flag in `getElementXPath` recursion [#6335](https://github.com/open-telemetry/opentelemetry-js/pull/6335) @akkupratap323 ## 2.5.1 diff --git a/api/src/trace/span.ts b/api/src/trace/span.ts index 65e932f792..1d26cb7c68 100644 --- a/api/src/trace/span.ts +++ b/api/src/trace/span.ts @@ -89,11 +89,21 @@ export interface Span { addLinks(links: Link[]): this; /** - * Sets a status to the span. If used, this will override the default Span - * status. Default is {@link SpanStatusCode.UNSET}. SetStatus overrides the value - * of previous calls to SetStatus on the Span. + * Sets the status of the span. * - * @param status the SpanStatus to set. + * By default, a span has status {@link SpanStatusCode.UNSET}. + * Calling this method overrides that default. + * + * The status codes have a total order: `OK > ERROR > UNSET`. + * + * - Once {@link SpanStatusCode.OK} is set, any further attempts to change + * the status are ignored. + * - Any attempt to set {@link SpanStatusCode.UNSET} is always ignored. + * + * The `message` field is only used when {@link SpanStatusCode.ERROR} is set. + * For all other status codes, `message` is ignored. + * + * @param status The {@link SpanStatus} to set. */ setStatus(status: SpanStatus): this; diff --git a/packages/opentelemetry-sdk-trace-base/src/Span.ts b/packages/opentelemetry-sdk-trace-base/src/Span.ts index f276f1ceb3..391418ee16 100644 --- a/packages/opentelemetry-sdk-trace-base/src/Span.ts +++ b/packages/opentelemetry-sdk-trace-base/src/Span.ts @@ -240,19 +240,26 @@ export class SpanImpl implements Span { setStatus(status: SpanStatus): this { if (this._isSpanEnded()) return this; - this.status = { ...status }; + if (status.code === SpanStatusCode.UNSET) return this; + if (this.status.code === SpanStatusCode.OK) return this; + + const newStatus: SpanStatus = { code: status.code }; // When using try-catch, the caught "error" is of type `any`. When then assigning `any` to `status.message`, // TypeScript will not error. While this can happen during use of any API, it is more common on Span#setStatus() // as it's likely used in a catch-block. Therefore, we validate if `status.message` is actually a string, null, or // undefined to avoid an incorrect type causing issues downstream. - if (this.status.message != null && typeof status.message !== 'string') { - diag.warn( - `Dropping invalid status.message of type '${typeof status.message}', expected 'string'` - ); - delete this.status.message; + if (status.code === SpanStatusCode.ERROR) { + if (typeof status.message === 'string') { + newStatus.message = status.message; + } else if (status.message != null) { + diag.warn( + `Dropping invalid status.message of type '${typeof status.message}', expected 'string'` + ); + } } + this.status = newStatus; return this; } diff --git a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts index db491c892a..c825846699 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts @@ -943,51 +943,248 @@ describe('Span', () => { assert.strictEqual(span.events.length, 0); }); - it('should set an error status', () => { - const span = new SpanImpl({ - scope: tracer.instrumentationScope, - resource: tracer['_resource'], - context: ROOT_CONTEXT, - spanContext, - name, - kind: SpanKind.CLIENT, - spanLimits: tracer.getSpanLimits(), - spanProcessor: tracer['_spanProcessor'], + describe('setStatus', () => { + it('should set an error status', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ + code: SpanStatusCode.ERROR, + message: 'This is an error', + }); + + assert.strictEqual(span.status.code, SpanStatusCode.ERROR); + assert.strictEqual(span.status.message, 'This is an error'); }); - span.setStatus({ - code: SpanStatusCode.ERROR, - message: 'This is an error', + + it('should set an OK status', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ code: SpanStatusCode.OK }); + + assert.strictEqual(span.status.code, SpanStatusCode.OK); + assert.strictEqual(span.status.message, undefined); }); - span.end(); - assert.strictEqual(span.status.code, SpanStatusCode.ERROR); - assert.strictEqual(span.status.message, 'This is an error'); - }); + it('should ignore attempts to set UNSET from initial state', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); - it('should drop non-string status message', function () { - const warnStub = sinon.spy(diag, 'warn'); - const span = new SpanImpl({ - scope: tracer.instrumentationScope, - resource: tracer['_resource'], - context: ROOT_CONTEXT, - spanContext, - name, - kind: SpanKind.CLIENT, - spanLimits: tracer.getSpanLimits(), - spanProcessor: tracer['_spanProcessor'], + span.setStatus({ code: SpanStatusCode.UNSET }); + + assert.strictEqual(span.status.code, SpanStatusCode.UNSET); }); - span.setStatus({ - code: SpanStatusCode.ERROR, - message: new Error('this is not a string') as any, + + it('should drop non-string status message', function () { + const warnStub = sinon.spy(diag, 'warn'); + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ + code: SpanStatusCode.ERROR, + message: new Error('this is not a string') as any, + }); + + assert.strictEqual(span.status.code, SpanStatusCode.ERROR); + assert.strictEqual(span.status.message, undefined); + sinon.assert.calledOnceWithExactly( + warnStub, + "Dropping invalid status.message of type 'object', expected 'string'" + ); }); - span.end(); - assert.strictEqual(span.status.code, SpanStatusCode.ERROR); - assert.strictEqual(span.status.message, undefined); - sinon.assert.calledOnceWithExactly( - warnStub, - "Dropping invalid status.message of type 'object', expected 'string'" - ); + it('should ignore message for OK status', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ code: SpanStatusCode.OK, message: 'should be ignored' }); + + assert.strictEqual(span.status.code, SpanStatusCode.OK); + assert.strictEqual(span.status.message, undefined); + }); + + it('should ignore message for UNSET status', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ + code: SpanStatusCode.UNSET, + message: 'should be ignored', + }); + + assert.strictEqual(span.status.code, SpanStatusCode.UNSET); + assert.strictEqual(span.status.message, undefined); + }); + + it('should ignore attempts to set UNSET status', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ code: SpanStatusCode.ERROR, message: 'error' }); + span.setStatus({ code: SpanStatusCode.UNSET }); + + assert.strictEqual(span.status.code, SpanStatusCode.ERROR); + assert.strictEqual(span.status.message, 'error'); + }); + + it('should not allow overwriting OK status with ERROR', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ code: SpanStatusCode.OK }); + span.setStatus({ code: SpanStatusCode.ERROR, message: 'error' }); + + assert.strictEqual(span.status.code, SpanStatusCode.OK); + assert.strictEqual(span.status.message, undefined); + }); + + it('should not allow overwriting OK status with UNSET', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ code: SpanStatusCode.OK }); + span.setStatus({ code: SpanStatusCode.UNSET }); + + assert.strictEqual(span.status.code, SpanStatusCode.OK); + }); + + it('should allow overwriting ERROR status with OK', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ code: SpanStatusCode.ERROR, message: 'error' }); + span.setStatus({ code: SpanStatusCode.OK }); + + assert.strictEqual(span.status.code, SpanStatusCode.OK); + assert.strictEqual(span.status.message, undefined); + }); + + it('should allow overwriting ERROR with another ERROR', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ code: SpanStatusCode.ERROR, message: 'first' }); + span.setStatus({ code: SpanStatusCode.ERROR, message: 'second' }); + + assert.strictEqual(span.status.code, SpanStatusCode.ERROR); + assert.strictEqual(span.status.message, 'second'); + }); + + it('should not update status after span is ended', () => { + const span = new SpanImpl({ + scope: tracer.instrumentationScope, + resource: tracer['_resource'], + context: ROOT_CONTEXT, + spanContext, + name, + kind: SpanKind.CLIENT, + spanLimits: tracer.getSpanLimits(), + spanProcessor: tracer['_spanProcessor'], + }); + + span.setStatus({ + code: SpanStatusCode.ERROR, + message: 'This is an error', + }); + span.end(); + + span.setStatus({ + code: SpanStatusCode.OK, + message: 'OK', + }); + + assert.strictEqual(span.status.code, SpanStatusCode.ERROR); + assert.strictEqual(span.status.message, 'This is an error'); + }); }); it('should return ReadableSpan', () => { @@ -1182,33 +1379,6 @@ describe('Span', () => { assert.strictEqual(span.events.length, 2); }); - it('should return ReadableSpan with new status', () => { - const span = new SpanImpl({ - scope: tracer.instrumentationScope, - resource: tracer['_resource'], - context: ROOT_CONTEXT, - spanContext, - name, - kind: SpanKind.CLIENT, - spanLimits: tracer.getSpanLimits(), - spanProcessor: tracer['_spanProcessor'], - }); - span.setStatus({ - code: SpanStatusCode.ERROR, - message: 'This is an error', - }); - assert.strictEqual(span.status.code, SpanStatusCode.ERROR); - assert.strictEqual(span.status.message, 'This is an error'); - span.end(); - - // shouldn't update status - span.setStatus({ - code: SpanStatusCode.OK, - message: 'OK', - }); - assert.strictEqual(span.status.code, SpanStatusCode.ERROR); - }); - it('should only end a span once', () => { const span = new SpanImpl({ scope: tracer.instrumentationScope, diff --git a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts index 91e54ae6cb..fc73b746ca 100644 --- a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts +++ b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts @@ -329,7 +329,9 @@ describe('OpenTracing Shim', () => { span.setTag('error', false); assert.strictEqual(otSpan.status.code, SpanStatusCode.OK); + }); + it('maps string error tag to status code', () => { span.setTag('error', 'true'); assert.strictEqual(otSpan.status.code, SpanStatusCode.ERROR); @@ -352,7 +354,9 @@ describe('OpenTracing Shim', () => { span.addTags({ hello: 'stars', error: false }); assert.strictEqual(otSpan.status.code, SpanStatusCode.OK); + }); + it('maps string error tag to status code when adding multiple tags', () => { span.addTags({ hello: 'stars', error: 'true' }); assert.strictEqual(otSpan.status.code, SpanStatusCode.ERROR); From 30605228cc7b6404c2338fbe7ac949ee438c1c23 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Sat, 7 Mar 2026 00:44:12 +0000 Subject: [PATCH 2/2] chore(deps): update dependency eslint to v10 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3f7606b8cf..ff815f93d5 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,7 @@ "@typescript-eslint/parser": "8.41.0", "assert": "2.1.0", "benchmark": "2.1.4", - "eslint": "8.57.1", + "eslint": "10.0.3", "eslint-config-prettier": "10.1.8", "eslint-plugin-header": "3.1.1", "eslint-plugin-n": "17.23.2",