Skip to content

Commit 1bc5f72

Browse files
committed
handle client reports for core/browser
1 parent c5da976 commit 1bc5f72

9 files changed

Lines changed: 269 additions & 92 deletions

File tree

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces-streamed/consistent-sampling/meta-negative/test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,14 @@ sentryTest.describe('When `consistentTraceSampling` is `true` and page contains
8383
timestamp: expect.any(Number),
8484
discarded_events: [
8585
{
86-
category: 'transaction',
87-
quantity: 4,
86+
category: 'span',
87+
quantity: expect.any(Number),
8888
reason: 'sample_rate',
8989
},
9090
],
9191
});
92+
// exact number depends on performance observer emissions
93+
expect(clientReport.discarded_events[0].quantity).toBeGreaterThanOrEqual(10);
9294
});
9395

9496
expect(spansReceived).toHaveLength(0);

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces-streamed/consistent-sampling/meta-precedence/test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,14 @@ sentryTest.describe('When `consistentTraceSampling` is `true` and page contains
6969
timestamp: expect.any(Number),
7070
discarded_events: [
7171
{
72-
category: 'transaction',
73-
quantity: 2,
72+
category: 'span',
73+
quantity: expect.any(Number),
7474
reason: 'sample_rate',
7575
},
7676
],
7777
});
78+
// exact number depends on performance observer emissions
79+
expect(clientReport.discarded_events[0].quantity).toBeGreaterThanOrEqual(3);
7880
});
7981

8082
await sentryTest.step('Navigate to another page with meta tags', async () => {

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces-streamed/consistent-sampling/tracesSampler-precedence/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ sentryTest.describe('When `consistentTraceSampling` is `true`', () => {
5353
timestamp: expect.any(Number),
5454
discarded_events: [
5555
{
56-
category: 'transaction',
56+
category: 'span',
5757
quantity: 1,
5858
reason: 'sample_rate',
5959
},
@@ -76,7 +76,7 @@ sentryTest.describe('When `consistentTraceSampling` is `true`', () => {
7676
timestamp: expect.any(Number),
7777
discarded_events: [
7878
{
79-
category: 'transaction',
79+
category: 'span',
8080
quantity: 1,
8181
reason: 'sample_rate',
8282
},

dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/child/init.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,6 @@ Sentry.init({
66
dsn: 'https://public@dsn.ingest.sentry.io/1337',
77
integrations: [Sentry.spanStreamingIntegration()],
88
ignoreSpans: [/ignore/],
9+
parentSpanIsAlwaysRootSpan: false,
910
tracesSampleRate: 1,
1011
});
Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,23 @@
11
Sentry.startSpan({ name: 'parent-span' }, () => {
22
Sentry.startSpan({ name: 'keep-me' }, () => {});
33

4-
// This child matches ignoreSpans — should be dropped
4+
// This child matches ignoreSpans —> dropped
55
Sentry.startSpan({ name: 'ignore-child' }, () => {
6-
// Grandchild should be reparented to 'parent-span'
7-
Sentry.startSpan({ name: 'grandchild-1' }, () => {});
6+
// dropped
7+
Sentry.startSpan({ name: 'ignore-grandchild-1' }, () => {
8+
// kept
9+
Sentry.startSpan({ name: 'great-grandchild-1' }, () => {
10+
// dropped
11+
Sentry.startSpan({ name: 'ignore-great-great-grandchild-1' }, () => {
12+
// kept
13+
Sentry.startSpan({ name: 'great-great-great-grandchild-1' }, () => {});
14+
});
15+
});
16+
});
17+
// Grandchild is reparented to 'parent-span' —> kept
818
Sentry.startSpan({ name: 'grandchild-2' }, () => {});
919
});
1020

21+
// kept
1122
Sentry.startSpan({ name: 'another-keeper' }, () => {});
1223
});

dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/child/test.ts

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,47 +9,52 @@ import {
99
import { waitForStreamedSpans } from '../../../../utils/spanUtils';
1010
import type { ClientReport } from '@sentry/core';
1111

12-
sentryTest(
13-
'ignored child spans are dropped and their children are reparented to the grandparent',
14-
async ({ getLocalTestUrl, page }) => {
15-
if (shouldSkipTracingTest()) {
16-
sentryTest.skip();
17-
}
12+
sentryTest('ignored child spans are dropped and their children are reparented', async ({ getLocalTestUrl, page }) => {
13+
if (shouldSkipTracingTest()) {
14+
sentryTest.skip();
15+
}
1816

19-
const spansPromise = waitForStreamedSpans(page, spans => !!spans?.find(s => s.name === 'parent-span'));
17+
const spansPromise = waitForStreamedSpans(page, spans => !!spans?.find(s => s.name === 'parent-span'));
2018

21-
const clientReportPromise = waitForClientReportRequest(page);
19+
const clientReportPromise = waitForClientReportRequest(page);
2220

23-
const url = await getLocalTestUrl({ testDir: __dirname });
24-
await page.goto(url);
21+
const url = await getLocalTestUrl({ testDir: __dirname });
22+
await page.goto(url);
2523

26-
const spans = await spansPromise;
24+
const spans = await spansPromise;
2725

28-
await hidePage(page);
26+
await hidePage(page);
2927

30-
const clientReport = envelopeRequestParser<ClientReport>(await clientReportPromise);
28+
const clientReport = envelopeRequestParser<ClientReport>(await clientReportPromise);
3129

32-
const segmentSpanId = spans.find(s => s.name === 'parent-span')?.span_id;
30+
const segmentSpanId = spans.find(s => s.name === 'parent-span')?.span_id;
3331

34-
expect(spans.some(s => s.name === 'keep-me')).toBe(true);
35-
expect(spans.some(s => s.name === 'another-keeper')).toBe(true);
32+
expect(spans.length).toBe(6);
3633

37-
expect(spans.some(s => s.name?.includes('ignore'))).toBe(false);
34+
expect(spans.some(s => s.name === 'keep-me')).toBe(true);
35+
expect(spans.some(s => s.name === 'another-keeper')).toBe(true);
3836

39-
const grandchild1 = spans.find(s => s.name === 'grandchild-1');
40-
const grandchild2 = spans.find(s => s.name === 'grandchild-2');
41-
expect(grandchild1).toBeDefined();
42-
expect(grandchild2).toBeDefined();
37+
expect(spans.some(s => s.name?.includes('ignore'))).toBe(false);
4338

44-
expect(grandchild1?.parent_span_id).toBe(segmentSpanId);
45-
expect(grandchild2?.parent_span_id).toBe(segmentSpanId);
39+
const greatGrandChild1 = spans.find(s => s.name === 'great-grandchild-1');
40+
const grandchild2 = spans.find(s => s.name === 'grandchild-2');
41+
const greatGreatGreatGrandChild1 = spans.find(s => s.name === 'great-great-great-grandchild-1');
4642

47-
expect(clientReport.discarded_events).toEqual([
48-
{
49-
category: 'span',
50-
quantity: 1,
51-
reason: 'ignored',
52-
},
53-
]);
54-
},
55-
);
43+
expect(greatGrandChild1).toBeDefined();
44+
expect(grandchild2).toBeDefined();
45+
expect(greatGreatGreatGrandChild1).toBeDefined();
46+
47+
expect(greatGrandChild1?.parent_span_id).toBe(segmentSpanId);
48+
expect(grandchild2?.parent_span_id).toBe(segmentSpanId);
49+
expect(greatGreatGreatGrandChild1?.parent_span_id).toBe(greatGrandChild1?.span_id);
50+
51+
expect(spans.every(s => s.name === 'parent-span' || !s.is_segment)).toBe(true);
52+
53+
expect(clientReport.discarded_events).toEqual([
54+
{
55+
category: 'span',
56+
quantity: 3, // 3 ignored child spans
57+
reason: 'ignored',
58+
},
59+
]);
60+
});

packages/core/src/tracing/sentryNonRecordingSpan.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { EventDropReason } from '../types-hoist/clientreport';
12
import type {
23
SentrySpanArguments,
34
Span,
@@ -10,16 +11,28 @@ import type { SpanStatus } from '../types-hoist/spanStatus';
1011
import { generateSpanId, generateTraceId } from '../utils/propagationContext';
1112
import { TRACE_FLAG_NONE } from '../utils/spanUtils';
1213

14+
interface SentryNonRecordingSpanArguments extends SentrySpanArguments {
15+
dropReason?: EventDropReason;
16+
}
17+
1318
/**
1419
* A Sentry Span that is non-recording, meaning it will not be sent to Sentry.
1520
*/
1621
export class SentryNonRecordingSpan implements Span {
1722
private _traceId: string;
1823
private _spanId: string;
1924

20-
public constructor(spanContext: SentrySpanArguments = {}) {
25+
/**
26+
* Reason why this span was dropped, if applicable ('ignored' or 'sample_rate').
27+
* Used to propagate the correct client report outcome to descendant spans
28+
* when span streaming is enabled.
29+
*/
30+
public dropReason?: EventDropReason;
31+
32+
public constructor(spanContext: SentryNonRecordingSpanArguments = {}) {
2133
this._traceId = spanContext.traceId || generateTraceId();
2234
this._spanId = spanContext.spanId || generateSpanId();
35+
this.dropReason = spanContext.dropReason;
2336
}
2437

2538
/** @inheritdoc */

packages/core/src/tracing/trace.ts

Lines changed: 71 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { debug } from '../utils/debug-logger';
2020
import { handleCallbackErrors } from '../utils/handleCallbackErrors';
2121
import { hasSpansEnabled } from '../utils/hasSpansEnabled';
2222
import { shouldIgnoreSpan } from '../utils/should-ignore-span';
23+
import { hasSpanStreamingEnabled } from './spans/hasSpanStreamingEnabled';
2324
import { parseSampleRate } from '../utils/parseSampleRate';
2425
import { generateTraceId } from '../utils/propagationContext';
2526
import { safeMathRandom } from '../utils/randomSafeContext';
@@ -33,6 +34,7 @@ import { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
3334
import { SentrySpan } from './sentrySpan';
3435
import { SPAN_STATUS_ERROR } from './spanstatus';
3536
import { setCapturedScopesOnSpan } from './utils';
37+
import type { Client } from '../client';
3638

3739
const SUPPRESS_TRACING_KEY = '__SENTRY_SUPPRESS_TRACING__';
3840

@@ -68,22 +70,12 @@ export function startSpan<T>(options: StartSpanOptions, callback: (span: Span) =
6870
const parentSpan = getParentSpan(scope, customParentSpan);
6971

7072
const client = getClient();
71-
const ignoreSpans = client?.getOptions().ignoreSpans;
72-
if (ignoreSpans?.length) {
73-
const op = spanArguments.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_OP];
74-
if (shouldIgnoreSpan({ description: spanArguments.name || '', op }, ignoreSpans)) {
75-
client?.recordDroppedEvent('ignored', 'span');
76-
const nonRecordingSpan = new SentryNonRecordingSpan();
77-
// For root spans, set on scope (like unsampled). For child spans, don't — keep parent active.
78-
if (!parentSpan) {
79-
_setSpanForScope(scope, nonRecordingSpan);
80-
}
81-
return handleCallbackErrors(
82-
() => callback(nonRecordingSpan),
83-
() => {},
84-
() => nonRecordingSpan.end(),
85-
);
86-
}
73+
if (_shouldIgnoreStreamedSpan(client, spanArguments)) {
74+
return handleCallbackErrors(
75+
() => callback(_createIgnoredSpan(client, parentSpan, scope)),
76+
() => {},
77+
() => {},
78+
);
8779
}
8880

8981
const shouldSkipSpan = options.onlyIfParent && !parentSpan;
@@ -145,20 +137,11 @@ export function startSpanManual<T>(options: StartSpanOptions, callback: (span: S
145137
const parentSpan = getParentSpan(scope, customParentSpan);
146138

147139
const client = getClient();
148-
const ignoreSpans = client?.getOptions().ignoreSpans;
149-
if (ignoreSpans?.length) {
150-
const op = spanArguments.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_OP];
151-
if (shouldIgnoreSpan({ description: spanArguments.name || '', op }, ignoreSpans)) {
152-
client?.recordDroppedEvent('ignored', 'span');
153-
const nonRecordingSpan = new SentryNonRecordingSpan();
154-
if (!parentSpan) {
155-
_setSpanForScope(scope, nonRecordingSpan);
156-
}
157-
return handleCallbackErrors(
158-
() => callback(nonRecordingSpan, () => nonRecordingSpan.end()),
159-
() => {},
160-
);
161-
}
140+
if (_shouldIgnoreStreamedSpan(client, spanArguments)) {
141+
return handleCallbackErrors(
142+
() => callback(_createIgnoredSpan(client, parentSpan, scope), () => {}),
143+
() => {},
144+
);
162145
}
163146

164147
const shouldSkipSpan = options.onlyIfParent && !parentSpan;
@@ -222,13 +205,8 @@ export function startInactiveSpan(options: StartSpanOptions): Span {
222205
const parentSpan = getParentSpan(scope, customParentSpan);
223206

224207
const client = getClient();
225-
const ignoreSpans = client?.getOptions().ignoreSpans;
226-
if (ignoreSpans?.length) {
227-
const op = spanArguments.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_OP];
228-
if (shouldIgnoreSpan({ description: spanArguments.name || '', op }, ignoreSpans)) {
229-
client?.recordDroppedEvent('ignored', 'span');
230-
return new SentryNonRecordingSpan();
231-
}
208+
if (_shouldIgnoreStreamedSpan(client, spanArguments)) {
209+
return _createIgnoredSpan(client, parentSpan, scope);
232210
}
233211

234212
const shouldSkipSpan = options.onlyIfParent && !parentSpan;
@@ -513,7 +491,7 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent
513491

514492
if (!sampled && client) {
515493
DEBUG_BUILD && debug.log('[Tracing] Discarding root span because its trace was not chosen to be sampled.');
516-
client.recordDroppedEvent('sample_rate', 'transaction');
494+
client.recordDroppedEvent('sample_rate', hasSpanStreamingEnabled(client) ? 'span' : 'transaction');
517495
}
518496

519497
if (client) {
@@ -543,15 +521,33 @@ function _startChildSpan(parentSpan: Span, scope: Scope, spanArguments: SentrySp
543521
addChildSpanToSpan(parentSpan, childSpan);
544522

545523
const client = getClient();
546-
if (client) {
547-
client.emit('spanStart', childSpan);
548-
// If it has an endTimestamp, it's already ended
549-
if (spanArguments.endTimestamp) {
550-
client.emit('spanEnd', childSpan);
551-
client.emit('afterSpanEnd', childSpan);
524+
525+
if (!client) {
526+
return childSpan;
527+
}
528+
529+
if (hasSpanStreamingEnabled(client) && childSpan instanceof SentryNonRecordingSpan) {
530+
if (parentSpan instanceof SentryNonRecordingSpan && parentSpan.dropReason) {
531+
// We land here if the parent span was a segment span that was ignored (`ignoreSpans`).
532+
// In this case, the child was also ignored (see `sampled` above) but we need to
533+
// record a client outcome for the child.
534+
childSpan.dropReason = parentSpan.dropReason;
535+
client.recordDroppedEvent(parentSpan.dropReason, 'span');
536+
} else {
537+
// Otherwise, the child is not sampled due to sampling of the parent span,
538+
// hence we record a sample_rate client outcome for the child.
539+
childSpan.dropReason = 'sample_rate';
540+
client.recordDroppedEvent('sample_rate', 'span');
552541
}
553542
}
554543

544+
client.emit('spanStart', childSpan);
545+
// If it has an endTimestamp, it's already ended
546+
if (spanArguments.endTimestamp) {
547+
client.emit('spanEnd', childSpan);
548+
client.emit('afterSpanEnd', childSpan);
549+
}
550+
555551
return childSpan;
556552
}
557553

@@ -588,3 +584,34 @@ function getActiveSpanWrapper<T>(parentSpan: Span | undefined | null): (callback
588584
}
589585
: (callback: () => T) => callback();
590586
}
587+
588+
/* Checks if `ignoreSpans` applies (extracted for bundle size)*/
589+
function _shouldIgnoreStreamedSpan(client: Client | undefined, spanArguments: SentrySpanArguments): boolean {
590+
const ignoreSpans = client?.getOptions().ignoreSpans;
591+
592+
if (!client || !hasSpanStreamingEnabled(client) || !ignoreSpans?.length) {
593+
return false;
594+
}
595+
596+
return shouldIgnoreSpan(
597+
{ description: spanArguments.name || '', op: spanArguments.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_OP] },
598+
ignoreSpans,
599+
);
600+
}
601+
602+
/* creates a non-recording span that is marked as ignored and sets it on the scope if applicable */
603+
function _createIgnoredSpan(
604+
client: Client | undefined,
605+
parentSpan: SentrySpan | undefined,
606+
scope: Scope,
607+
): SentryNonRecordingSpan {
608+
client?.recordDroppedEvent('ignored', 'span');
609+
const nonRecordingSpan = new SentryNonRecordingSpan({ dropReason: 'ignored' });
610+
if (!parentSpan) {
611+
// Put the ignored non-recording segment span onto the scope so that `getActiveSpan()` returns it
612+
// For child spans, we don't do this because there _is_ an active span on the scope. We can change
613+
// this if necessary.
614+
_setSpanForScope(scope, nonRecordingSpan);
615+
}
616+
return nonRecordingSpan;
617+
}

0 commit comments

Comments
 (0)