Skip to content

Commit 8fb1dc7

Browse files
authored
feat(browser): Always emit http.client streamed spans (#20420)
- When span streaming is enabled, always emit http.client spans even without an active parent span (like pageload/navigation) - Previously these spans were dropped with a no_parent_span client outcome in all cases - Applies to browser fetch instrumentation, browser XHR instrumentation and the OTel sampler - Non-streaming behavior unchanged: spans without a parent are still dropped and recorded as `no_parent_span` client outcomes closes #17932
1 parent 0bd937b commit 8fb1dc7

10 files changed

Lines changed: 101 additions & 46 deletions

File tree

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { expect } from '@playwright/test';
2-
import type { ClientReport } from '@sentry/core';
32
import { sentryTest } from '../../../utils/fixtures';
3+
import { getSpanOp, waitForStreamedSpan } from '../../../utils/spanUtils';
44
import {
55
envelopeRequestParser,
66
hidePage,
@@ -9,7 +9,7 @@ import {
99
} from '../../../utils/helpers';
1010

1111
sentryTest(
12-
'records no_parent_span client report for fetch requests without an active span',
12+
'sends http.client span for fetch requests without an active span when span streaming is enabled',
1313
async ({ getLocalTestUrl, page }) => {
1414
sentryTest.skip(shouldSkipTracingTest());
1515

@@ -23,22 +23,14 @@ sentryTest(
2323

2424
const url = await getLocalTestUrl({ testDir: __dirname });
2525

26-
const clientReportPromise = waitForClientReportRequest(page, report => {
27-
return report.discarded_events.some(e => e.reason === 'no_parent_span');
28-
});
26+
const spanPromise = waitForStreamedSpan(page, span => getSpanOp(span) === 'http.client');
2927

3028
await page.goto(url);
3129

32-
await hidePage(page);
33-
34-
const clientReport = envelopeRequestParser<ClientReport>(await clientReportPromise);
30+
const span = await spanPromise;
3531

36-
expect(clientReport.discarded_events).toEqual([
37-
{
38-
category: 'span',
39-
quantity: 1,
40-
reason: 'no_parent_span',
41-
},
42-
]);
32+
expect(span.name).toMatch(/^GET /);
33+
expect(span.attributes?.['sentry.origin']).toEqual({ type: 'string', value: 'auto.http.browser' });
34+
expect(span.attributes?.['sentry.op']).toEqual({ type: 'string', value: 'http.client' });
4335
},
4436
);
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import * as Sentry from '@sentry/node';
2+
fetch('http://localhost:9999/external').catch(async () => {
3+
await Sentry.flush();
4+
});

dev-packages/node-integration-tests/suites/tracing/no-parent-span-client-report-streamed/scenario.mjs

Lines changed: 0 additions & 2 deletions
This file was deleted.

dev-packages/node-integration-tests/suites/tracing/no-parent-span-client-report-streamed/test.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,24 @@
11
import { afterAll, describe, expect } from 'vitest';
22
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
33

4-
describe('no_parent_span client report (streaming)', () => {
4+
describe('no_parent_span with streaming enabled', () => {
55
afterAll(() => {
66
cleanupChildProcesses();
77
});
88

9-
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
10-
test('records no_parent_span outcome for http.client span without a local parent', async () => {
9+
createEsmAndCjsTests(__dirname, 'scenario-fetch.mjs', 'instrument.mjs', (createRunner, test) => {
10+
test('sends http.client span without a local parent when span streaming is enabled', async () => {
1111
const runner = createRunner()
12-
.unignore('client_report')
1312
.expect({
14-
client_report: report => {
15-
expect(report.discarded_events).toEqual([
16-
{
17-
category: 'span',
18-
quantity: 1,
19-
reason: 'no_parent_span',
20-
},
21-
]);
13+
span: span => {
14+
const httpClientSpan = span.items.find(item =>
15+
item.attributes?.['sentry.op']
16+
? item.attributes['sentry.op'].type === 'string' && item.attributes['sentry.op'].value === 'http.client'
17+
: false,
18+
);
19+
20+
expect(httpClientSpan).toBeDefined();
21+
expect(httpClientSpan?.name).toMatch(/^GET .*\/external$/);
2222
},
2323
})
2424
.start();

packages/browser/src/tracing/request.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -406,9 +406,11 @@ function xhrCallback(
406406

407407
const client = getClient();
408408
const hasParent = !!getActiveSpan();
409+
// With span streaming, we always emit http.client spans, even without a parent span
410+
const shouldEmitSpan = hasParent || (!!client && hasSpanStreamingEnabled(client));
409411

410412
const span =
411-
shouldCreateSpanResult && hasParent
413+
shouldCreateSpanResult && shouldEmitSpan
412414
? startInactiveSpan({
413415
name: `${method} ${urlForSpanName}`,
414416
attributes: {
@@ -425,7 +427,7 @@ function xhrCallback(
425427
})
426428
: new SentryNonRecordingSpan();
427429

428-
if (shouldCreateSpanResult && !hasParent) {
430+
if (shouldCreateSpanResult && !shouldEmitSpan) {
429431
client?.recordDroppedEvent('no_parent_span', 'span');
430432
}
431433

@@ -438,7 +440,7 @@ function xhrCallback(
438440
// If performance is disabled (TWP) or there's no active root span (pageload/navigation/interaction),
439441
// we do not want to use the span as base for the trace headers,
440442
// which means that the headers will be generated from the scope and the sampling decision is deferred
441-
hasSpansEnabled() && hasParent ? span : undefined,
443+
hasSpansEnabled() && shouldEmitSpan ? span : undefined,
442444
propagateTraceparent,
443445
);
444446
}

packages/core/src/fetch.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { getClient } from './currentScopes';
22
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from './semanticAttributes';
33
import { setHttpStatus, SPAN_STATUS_ERROR, startInactiveSpan } from './tracing';
44
import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan';
5+
import { hasSpanStreamingEnabled } from './tracing/spans/hasSpanStreamingEnabled';
56
import type { FetchBreadcrumbHint } from './types-hoist/breadcrumb';
67
import type { HandlerDataFetch } from './types-hoist/instrument';
78
import type { ResponseHookInfo } from './types-hoist/request';
@@ -110,13 +111,15 @@ export function instrumentFetchRequest(
110111

111112
const client = getClient();
112113
const hasParent = !!getActiveSpan();
114+
// With span streaming, we always emit http.client spans, even without a parent span
115+
const shouldEmitSpan = hasParent || (!!client && hasSpanStreamingEnabled(client));
113116

114117
const span =
115-
shouldCreateSpanResult && hasParent
118+
shouldCreateSpanResult && shouldEmitSpan
116119
? startInactiveSpan(getSpanStartOptions(url, method, spanOrigin))
117120
: new SentryNonRecordingSpan();
118121

119-
if (shouldCreateSpanResult && !hasParent) {
122+
if (shouldCreateSpanResult && !shouldEmitSpan) {
120123
client?.recordDroppedEvent('no_parent_span', 'span');
121124
}
122125

@@ -136,7 +139,7 @@ export function instrumentFetchRequest(
136139
// If performance is disabled (TWP) or there's no active root span (pageload/navigation/interaction),
137140
// we do not want to use the span as base for the trace headers,
138141
// which means that the headers will be generated from the scope and the sampling decision is deferred
139-
hasSpansEnabled() && hasParent ? span : undefined,
142+
hasSpansEnabled() && shouldEmitSpan ? span : undefined,
140143
propagateTraceparent,
141144
);
142145
if (headers) {

packages/core/src/tracing/spans/captureSpan.ts

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
} from '../../semanticAttributes';
1919
import type { SerializedStreamedSpan, Span, StreamedSpanJSON } from '../../types-hoist/span';
2020
import { getCombinedScopeData } from '../../utils/scopeData';
21+
import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '../../utils/url';
2122
import {
2223
INTERNAL_getSegmentSpan,
2324
showSpanDropWarning,
@@ -241,21 +242,55 @@ function inferHttpSpanData(
241242
return;
242243
}
243244

244-
// Only overwrite the span name when we have an explicit http.route — it's more specific than
245-
// what OTel instrumentation sets as the span name. For all other cases (url.full, http.target),
246-
// the OTel-set name is already good enough and we'd risk producing a worse name (e.g. full URL).
247245
const httpRoute = attributes['http.route'];
248246
if (typeof httpRoute === 'string') {
249247
spanJSON.name = `${httpMethod} ${httpRoute}`;
250248
safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' });
251249
} else {
252-
// Fallback: set source to 'url' for HTTP spans without a route.
253-
// The spec requires sentry.span.source on segment spans, and the non-streamed exporter
254-
// always sets this — so we need to ensure it's present for streamed spans too.
250+
// Infer span name from URL attributes, matching the non-streamed exporter's behavior.
251+
// Only overwrite the name for OTel spans (known spanKind)
252+
if (spanKind === SPAN_KIND_CLIENT || spanKind === SPAN_KIND_SERVER) {
253+
const urlPath = getUrlPath(attributes, spanKind);
254+
if (urlPath) {
255+
spanJSON.name = `${httpMethod} ${urlPath}`;
256+
}
257+
}
255258
safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' });
256259
}
257260
}
258261

262+
/**
263+
* Extract a URL path from span attributes for use in the span name.
264+
* Mirrors the logic in the non-streamed exporter's `getSanitizedUrl`.
265+
*/
266+
function getUrlPath(
267+
attributes: RawAttributes<Record<string, unknown>>,
268+
spanKind: number | undefined,
269+
): string | undefined {
270+
const httpUrl = attributes['http.url'] || attributes['url.full'];
271+
const httpTarget = attributes['http.target'];
272+
273+
const parsedUrl = typeof httpUrl === 'string' ? parseUrl(httpUrl) : undefined;
274+
const sanitizedUrl = parsedUrl ? getSanitizedUrlString(parsedUrl) : undefined;
275+
276+
// For server spans, prefer the relative target path
277+
if (spanKind === SPAN_KIND_SERVER && typeof httpTarget === 'string') {
278+
return stripUrlQueryAndFragment(httpTarget);
279+
}
280+
281+
// For client spans (and others), use the full sanitized URL
282+
if (sanitizedUrl) {
283+
return sanitizedUrl;
284+
}
285+
286+
// Fall back to target if no full URL is available
287+
if (typeof httpTarget === 'string') {
288+
return stripUrlQueryAndFragment(httpTarget);
289+
}
290+
291+
return undefined;
292+
}
293+
259294
function inferDbSpanData(spanJSON: StreamedSpanJSON, attributes: RawAttributes<Record<string, unknown>>): void {
260295
safeSetSpanJSONAttributes(spanJSON, { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'db' });
261296

packages/core/test/lib/tracing/spans/captureSpan.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -530,10 +530,10 @@ describe('inferSpanDataFromOtelAttributes', () => {
530530
expect(spanJSON.attributes?.['sentry.source']).toBe('route');
531531
});
532532

533-
it('does not overwrite name when no http.route but sets source to url', () => {
533+
it('infers name from url.full when no http.route and sets source to url', () => {
534534
const spanJSON = makeSpanJSON('GET', { 'http.request.method': 'GET', 'url.full': 'http://example.com/api' });
535535
inferSpanDataFromOtelAttributes(spanJSON, 2);
536-
expect(spanJSON.name).toBe('GET');
536+
expect(spanJSON.name).toBe('GET http://example.com/api');
537537
expect(spanJSON.attributes?.['sentry.source']).toBe('url');
538538
});
539539

packages/opentelemetry/src/sampler.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,13 @@ export class SentrySampler implements Sampler {
7575
const maybeSpanHttpMethod = spanAttributes[SEMATTRS_HTTP_METHOD] || spanAttributes[ATTR_HTTP_REQUEST_METHOD];
7676

7777
// If we have a http.client span that has no local parent, we never want to sample it
78-
// but we want to leave downstream sampling decisions up to the server
78+
// but we want to leave downstream sampling decisions up to the server.
79+
// Exception: when span streaming is enabled, we always emit these spans.
7980
if (spanKind === SpanKind.CLIENT && maybeSpanHttpMethod && (!parentSpan || parentContext?.isRemote)) {
80-
this._client.recordDroppedEvent('no_parent_span', 'span');
81-
return wrapSamplingDecision({ decision: undefined, context, spanAttributes });
81+
if (!this._isSpanStreaming) {
82+
this._client.recordDroppedEvent('no_parent_span', 'span');
83+
return wrapSamplingDecision({ decision: undefined, context, spanAttributes });
84+
}
8285
}
8386

8487
const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined;

packages/opentelemetry/test/sampler.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,5 +348,23 @@ describe('SentrySampler', () => {
348348
expect(spyOnDroppedEvent).toHaveBeenCalledTimes(1);
349349
expect(spyOnDroppedEvent).toHaveBeenCalledWith('sample_rate', 'span');
350350
});
351+
352+
it('always emits streamed http.client spans without a local parent', () => {
353+
const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1, traceLifecycle: 'stream' }));
354+
const spyOnDroppedEvent = vi.spyOn(client, 'recordDroppedEvent');
355+
const sampler = new SentrySampler(client);
356+
357+
const ctx = context.active();
358+
const traceId = generateTraceId();
359+
const spanName = 'GET http://example.com/api';
360+
const spanKind = SpanKind.CLIENT;
361+
const spanAttributes = {
362+
[ATTR_HTTP_REQUEST_METHOD]: 'GET',
363+
};
364+
365+
const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, undefined);
366+
expect(actual.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED);
367+
expect(spyOnDroppedEvent).not.toHaveBeenCalled();
368+
});
351369
});
352370
});

0 commit comments

Comments
 (0)