-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(core): Backfill otel attributes on streamed spans #20439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
a8b6c13
f23e7d1
49564b3
f994eb1
d602203
623ad14
373ee88
ccc0c2c
c2e1043
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
|
|
||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| release: '1.0', | ||
| tracesSampleRate: 1.0, | ||
| traceLifecycle: 'stream', | ||
| transport: loggingTransport, | ||
| }); | ||
|
|
||
| async function run(): Promise<void> { | ||
| await Sentry.startSpan({ name: 'test_transaction' }, async () => { | ||
| await fetch(`${process.env.SERVER_URL}/api/v0`); | ||
| }); | ||
|
|
||
| await Sentry.flush(); | ||
| } | ||
|
|
||
| void run(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { createTestServer } from '@sentry-internal/test-utils'; | ||
| import { expect, test } from 'vitest'; | ||
| import { createRunner } from '../../../../utils/runner'; | ||
|
|
||
| test('infers sentry.op for streamed outgoing fetch spans', async () => { | ||
| expect.assertions(2); | ||
|
|
||
| const [SERVER_URL, closeTestServer] = await createTestServer() | ||
| .get('/api/v0', () => { | ||
| expect(true).toBe(true); | ||
| }) | ||
| .start(); | ||
|
|
||
| await createRunner(__dirname, 'scenario.ts') | ||
| .withEnv({ SERVER_URL }) | ||
| .expect({ | ||
| span: container => { | ||
| const httpClientSpan = container.items.find( | ||
| item => | ||
| item.attributes?.['sentry.op']?.type === 'string' && item.attributes['sentry.op'].value === 'http.client', | ||
| ); | ||
|
|
||
| expect(httpClientSpan).toBeDefined(); | ||
| }, | ||
| }) | ||
| .start() | ||
| .completed(); | ||
| closeTestServer(); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
|
|
||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| release: '1.0', | ||
| tracesSampleRate: 1.0, | ||
| transport: loggingTransport, | ||
| traceLifecycle: 'stream', | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; | ||
| import cors from 'cors'; | ||
| import express from 'express'; | ||
|
|
||
| const app = express(); | ||
|
|
||
| app.use(cors()); | ||
|
|
||
| app.get('/test', (_req, res) => { | ||
| res.send({ response: 'ok' }); | ||
| }); | ||
|
|
||
| Sentry.setupExpressErrorHandler(app); | ||
|
|
||
| startExpressServerAndSendPortToRunner(app); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| import { afterAll, describe, expect } from 'vitest'; | ||
| import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; | ||
|
|
||
| describe('httpIntegration-streamed', () => { | ||
| afterAll(() => { | ||
| cleanupChildProcesses(); | ||
| }); | ||
|
|
||
| createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument.mjs', (createRunner, test) => { | ||
| test('infers sentry.op, name, and source for streamed server spans', async () => { | ||
| const runner = createRunner() | ||
| .expect({ | ||
| span: container => { | ||
| const serverSpan = container.items.find( | ||
| item => | ||
| item.attributes?.['sentry.op']?.type === 'string' && | ||
| item.attributes['sentry.op'].value === 'http.server', | ||
| ); | ||
|
|
||
| expect(serverSpan).toBeDefined(); | ||
| expect(serverSpan?.is_segment).toBe(true); | ||
| expect(serverSpan?.name).toBe('GET /test'); | ||
| expect(serverSpan?.attributes?.['sentry.source']).toEqual({ type: 'string', value: 'route' }); | ||
| expect(serverSpan?.attributes?.['sentry.span.source']).toEqual({ type: 'string', value: 'route' }); | ||
| }, | ||
| }) | ||
| .start(); | ||
|
|
||
| await runner.makeRequest('get', '/test'); | ||
|
|
||
| await runner.completed(); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,9 @@ import type { RawAttributes } from '../../attributes'; | |
| import type { Client } from '../../client'; | ||
| import type { ScopeData } from '../../scope'; | ||
| import { | ||
| SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_OP, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_RELEASE, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_SDK_NAME, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_SDK_VERSION, | ||
|
|
@@ -58,6 +60,14 @@ export function captureSpan(span: Span, client: Client): SerializedStreamedSpanW | |
| client.emit('processSegmentSpan', spanJSON); | ||
| } | ||
|
|
||
| // Backfill span data from OTel semantic conventions when not explicitly set. | ||
| // OTel-originated spans don't have sentry.op, description, etc. — the non-streamed path | ||
| // infers these in the SentrySpanExporter, but streamed spans skip the exporter entirely. | ||
| // Access `kind` via duck-typing — OTel span objects have this property but it's not on Sentry's Span type. | ||
| // This must run before hooks and beforeSendSpan so that user callbacks can see and override inferred values. | ||
| const spanKind = (span as { kind?: number }).kind; | ||
| inferSpanDataFromOtelAttributes(spanJSON, spanKind); | ||
|
|
||
| // This allows hook subscribers to mutate the span JSON | ||
| // This also invokes the `processSpan` hook of all integrations | ||
| client.emit('processSpan', spanJSON); | ||
|
|
@@ -150,3 +160,105 @@ export function safeSetSpanJSONAttributes( | |
| } | ||
| }); | ||
| } | ||
|
|
||
| // OTel SpanKind values (numeric to avoid importing from @opentelemetry/api) | ||
| const SPAN_KIND_SERVER = 1; | ||
| const SPAN_KIND_CLIENT = 2; | ||
|
|
||
| /** | ||
| * Infer and backfill span data from OTel semantic conventions. | ||
| * This mirrors what the `SentrySpanExporter` does for non-streamed spans via `getSpanData`/`inferSpanData`. | ||
| * Streamed spans skip the exporter, so we do the inference here during capture. | ||
| * | ||
| * Backfills: `sentry.op`, `sentry.source`, and `name` (description). | ||
| * Uses `safeSetSpanJSONAttributes` so explicitly set attributes are never overwritten. | ||
| */ | ||
| function inferSpanDataFromOtelAttributes(spanJSON: StreamedSpanJSON, spanKind?: number): void { | ||
| const attributes = spanJSON.attributes; | ||
| if (!attributes) { | ||
| return; | ||
| } | ||
|
|
||
| const httpMethod = attributes['http.request.method'] || attributes['http.method']; | ||
| if (httpMethod) { | ||
| inferHttpSpanData(spanJSON, attributes, spanKind, httpMethod); | ||
| return; | ||
| } | ||
|
|
||
| const dbSystem = attributes['db.system.name'] || attributes['db.system']; | ||
| const opIsCache = | ||
| typeof attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] === 'string' && | ||
| `${attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP]}`.startsWith('cache.'); | ||
| if (dbSystem && !opIsCache) { | ||
| inferDbSpanData(spanJSON, attributes); | ||
| return; | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| if (attributes['rpc.service']) { | ||
| safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'rpc' }); | ||
| return; | ||
| } | ||
|
|
||
| if (attributes['messaging.system']) { | ||
| safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'message' }); | ||
| return; | ||
| } | ||
|
|
||
| const faasTrigger = attributes['faas.trigger']; | ||
| if (faasTrigger) { | ||
| safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${faasTrigger}` }); | ||
| } | ||
| } | ||
|
|
||
| function inferHttpSpanData( | ||
| spanJSON: StreamedSpanJSON, | ||
| attributes: RawAttributes<Record<string, unknown>>, | ||
| spanKind: number | undefined, | ||
| httpMethod: unknown, | ||
| ): void { | ||
| // Infer op: http.client, http.server, or just http | ||
| const opParts = ['http']; | ||
| if (spanKind === SPAN_KIND_CLIENT) { | ||
| opParts.push('client'); | ||
| } else if (spanKind === SPAN_KIND_SERVER) { | ||
| opParts.push('server'); | ||
| } | ||
| if (attributes['sentry.http.prefetch']) { | ||
| opParts.push('prefetch'); | ||
| } | ||
| safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: opParts.join('.') }); | ||
|
|
||
| // If the user already set a custom name or source, don't overwrite | ||
| if ( | ||
| attributes[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME] || | ||
| attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom' | ||
| ) { | ||
| return; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Custom span names ignored in streaming pathHigh Severity When Additional Locations (1)Reviewed by Cursor Bugbot for commit c2e1043. Configure here. |
||
|
|
||
| // Only overwrite the span name when we have an explicit http.route — it's more specific than | ||
| // what OTel instrumentation sets as the span name. For all other cases (url.full, http.target), | ||
| // the OTel-set name is already good enough and we'd risk producing a worse name (e.g. full URL). | ||
| const httpRoute = attributes['http.route']; | ||
| if (typeof httpRoute === 'string') { | ||
| spanJSON.name = `${httpMethod} ${httpRoute}`; | ||
| safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' }); | ||
| } | ||
| } | ||
|
|
||
| function inferDbSpanData(spanJSON: StreamedSpanJSON, attributes: RawAttributes<Record<string, unknown>>): void { | ||
| safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'db' }); | ||
|
|
||
| if ( | ||
| attributes[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME] || | ||
| attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom' | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const statement = attributes['db.statement']; | ||
| if (statement) { | ||
| spanJSON.name = `${statement}`; | ||
|
sentry[bot] marked this conversation as resolved.
|
||
| safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task' }); | ||
| } | ||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Segment name attribute stale after name inference
Medium Severity
applyCommonSpanAttributessetssentry.segment.namefromserializedSegmentSpan.name(the original OTel name) at line 116 beforeinferSpanDataFromOtelAttributesruns. When inference later updatesspanJSON.name(e.g., from"GET"to"GET /test"), thesentry.segment.nameattribute remains stale becausesafeSetSpanJSONAttributeswon't overwrite existing keys. For segment spans, this creates a mismatch betweennameandsentry.segment.name.Additional Locations (1)
packages/core/src/tracing/spans/captureSpan.ts#L115-L116Reviewed by Cursor Bugbot for commit c2e1043. Configure here.