Skip to content

Commit

Permalink
fix(sdk-trace-base): avoid keeping non-string status.message on Span#…
Browse files Browse the repository at this point in the history
…setStatus() (#4999)
  • Loading branch information
pichlermarc authored Sep 23, 2024
1 parent 4ef7395 commit 8900cfd
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 13 additions & 1 deletion packages/opentelemetry-sdk-trace-base/src/Span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
26 changes: 26 additions & 0 deletions packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 8900cfd

Please sign in to comment.