Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
6 changes: 3 additions & 3 deletions .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ module.exports = [
path: createCDNPath('bundle.tracing.replay.min.js'),
gzip: false,
brotli: false,
limit: '251 KB',
limit: '252 KB',
},
{
name: 'CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed',
Expand All @@ -290,7 +290,7 @@ module.exports = [
path: createCDNPath('bundle.tracing.replay.feedback.min.js'),
gzip: false,
brotli: false,
limit: '264 KB',
limit: '265 KB',
},
{
name: 'CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed',
Expand Down Expand Up @@ -324,7 +324,7 @@ module.exports = [
import: createImport('init'),
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
gzip: true,
limit: '59 KB',
limit: '60 KB',
},
// Node SDK (ESM)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
integrations: [
Sentry.browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false }),
Sentry.spanStreamingIntegration(),
],
tracesSampleRate: 1,
sendClientReports: true,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fetch('http://sentry-test-site.example/api/test');
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { expect } from '@playwright/test';
import type { ClientReport } from '@sentry/core';
import { sentryTest } from '../../../utils/fixtures';
import {
envelopeRequestParser,
hidePage,
shouldSkipTracingTest,
testingCdnBundle,
waitForClientReportRequest,
} from '../../../utils/helpers';

sentryTest(
'records no_parent_span client report for fetch requests without an active span',
async ({ getLocalTestUrl, page }) => {
sentryTest.skip(shouldSkipTracingTest() || testingCdnBundle());

await page.route('http://sentry-test-site.example/api/test', route => {
route.fulfill({
status: 200,
body: 'ok',
headers: { 'Content-Type': 'text/plain' },
});
});

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

const clientReportPromise = waitForClientReportRequest(page, report => {
return report.discarded_events.some(e => e.reason === 'no_parent_span');
});

await page.goto(url);

await hidePage(page);

const clientReport = envelopeRequestParser<ClientReport>(await clientReportPromise);

expect(clientReport.discarded_events).toEqual([
{
category: 'span',
quantity: 1,
reason: 'no_parent_span',
},
]);
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
integrations: [Sentry.browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false })],
tracesSampleRate: 1,
sendClientReports: true,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fetch('http://sentry-test-site.example/api/test');
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { expect } from '@playwright/test';
import type { ClientReport } from '@sentry/core';
import { sentryTest } from '../../../utils/fixtures';
import {
envelopeRequestParser,
hidePage,
shouldSkipTracingTest,
testingCdnBundle,
waitForClientReportRequest,
} from '../../../utils/helpers';

sentryTest(
'records no_parent_span client report for fetch requests without an active span',
async ({ getLocalTestUrl, page }) => {
sentryTest.skip(shouldSkipTracingTest() || testingCdnBundle());

await page.route('http://sentry-test-site.example/api/test', route => {
route.fulfill({
status: 200,
body: 'ok',
headers: { 'Content-Type': 'text/plain' },
});
});

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

const clientReportPromise = waitForClientReportRequest(page, report => {
return report.discarded_events.some(e => e.reason === 'no_parent_span');
});

await page.goto(url);

await hidePage(page);

const clientReport = envelopeRequestParser<ClientReport>(await clientReportPromise);

expect(clientReport.discarded_events).toEqual([
{
category: 'span',
quantity: 1,
reason: 'no_parent_span',
},
]);
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
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',
clientReportFlushInterval: 1_000,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import http from 'http';
http.get('http://localhost:9999/external', () => {}).on('error', () => {});
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { afterAll, describe, expect } from 'vitest';
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';

describe('no_parent_span client report (streaming)', () => {
afterAll(() => {
cleanupChildProcesses();
});

createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
test('records no_parent_span outcome for http.client span without a local parent', async () => {
const runner = createRunner()
.unignore('client_report')
.expect({
client_report: report => {
expect(report.discarded_events).toEqual([
{
category: 'span',
quantity: 1,
reason: 'no_parent_span',
},
]);
},
})
.start();

await runner.completed();
});
});
});
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,
clientReportFlushInterval: 1_000,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import http from 'http';
http.get('http://localhost:9999/external', () => {}).on('error', () => {});
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { afterAll, describe, expect } from 'vitest';
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';

describe('no_parent_span client report (streaming)', () => {
Comment thread
s1gr1d marked this conversation as resolved.
Outdated
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
afterAll(() => {
cleanupChildProcesses();
});

createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
test('records no_parent_span outcome for http.client span without a local parent', async () => {
const runner = createRunner()
.unignore('client_report')
.expect({
client_report: report => {
expect(report.discarded_events).toEqual([
{
category: 'span',
quantity: 1,
reason: 'no_parent_span',
},
]);
},
})
.start();

await runner.completed();
});
});
});
6 changes: 5 additions & 1 deletion packages/browser/src/tracing/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ function xhrCallback(

const urlForSpanName = stripDataUrlContent(stripUrlQueryAndFragment(url));

const client = getClient();
const hasParent = !!getActiveSpan();

const span =
Expand All @@ -424,6 +425,10 @@ function xhrCallback(
})
: new SentryNonRecordingSpan();

if (shouldCreateSpanResult && !hasParent) {
client?.recordDroppedEvent('no_parent_span', 'span');
}

xhr.__sentry_xhr_span_id__ = span.spanContext().spanId;
spans[xhr.__sentry_xhr_span_id__] = span;

Expand All @@ -438,7 +443,6 @@ function xhrCallback(
);
}

const client = getClient();
if (client) {
client.emit('beforeOutgoingRequestSpan', span, handlerData as XhrHint);
}
Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,18 @@ export function instrumentFetchRequest(
const { spanOrigin = 'auto.http.browser', propagateTraceparent = false } =
typeof spanOriginOrOptions === 'object' ? spanOriginOrOptions : { spanOrigin: spanOriginOrOptions };

const client = getClient();
const hasParent = !!getActiveSpan();

const span =
shouldCreateSpanResult && hasParent
? startInactiveSpan(getSpanStartOptions(url, method, spanOrigin))
: new SentryNonRecordingSpan();

if (shouldCreateSpanResult && !hasParent) {
client?.recordDroppedEvent('no_parent_span', 'span');
}

handlerData.fetchData.__span = span.spanContext().spanId;
spans[span.spanContext().spanId] = span;

Expand All @@ -141,8 +146,6 @@ export function instrumentFetchRequest(
}
}

const client = getClient();

if (client) {
const fetchHint = {
input: handlerData.args,
Expand Down
23 changes: 17 additions & 6 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ export function startSpan<T>(options: StartSpanOptions, callback: (span: Span) =
return wrapper(() => {
const scope = getCurrentScope();
const parentSpan = getParentSpan(scope, customParentSpan);
const client = getClient();

const shouldSkipSpan = options.onlyIfParent && !parentSpan;
const activeSpan = shouldSkipSpan
const missingRequiredParent = options.onlyIfParent && !parentSpan;
const activeSpan = missingRequiredParent
? new SentryNonRecordingSpan()
: createChildOrRootSpan({
parentSpan,
Expand All @@ -79,6 +80,10 @@ export function startSpan<T>(options: StartSpanOptions, callback: (span: Span) =
scope,
});

if (missingRequiredParent) {
client?.recordDroppedEvent('no_parent_span', 'span');
}

// Ignored root spans still need to be set on scope so that `getActiveSpan()` returns them
// and descendants are also non-recording. Ignored child spans don't need this because
// the parent span is already on scope.
Expand Down Expand Up @@ -132,8 +137,8 @@ export function startSpanManual<T>(options: StartSpanOptions, callback: (span: S
const scope = getCurrentScope();
const parentSpan = getParentSpan(scope, customParentSpan);

const shouldSkipSpan = options.onlyIfParent && !parentSpan;
const activeSpan = shouldSkipSpan
const missingRequiredParent = options.onlyIfParent && !parentSpan;
const activeSpan = missingRequiredParent
? new SentryNonRecordingSpan()
: createChildOrRootSpan({
parentSpan,
Expand All @@ -142,6 +147,10 @@ export function startSpanManual<T>(options: StartSpanOptions, callback: (span: S
scope,
});

if (missingRequiredParent) {
getClient()?.recordDroppedEvent('no_parent_span', 'span');
}

// We don't set ignored child spans onto the scope because there likely is an active,
// unignored span on the scope already.
if (!_isIgnoredSpan(activeSpan) || !parentSpan) {
Expand Down Expand Up @@ -195,10 +204,12 @@ export function startInactiveSpan(options: StartSpanOptions): Span {
return wrapper(() => {
const scope = getCurrentScope();
const parentSpan = getParentSpan(scope, customParentSpan);
const client = getClient();

const shouldSkipSpan = options.onlyIfParent && !parentSpan;
const missingRequiredParent = options.onlyIfParent && !parentSpan;

if (shouldSkipSpan) {
if (missingRequiredParent) {
client?.recordDroppedEvent('no_parent_span', 'span');
return new SentryNonRecordingSpan();
}

Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/types-hoist/clientreport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ export type EventDropReason =
| 'internal_sdk_error'
| 'buffer_overflow'
| 'ignored'
| 'invalid';
| 'invalid'
| 'no_parent_span';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we need a change in relay for this?


export type Outcome = {
reason: EventDropReason;
Expand Down
Loading
Loading