Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ Sentry.startSpan({ name: 'test-span', op: 'test' }, () => {
inactiveSpan.end();

Sentry.startSpanManual({ name: 'test-manual-span' }, span => {
// noop
// 2 = SPAN_STATUS_ERROR. The message must be preserved as the `sentry.status.message`
// attribute on the streamed span, since v2 statuses are reduced to ok/error.
span.setStatus({ code: 2, message: 'Connection Refused' });
span.end();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
SEMANTIC_ATTRIBUTE_SENTRY_SEGMENT_ID,
SEMANTIC_ATTRIBUTE_SENTRY_SEGMENT_NAME,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE,
} from '@sentry/core';
import { sentryTest } from '../../../../utils/fixtures';
import { shouldSkipTracingTest } from '../../../../utils/helpers';
Expand Down Expand Up @@ -171,14 +172,18 @@ sentryTest(
type: 'string',
value: 'production',
},
[SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]: {
type: 'string',
value: 'Connection Refused',
},
},
end_timestamp: expect.any(Number),
is_segment: false,
name: 'test-manual-span',
parent_span_id: segmentSpanId,
span_id: expect.stringMatching(/^[\da-f]{16}$/),
start_timestamp: expect.any(Number),
status: 'ok',
status: 'error',
trace_id: traceId,
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
cleanupChildProcesses();
});

const assertMysqlSpans = (container: SerializedStreamedSpanContainer): void => {
const assertMysqlSpans = (container: SerializedStreamedSpanContainer, override?: Record<string, unknown>): void => {
const segmentSpan = container.items.find(item => item.is_segment);
expect(segmentSpan?.name).toBe('Test Transaction');

Expand Down Expand Up @@ -93,7 +93,7 @@
trace_id: expect.stringMatching(/^[\da-f]{32}$/),
};

expect(dbSpans).toEqual([

Check failure on line 96 in dev-packages/node-integration-tests/suites/tracing/mysql-streamed/test.ts

View workflow job for this annotation

GitHub Actions / Node (18) Integration Tests

suites/tracing/mysql-streamed/test.ts > mysql auto instrumentation (streamed) > without connection.connect() > should auto-instrument `mysql` package without connection.connect()

AssertionError: expected [ { …(9) }, …(1) ] to deeply equal [ …(2) ] - Expected + Received @@ -63,10 +63,14 @@ }, "sentry.span.source": { "type": "string", "value": "task", }, + "sentry.status.message": { + "type": "string", + "value": "connect ECONNREFUSED ::1:3306", + }, }, "end_timestamp": Any<Number>, "is_segment": false, "name": "SELECT 1 + 1 AS solution", "parent_span_id": StringMatching /^[\da-f]{16}$/, @@ -77,11 +81,11 @@ }, { "attributes": { "db.connection_string": { "type": "string", - "value": StringMatching /^jdbc:mysql:\/\/localhost:.*/, + "value": "jdbc:mysql://localhost:3306", }, "db.statement": { "type": "string", "value": "SELECT NOW()", }, @@ -97,11 +101,11 @@ "type": "string", "value": "localhost", }, "net.peer.port": { "type": "integer", - "value": Any<Number>, + "value": 3306, }, "otel.kind": { "type": "string", "value": "CLIENT", }, @@ -121,15 +125,15 @@ "type": "string", "value": "sentry.javascript.node", }, "sentry.sdk.version": { "type": "string", - "value": Any<String>, + "value": "10.62.0", }, "sentry.segment.id": { "type": "string", - "value": StringMatching /^[\da-f]{16}$/, + "value": "0e1bdd2571f3c823", }, "sentry.segment.name": { "type": "string", "value": "Test Transaction", }, ❯ assertMysqlSpans suites/tracing/mysql-streamed/test.ts:96:21 ❯ span suites/tracing/mysql-streamed/test.ts:155:15 ❯ expectSpanContainer utils/runner/createRunner.ts:725:5 ❯ assertExpectedEnvelope utils/runner/createRunner.ts:659:5 ❯ newEnvelope utils/runner/createRunner.ts:331:15 ❯ tryParseEnvelopeFromStdoutLine utils/runner/createRunner.ts:449:15 ❯ Socket.<anonymous> utils/runner/createRunner.ts:467:15

Check failure on line 96 in dev-packages/node-integration-tests/suites/tracing/mysql-streamed/test.ts

View workflow job for this annotation

GitHub Actions / Node (18) Integration Tests

suites/tracing/mysql-streamed/test.ts > mysql auto instrumentation (streamed) > query without callback > should auto-instrument `mysql` package when using query without callback

AssertionError: expected [ { …(9) }, …(1) ] to deeply equal [ …(2) ] - Expected + Received @@ -63,10 +63,14 @@ }, "sentry.span.source": { "type": "string", "value": "task", }, + "sentry.status.message": { + "type": "string", + "value": "connect ECONNREFUSED ::1:3306", + }, }, "end_timestamp": Any<Number>, "is_segment": false, "name": "SELECT 1 + 1 AS solution", "parent_span_id": StringMatching /^[\da-f]{16}$/, @@ -77,11 +81,11 @@ }, { "attributes": { "db.connection_string": { "type": "string", - "value": StringMatching /^jdbc:mysql:\/\/localhost:.*/, + "value": "jdbc:mysql://localhost:3306", }, "db.statement": { "type": "string", "value": "SELECT NOW()", }, @@ -97,11 +101,11 @@ "type": "string", "value": "localhost", }, "net.peer.port": { "type": "integer", - "value": Any<Number>, + "value": 3306, }, "otel.kind": { "type": "string", "value": "CLIENT", }, @@ -121,15 +125,15 @@ "type": "string", "value": "sentry.javascript.node", }, "sentry.sdk.version": { "type": "string", - "value": Any<String>, + "value": "10.62.0", }, "sentry.segment.id": { "type": "string", - "value": StringMatching /^[\da-f]{16}$/, + "value": "d168664dbf8adb91", }, "sentry.segment.name": { "type": "string", "value": "Test Transaction", }, @@ -138,10 +142,14 @@ "value": "task", }, "sentry.span.source": { "type": "string", "value": "task", + }, + "sentry.status.message": { + "type": "string", + "value": "connect ECONNREFUSED ::1:3306", }, }, "end_timestamp": Any<Number>, "is_segment": false, "name": "SELECT NOW()", ❯ assertMysqlSpans suites/tracing/mysql-streamed/test.ts:96:21 ❯ expectSpanContainer utils/runner/createRunner.ts:725:5 ❯ assertExpectedEnvelope utils/runner/createRunner.ts:659:5 ❯ newEnvelope utils/runner/createRunner.ts:331:15 ❯ tryParseEnvelopeFromStdoutLine utils/runner/createRunner.ts:449:15 ❯ Socket.<anonymous> utils/runner/createRunner.ts:467:15

Check failure on line 96 in dev-packages/node-integration-tests/suites/tracing/mysql-streamed/test.ts

View workflow job for this annotation

GitHub Actions / Node (18) Integration Tests

suites/tracing/mysql-streamed/test.ts > mysql auto instrumentation (streamed) > with connection.connect() > should auto-instrument `mysql` package when using connection.connect()

AssertionError: expected [ { …(9) }, …(1) ] to deeply equal [ …(2) ] - Expected + Received @@ -63,10 +63,14 @@ }, "sentry.span.source": { "type": "string", "value": "task", }, + "sentry.status.message": { + "type": "string", + "value": "connect ECONNREFUSED ::1:3306", + }, }, "end_timestamp": Any<Number>, "is_segment": false, "name": "SELECT 1 + 1 AS solution", "parent_span_id": StringMatching /^[\da-f]{16}$/, @@ -77,11 +81,11 @@ }, { "attributes": { "db.connection_string": { "type": "string", - "value": StringMatching /^jdbc:mysql:\/\/localhost:.*/, + "value": "jdbc:mysql://localhost:3306", }, "db.statement": { "type": "string", "value": "SELECT NOW()", }, @@ -97,11 +101,11 @@ "type": "string", "value": "localhost", }, "net.peer.port": { "type": "integer", - "value": Any<Number>, + "value": 3306, }, "otel.kind": { "type": "string", "value": "CLIENT", }, @@ -121,15 +125,15 @@ "type": "string", "value": "sentry.javascript.node", }, "sentry.sdk.version": { "type": "string", - "value": Any<String>, + "value": "10.62.0", }, "sentry.segment.id": { "type": "string", - "value": StringMatching /^[\da-f]{16}$/, + "value": "e03d11cbed5fe9a9", }, "sentry.segment.name": { "type": "string", "value": "Test Transaction", }, ❯ assertMysqlSpans suites/tracing/mysql-streamed/test.ts:96:21 ❯ span suites/tracing/mysql-streamed/test.ts:129:15 ❯ expectSpanContainer utils/runner/createRunner.ts:725:5 ❯ assertExpectedEnvelope utils/runner/createRunner.ts:659:5 ❯ newEnvelope utils/runner/createRunner.ts:331:15 ❯ tryParseEnvelopeFromStdoutLine utils/runner/createRunner.ts:449:15 ❯ Socket.<anonymous> utils/runner/createRunner.ts:467:15
{
attributes: {
...COMMON_ATTRIBUTES,
Expand All @@ -112,6 +112,7 @@
type: 'string',
value: 'SELECT NOW()',
},
...override?.attributes,
},
name: 'SELECT NOW()',
...COMMON_SPAN_PROPS,
Expand All @@ -122,7 +123,17 @@
describe('with connection.connect()', () => {
createCjsTests(__dirname, 'scenario-withConnect.mjs', 'instrument.mjs', (createTestRunner, test) => {
test('should auto-instrument `mysql` package when using connection.connect()', async () => {
await createTestRunner().expect({ span: assertMysqlSpans }).start().completed();
await createTestRunner()
.expect({
span: container =>
assertMysqlSpans(container, {
attributes: {
'sentry.status.message': { type: 'string', value: 'Cannot enqueue Query after fatal error.' },
},
}),
})
.start()
.completed();
});
});
});
Expand All @@ -138,7 +149,17 @@
describe('without connection.connect()', () => {
createCjsTests(__dirname, 'scenario-withoutConnect.mjs', 'instrument.mjs', (createTestRunner, test) => {
test('should auto-instrument `mysql` package without connection.connect()', async () => {
await createTestRunner().expect({ span: assertMysqlSpans }).start().completed();
await createTestRunner()
.expect({
span: container =>
assertMysqlSpans(container, {
attributes: {
'sentry.status.message': { type: 'string', value: 'Cannot enqueue Query after fatal error.' },
},
}),
})
.start()
.completed();
});
});
});
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/semanticAttributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ export const SEMANTIC_ATTRIBUTE_SENTRY_OP = 'sentry.op';
*/
export const SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN = 'sentry.origin';

/**
* Holds the human-readable span status message (e.g. set via
* `span.setStatus({ code, message })`).
*
* Streamed (v2) span statuses are reduced to `ok`/`error`, so we preserve the
* message as an attribute instead of dropping it. This mirrors the attribute
* Sentry's OTLP ingestion uses for the same purpose.
*/
export const SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE = 'sentry.status.message';

/** The reason why an idle span finished. */
export const SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON = 'sentry.idle_span_finish_reason';

Expand Down
7 changes: 4 additions & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ import type { TimedEvent } from '../types/timedEvent';
import { debug } from '../utils/debug-logger';
import { generateSpanId, generateTraceId } from '../utils/propagationContext';
import {
addStatusMessageAttribute,
convertSpanLinksForEnvelope,
getRootSpan,
getSimpleStatusMessage,
getSimpleStatus,
getSpanDescendants,
getStatusMessage,
getStreamedSpanLinks,
Expand Down Expand Up @@ -271,8 +272,8 @@ export class SentrySpan implements Span {
// just in case _endTime is not set, we use the start time (i.e. duration 0)
end_timestamp: this._endTime ?? this._startTime,
is_segment: this._isStandaloneSpan || this === getRootSpan(this),
status: getSimpleStatusMessage(this._status),
attributes: this._attributes,
status: getSimpleStatus(this._status),
attributes: addStatusMessageAttribute(this._attributes, this._status),
links: getStreamedSpanLinks(this._links),
};
}
Expand Down
26 changes: 23 additions & 3 deletions packages/core/src/utils/spanUtils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// oxlint-disable max-lines
import { getAsyncContextStrategy } from '../asyncContext';
import type { RawAttributes } from '../attributes';
import { serializeAttributes } from '../attributes';
Expand All @@ -8,6 +9,7 @@ import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE,
} from '../semanticAttributes';
import type { SentrySpan } from '../tracing/sentrySpan';
import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus';
Expand Down Expand Up @@ -228,8 +230,8 @@ export function spanToStreamedSpanJSON(span: Span): StreamedSpanJSON {
start_timestamp: spanTimeInputToSeconds(startTime),
end_timestamp: spanTimeInputToSeconds(endTime),
is_segment: span === INTERNAL_getSegmentSpan(span),
status: getSimpleStatusMessage(status),
attributes,
status: getSimpleStatus(status),
attributes: addStatusMessageAttribute(attributes, status),
links: getStreamedSpanLinks(links),
};
}
Expand Down Expand Up @@ -330,7 +332,7 @@ export function getStatusMessage(status: SpanStatus | undefined): string | undef
/**
* Convert the various statuses to the simple ones expected by Sentry for streamed spans ('ok' is default).
*/
export function getSimpleStatusMessage(status: SpanStatus | undefined): 'ok' | 'error' {
export function getSimpleStatus(status: SpanStatus | undefined): 'ok' | 'error' {
return !status ||
status.code === SPAN_STATUS_OK ||
status.code === SPAN_STATUS_UNSET ||
Expand All @@ -339,6 +341,24 @@ export function getSimpleStatusMessage(status: SpanStatus | undefined): 'ok' | '
: 'error';
}

/**
* Returns the span's attributes with the SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE attribute added
* if the span has an error status message worth preserving.
*
* An explicitly set attribute is never overwritten, and the original attributes
* reference is returned untouched when there is no message to add.
*/
export function addStatusMessageAttribute(
attributes: SpanAttributes,
status: SpanStatus | undefined,
): RawAttributes<Record<string, unknown>> {
const statusMessage = getSimpleStatus(status) === 'error' ? status?.message : undefined;
return {
...(statusMessage && { [SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]: statusMessage }),
...attributes,
};
Comment on lines +355 to +359

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The addStatusMessageAttribute function always creates a new object, which contradicts its documentation comment stating the original reference would be returned.
Severity: LOW

Suggested Fix

Modify addStatusMessageAttribute to check if a statusMessage exists. If it does not, return the original attributes object directly. Otherwise, proceed with creating and returning a new object with the added attribute.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/core/src/utils/spanUtils.ts#L355-L359

Potential issue: The function `addStatusMessageAttribute` in `spanUtils.ts` has a
comment stating that it will return the original `attributes` object reference untouched
if no status message is added. However, the implementation always creates a new object
via spread syntax (`{ ...attributes }`), even when no message is present. While this
does not cause a functional bug as no downstream code relies on reference equality, it
creates a minor inefficiency by allocating a new object unnecessarily and makes the
code's behavior inconsistent with its documentation.

Did we get this right? 👍 / 👎 to inform future reviews.

}

const CHILD_SPANS_FIELD = '_sentryChildSpans';
const ROOT_SPAN_FIELD = '_sentryRootSpan';

Expand Down
80 changes: 80 additions & 0 deletions packages/core/test/lib/utils/spanUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE,
SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE,
SentrySpan,
setCurrentClient,
Expand Down Expand Up @@ -493,6 +494,52 @@ describe('spanToJSON', () => {
],
});
});
it('preserves an error status message as the sentry.status.message attribute', () => {
const span = new SentrySpan({ name: 'test name' });
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'Connection Refused' });

const json = spanToStreamedSpanJSON(span);
expect(json.status).toBe('error');
expect(json.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]).toBe('Connection Refused');
});

it('does not set a status message for ok spans', () => {
const span = new SentrySpan({ name: 'test name' });
span.setStatus({ code: SPAN_STATUS_OK });

const json = spanToStreamedSpanJSON(span);
expect(json.status).toBe('ok');
expect(json.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]).toBeUndefined();
});

it('does not set a status message for error spans without a message', () => {
const span = new SentrySpan({ name: 'test name' });
span.setStatus({ code: SPAN_STATUS_ERROR });

const json = spanToStreamedSpanJSON(span);
expect(json.status).toBe('error');
expect(json.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]).toBeUndefined();
});

it('treats a cancelled status as ok and does not set a status message', () => {
const span = new SentrySpan({ name: 'test name' });
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'cancelled' });

const json = spanToStreamedSpanJSON(span);
expect(json.status).toBe('ok');
expect(json.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]).toBeUndefined();
});

it('does not overwrite an explicitly set sentry.status.message attribute', () => {
const span = new SentrySpan({
name: 'test name',
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]: 'explicit message' },
});
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'Connection Refused' });

const json = spanToStreamedSpanJSON(span);
expect(json.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]).toBe('explicit message');
});
});
describe('OpenTelemetry Span', () => {
it('converts a simple span', () => {
Expand Down Expand Up @@ -562,6 +609,7 @@ describe('spanToJSON', () => {
attr2: 2,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'test op',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto',
[SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]: 'unknown_error',
},
links: [
{
Expand All @@ -575,6 +623,38 @@ describe('spanToJSON', () => {
],
});
});

it('preserves a custom error status message as the sentry.status.message attribute', () => {
const span = createMockedOtelSpan({
spanId: 'SPAN-1',
traceId: 'TRACE-1',
name: 'test span',
startTime: 123,
endTime: 456,
attributes: {},
status: { code: SPAN_STATUS_ERROR, message: 'Connection Refused' },
});

const json = spanToStreamedSpanJSON(span);
expect(json.status).toBe('error');
expect(json.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]).toBe('Connection Refused');
});

it('does not set a status message for ok/unset spans', () => {
const span = createMockedOtelSpan({
spanId: 'SPAN-1',
traceId: 'TRACE-1',
name: 'test span',
startTime: 123,
endTime: 456,
attributes: {},
status: { code: SPAN_STATUS_UNSET },
});

const json = spanToStreamedSpanJSON(span);
expect(json.status).toBe('ok');
expect(json.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE]).toBeUndefined();
});
});
});

Expand Down
Loading