From 8900cfdbbab27f464df569a4c8309b96594c276b Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Mon, 23 Sep 2024 17:44:37 +0200 Subject: [PATCH] fix(sdk-trace-base): avoid keeping non-string status.message on Span#setStatus() (#4999) --- CHANGELOG.md | 2 ++ .../opentelemetry-sdk-trace-base/src/Span.ts | 14 +++++++++- .../test/common/Span.test.ts | 26 +++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec32110381..33e57f2fa9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se ### :bug: (Bug Fix) +* fix(sdk-trace-base): avoid keeping non-string `status.message` on `Span#setStatus()` [#4999](https://github.com/open-telemetry/opentelemetry-js/pull/4999) @pichlermarc + ### :books: (Refine Doc) ### :house: (Internal) diff --git a/packages/opentelemetry-sdk-trace-base/src/Span.ts b/packages/opentelemetry-sdk-trace-base/src/Span.ts index 3b7758f138..72eb541341 100644 --- a/packages/opentelemetry-sdk-trace-base/src/Span.ts +++ b/packages/opentelemetry-sdk-trace-base/src/Span.ts @@ -226,7 +226,19 @@ export class Span implements APISpan, ReadableSpan { setStatus(status: SpanStatus): this { if (this._isSpanEnded()) return this; - this.status = status; + this.status = { ...status }; + + // 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; + } + 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 33808acd54..ef06a1d884 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts @@ -853,6 +853,32 @@ describe('Span', () => { message: 'This is an error', }); span.end(); + + assert.strictEqual(span.status.code, SpanStatusCode.ERROR); + assert.strictEqual(span.status.message, 'This is an error'); + }); + + it('should drop non-string status message', function () { + const warnStub = sinon.spy(diag, 'warn'); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); + span.setStatus({ + code: SpanStatusCode.ERROR, + message: new Error('this is not a string') as any, + }); + 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 return ReadableSpan', () => {