Skip to content

Commit 738f17e

Browse files
authored
fix(v10/core): Don't instrument the SDK's own envelope requests (#24276)
Stops the outgoing-http instrumentation from instrumenting Sentry's own envelope sends. It recognized them only via `suppressTracing()`, which is unreadable when the SDK doesn't own the OTel setup, since no context manager propagates it. Each send then started a span that got dropped as `no_parent_span`, and since 10.72 the session sent on `beforeExit` seeds the first outcome, so the client report flush feeds itself and the process never exits. Matching the ingest URL instead doesn't depend on context propagation. closes #24262
1 parent f63c56a commit 738f17e

4 files changed

Lines changed: 96 additions & 1 deletion

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import * as Sentry from '@sentry/node';
2+
3+
Sentry.init({
4+
dsn: process.env.SENTRY_DSN,
5+
// A release makes the process session sendable, so the session is sent on `beforeExit`.
6+
release: '1.0.0',
7+
// Without the SDK's own OpenTelemetry setup there is no context manager, so the `suppressTracing()`
8+
// wrapper around transport requests has nowhere to store the suppression.
9+
skipOpenTelemetrySetup: true,
10+
});
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { afterAll, expect, test } from 'vitest';
2+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
3+
4+
afterAll(() => {
5+
cleanupChildProcesses();
6+
});
7+
8+
test('exits when client reports are flushed on `beforeExit` without the SDK owning the OpenTelemetry setup', async () => {
9+
// `ensureNoErrorOutput` is what separates a clean exit from a scenario that died on startup: both
10+
// leave the child exited, but only the latter writes to stderr.
11+
const runner = createRunner(__dirname, 'scenario.ts').withMockSentryServer().ensureNoErrorOutput().start();
12+
13+
// The scenario exits as soon as its `beforeExit` flush settles, so polling for the child's own
14+
// exit keeps the passing run short. A flush loop that keeps re-arming `beforeExit` never exits
15+
// and runs into the deadline.
16+
const deadline = Date.now() + 10_000;
17+
while (!runner.childHasExited() && Date.now() < deadline) {
18+
await new Promise(resolve => setTimeout(resolve, 100));
19+
}
20+
21+
expect(runner.childHasExited()).toBe(true);
22+
23+
await runner.completed();
24+
});

‎packages/core/src/integrations/http/client-subscriptions.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { LOG_PREFIX, HTTP_ON_CLIENT_REQUEST } from './constants';
3535
import type { ClientSubscriptionName } from './constants';
3636
import { getClient, getCurrentScope } from '../../currentScopes';
3737
import { hasSpansEnabled } from '../../utils/hasSpansEnabled';
38+
import { isSentryRequestUrl } from '../../utils/isSentryRequestUrl';
3839
import { doubleWrapWarning } from './double-wrap-warning';
3940

4041
type ChannelListener = (message: unknown, name: string | symbol) => void;
@@ -64,9 +65,20 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions):
6465
} = options;
6566

6667
const { request } = data as { request: HttpClientRequest };
68+
const requestUrl = getRequestUrlFromClientRequest(request);
69+
70+
// The suppression check above only works while the suppression is readable, which it is not when
71+
// the SDK does not own the OpenTelemetry setup: `suppressTracing()` writes to an OpenTelemetry
72+
// context that no registered context manager propagates. Recognizing the SDK's own requests by
73+
// their URL holds either way, and it has to: instrumenting them makes each envelope send record
74+
// a dropped-span outcome, which the `beforeExit` client report flush then sends as another
75+
// envelope, keeping the process alive forever.
76+
if (isSentryRequestUrl(requestUrl, getClient())) {
77+
return;
78+
}
6779

6880
// check if request is ignored. if so, we do nothing at all.
69-
if (options.ignoreOutgoingRequests?.(getRequestUrlFromClientRequest(request), request)) {
81+
if (options.ignoreOutgoingRequests?.(requestUrl, request)) {
7082
return;
7183
}
7284

‎packages/core/test/lib/integrations/http/client-subscriptions.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import { getHttpClientSubscriptions } from '../../../../src/integrations/http/cl
55
import type { HttpClientRequest, HttpIncomingMessage } from '../../../../src/integrations/http/types';
66
import { SUPPRESS_TRACING_KEY } from '../../../../src/tracing';
77
import { getCurrentScope, withScope } from '../../../../src/currentScopes';
8+
import type { Client } from '../../../../src/client';
9+
import { setCurrentClient } from '../../../../src/sdk';
10+
import { getDefaultTestClientOptions, TestClient } from '../../../mocks/client';
811

912
function makeMockRequest(): HttpClientRequest & {
1013
_responseListeners: ((res: HttpIncomingMessage) => void)[];
@@ -109,4 +112,50 @@ describe('getHttpClientSubscriptions', () => {
109112
expect(spy).toHaveBeenCalledOnce();
110113
});
111114
});
115+
116+
describe("the SDK's own requests", () => {
117+
afterEach(() => {
118+
setCurrentClient(undefined as unknown as Client);
119+
});
120+
121+
function setUpClientWithDsn(): void {
122+
const client = new TestClient(getDefaultTestClientOptions({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }));
123+
setCurrentClient(client);
124+
}
125+
126+
it('does not instrument requests to the ingest endpoint', () => {
127+
setUpClientWithDsn();
128+
const spy = vi.spyOn(breadcrumbModule, 'addOutgoingRequestBreadcrumb');
129+
const subscriptions = getHttpClientSubscriptions({ breadcrumbs: true, spans: false, propagateTrace: true });
130+
const handler = subscriptions[HTTP_ON_CLIENT_REQUEST];
131+
132+
const request = makeMockRequest();
133+
(request as { host: string }).host = 'dsn.ingest.sentry.io';
134+
(request as { path: string }).path = '/api/1337/envelope/?sentry_version=7&sentry_key=public';
135+
136+
handler({ request }, HTTP_ON_CLIENT_REQUEST);
137+
138+
expect(request._responseListeners).toHaveLength(0);
139+
expect(request.setHeader).not.toHaveBeenCalled();
140+
expect(spy).not.toHaveBeenCalled();
141+
});
142+
143+
it('still instruments requests to the same host that are not envelope sends', () => {
144+
setUpClientWithDsn();
145+
const spy = vi.spyOn(breadcrumbModule, 'addOutgoingRequestBreadcrumb');
146+
const subscriptions = getHttpClientSubscriptions({ breadcrumbs: true, spans: false });
147+
const handler = subscriptions[HTTP_ON_CLIENT_REQUEST];
148+
149+
const request = makeMockRequest();
150+
(request as { host: string }).host = 'dsn.ingest.sentry.io';
151+
152+
handler({ request }, HTTP_ON_CLIENT_REQUEST);
153+
154+
const response = makeMockResponse();
155+
request._responseListeners.forEach(fn => fn(response));
156+
response._endListeners.forEach(fn => fn());
157+
158+
expect(spy).toHaveBeenCalledOnce();
159+
});
160+
});
112161
});

0 commit comments

Comments
 (0)