Skip to content

Commit be13537

Browse files
Lms24s1gr1d
andauthored
feat(core): Emit no_parent_span client outcomes for discarded spans requiring a parent (#20350)
This PR adds reporting a client discard outcome for spans we skip because they require a parent span to be active. This happens when `onlyIfParent: true` is set when starting spans, or in custom logic for `http.client` spans. With this PR, we should now get a much clearer picture of how many spans users are missing out on. In span streaming, we'll always emit them (see #17932) spans. closes https://linear.app/getsentry/issue/SDK-1123/add-client-report-outcome-for-dropped-spans-because-of-missing-parent --------- Co-authored-by: s1gr1d <32902192+s1gr1d@users.noreply.github.com>
1 parent d37d6e4 commit be13537

22 files changed

Lines changed: 405 additions & 25 deletions

File tree

.size-limit.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ module.exports = [
203203
name: 'CDN Bundle (incl. Tracing, Logs, Metrics)',
204204
path: createCDNPath('bundle.tracing.logs.metrics.min.js'),
205205
gzip: true,
206-
limit: '46 KB',
206+
limit: '47 KB',
207207
},
208208
{
209209
name: 'CDN Bundle (incl. Replay, Logs, Metrics)',
@@ -324,7 +324,7 @@ module.exports = [
324324
import: createImport('init'),
325325
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
326326
gzip: true,
327-
limit: '59 KB',
327+
limit: '60 KB',
328328
},
329329
// Node SDK (ESM)
330330
{
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
integrations: [
8+
Sentry.browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false }),
9+
Sentry.spanStreamingIntegration(),
10+
],
11+
tracesSampleRate: 1,
12+
sendClientReports: true,
13+
});
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fetch('http://sentry-test-site.example/api/test');
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { expect } from '@playwright/test';
2+
import type { ClientReport } from '@sentry/core';
3+
import { sentryTest } from '../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
hidePage,
7+
shouldSkipTracingTest,
8+
testingCdnBundle,
9+
waitForClientReportRequest,
10+
} from '../../../utils/helpers';
11+
12+
sentryTest(
13+
'records no_parent_span client report for fetch requests without an active span',
14+
async ({ getLocalTestUrl, page }) => {
15+
sentryTest.skip(shouldSkipTracingTest() || testingCdnBundle());
16+
17+
await page.route('http://sentry-test-site.example/api/test', route => {
18+
route.fulfill({
19+
status: 200,
20+
body: 'ok',
21+
headers: { 'Content-Type': 'text/plain' },
22+
});
23+
});
24+
25+
const url = await getLocalTestUrl({ testDir: __dirname });
26+
27+
const clientReportPromise = waitForClientReportRequest(page, report => {
28+
return report.discarded_events.some(e => e.reason === 'no_parent_span');
29+
});
30+
31+
await page.goto(url);
32+
33+
await hidePage(page);
34+
35+
const clientReport = envelopeRequestParser<ClientReport>(await clientReportPromise);
36+
37+
expect(clientReport.discarded_events).toEqual([
38+
{
39+
category: 'span',
40+
quantity: 1,
41+
reason: 'no_parent_span',
42+
},
43+
]);
44+
},
45+
);
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
integrations: [Sentry.browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false })],
8+
tracesSampleRate: 1,
9+
sendClientReports: true,
10+
});
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fetch('http://sentry-test-site.example/api/test');
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { expect } from '@playwright/test';
2+
import type { ClientReport } from '@sentry/core';
3+
import { sentryTest } from '../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
hidePage,
7+
shouldSkipTracingTest,
8+
testingCdnBundle,
9+
waitForClientReportRequest,
10+
} from '../../../utils/helpers';
11+
12+
sentryTest(
13+
'records no_parent_span client report for fetch requests without an active span',
14+
async ({ getLocalTestUrl, page }) => {
15+
sentryTest.skip(shouldSkipTracingTest() || testingCdnBundle());
16+
17+
await page.route('http://sentry-test-site.example/api/test', route => {
18+
route.fulfill({
19+
status: 200,
20+
body: 'ok',
21+
headers: { 'Content-Type': 'text/plain' },
22+
});
23+
});
24+
25+
const url = await getLocalTestUrl({ testDir: __dirname });
26+
27+
const clientReportPromise = waitForClientReportRequest(page, report => {
28+
return report.discarded_events.some(e => e.reason === 'no_parent_span');
29+
});
30+
31+
await page.goto(url);
32+
33+
await hidePage(page);
34+
35+
const clientReport = envelopeRequestParser<ClientReport>(await clientReportPromise);
36+
37+
expect(clientReport.discarded_events).toEqual([
38+
{
39+
category: 'span',
40+
quantity: 1,
41+
reason: 'no_parent_span',
42+
},
43+
]);
44+
},
45+
);
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
traceLifecycle: 'stream',
10+
clientReportFlushInterval: 1_000,
11+
});
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import http from 'http';
2+
http.get('http://localhost:9999/external', () => {}).on('error', () => {});
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { afterAll, describe, expect } from 'vitest';
2+
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
3+
4+
describe('no_parent_span client report (streaming)', () => {
5+
afterAll(() => {
6+
cleanupChildProcesses();
7+
});
8+
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 () => {
11+
const runner = createRunner()
12+
.unignore('client_report')
13+
.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+
]);
22+
},
23+
})
24+
.start();
25+
26+
await runner.completed();
27+
});
28+
});
29+
});

0 commit comments

Comments
 (0)