From 462df8fb459a1d8a77a9b0b2f5ee8ebd08184fb4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 28 Aug 2024 07:53:57 +0000 Subject: [PATCH 1/6] fix(node): Suppress tracing for transport request execution rather than transport creation --- packages/node/src/transports/http.ts | 108 ++++++++++++++------------- 1 file changed, 55 insertions(+), 53 deletions(-) diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index 751e4f3b3f4d..c4f13c89ee1b 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -79,11 +79,8 @@ export function makeNodeTransport(options: NodeTransportOptions): Transport { ? (new HttpsProxyAgent(proxy) as http.Agent) : new nativeHttpModule.Agent({ keepAlive, maxSockets: 30, timeout: 2000 }); - // This ensures we do not generate any spans in OpenTelemetry for the transport - return suppressTracing(() => { - const requestExecutor = createRequestExecutor(options, options.httpModule ?? nativeHttpModule, agent); - return createTransport(options, requestExecutor); - }); + const requestExecutor = createRequestExecutor(options, options.httpModule ?? nativeHttpModule, agent); + return createTransport(options, requestExecutor); } /** @@ -122,54 +119,59 @@ function createRequestExecutor( const { hostname, pathname, port, protocol, search } = new URL(options.url); return function makeRequest(request: TransportRequest): Promise { return new Promise((resolve, reject) => { - let body = streamFromBody(request.body); - - const headers: Record = { ...options.headers }; - - if (request.body.length > GZIP_THRESHOLD) { - headers['content-encoding'] = 'gzip'; - body = body.pipe(createGzip()); - } - - const req = httpModule.request( - { - method: 'POST', - agent, - headers, - hostname, - path: `${pathname}${search}`, - port, - protocol, - ca: options.caCerts, - }, - res => { - res.on('data', () => { - // Drain socket - }); - - res.on('end', () => { - // Drain socket - }); - - res.setEncoding('utf8'); - - // "Key-value pairs of header names and values. Header names are lower-cased." - // https://nodejs.org/api/http.html#http_message_headers - const retryAfterHeader = res.headers['retry-after'] ?? null; - const rateLimitsHeader = res.headers['x-sentry-rate-limits'] ?? null; - - resolve({ - statusCode: res.statusCode, - headers: { - 'retry-after': retryAfterHeader, - 'x-sentry-rate-limits': Array.isArray(rateLimitsHeader) ? rateLimitsHeader[0] || null : rateLimitsHeader, - }, - }); - }, - ); - - req.on('error', reject); - body.pipe(req); + // This ensures we do not generate any spans in OpenTelemetry for the transport + suppressTracing(() => { + let body = streamFromBody(request.body); + + const headers: Record = { ...options.headers }; + + if (request.body.length > GZIP_THRESHOLD) { + headers['content-encoding'] = 'gzip'; + body = body.pipe(createGzip()); + } + + const req = httpModule.request( + { + method: 'POST', + agent, + headers, + hostname, + path: `${pathname}${search}`, + port, + protocol, + ca: options.caCerts, + }, + res => { + res.on('data', () => { + // Drain socket + }); + + res.on('end', () => { + // Drain socket + }); + + res.setEncoding('utf8'); + + // "Key-value pairs of header names and values. Header names are lower-cased." + // https://nodejs.org/api/http.html#http_message_headers + const retryAfterHeader = res.headers['retry-after'] ?? null; + const rateLimitsHeader = res.headers['x-sentry-rate-limits'] ?? null; + + resolve({ + statusCode: res.statusCode, + headers: { + 'retry-after': retryAfterHeader, + 'x-sentry-rate-limits': Array.isArray(rateLimitsHeader) + ? rateLimitsHeader[0] || null + : rateLimitsHeader, + }, + }); + }, + ); + + req.on('error', reject); + body.pipe(req); + }); }); }; } From 43a2cf1eea4a335917e409219eb35ca61e98aa56 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 28 Aug 2024 08:49:08 +0000 Subject: [PATCH 2/6] Remove logic to not create span for sentry requests (replaced with proper suppressTracing) --- packages/node/src/integrations/http.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 615506605c9b..5e0b8d311546 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -102,10 +102,6 @@ export const instrumentHttp = Object.assign( return false; } - if (isSentryRequestUrl(url, getClient())) { - return true; - } - const _ignoreOutgoingRequests = _httpOptions.ignoreOutgoingRequests; if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { return true; From 9bf399c51949e1db9dd4dfcd22db4cf4c4d699f9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 28 Aug 2024 08:49:28 +0000 Subject: [PATCH 3/6] Wrap all transport implementations in `suppressTracing` --- packages/browser/src/transports/fetch.ts | 24 +++++++++++--------- packages/cloudflare/src/transport.ts | 20 ++++++++-------- packages/deno/src/transports/index.ts | 20 ++++++++-------- packages/vercel-edge/src/transports/index.ts | 20 ++++++++-------- 4 files changed, 46 insertions(+), 38 deletions(-) diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts index 52ba6d71154c..243c30791e36 100644 --- a/packages/browser/src/transports/fetch.ts +++ b/packages/browser/src/transports/fetch.ts @@ -1,5 +1,5 @@ import { clearCachedImplementation, getNativeImplementation } from '@sentry-internal/browser-utils'; -import { createTransport } from '@sentry/core'; +import { createTransport, suppressTracing } from '@sentry/core'; import type { Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types'; import { rejectedSyncPromise } from '@sentry/utils'; import type { WINDOW } from '../helpers'; @@ -47,16 +47,18 @@ export function makeFetchTransport( } try { - return nativeFetch(options.url, requestOptions).then(response => { - pendingBodySize -= requestSize; - pendingCount--; - return { - statusCode: response.status, - headers: { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }, - }; + return suppressTracing(() => { + return nativeFetch(options.url, requestOptions).then(response => { + pendingBodySize -= requestSize; + pendingCount--; + return { + statusCode: response.status, + headers: { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }, + }; + }); }); } catch (e) { clearCachedImplementation('fetch'); diff --git a/packages/cloudflare/src/transport.ts b/packages/cloudflare/src/transport.ts index fd26b217c367..4f1314d693a7 100644 --- a/packages/cloudflare/src/transport.ts +++ b/packages/cloudflare/src/transport.ts @@ -1,4 +1,4 @@ -import { createTransport } from '@sentry/core'; +import { createTransport, suppressTracing } from '@sentry/core'; import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types'; import { SentryError } from '@sentry/utils'; @@ -89,14 +89,16 @@ export function makeCloudflareTransport(options: CloudflareTransportOptions): Tr ...options.fetchOptions, }; - return fetch(options.url, requestOptions).then(response => { - return { - statusCode: response.status, - headers: { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }, - }; + return suppressTracing(() => { + return fetch(options.url, requestOptions).then(response => { + return { + statusCode: response.status, + headers: { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }, + }; + }); }); } diff --git a/packages/deno/src/transports/index.ts b/packages/deno/src/transports/index.ts index c678688c2462..1b2b3c661af9 100644 --- a/packages/deno/src/transports/index.ts +++ b/packages/deno/src/transports/index.ts @@ -1,4 +1,4 @@ -import { createTransport } from '@sentry/core'; +import { createTransport, suppressTracing } from '@sentry/core'; import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types'; import { consoleSandbox, logger, rejectedSyncPromise } from '@sentry/utils'; @@ -37,14 +37,16 @@ export function makeFetchTransport(options: DenoTransportOptions): Transport { }; try { - return fetch(options.url, requestOptions).then(response => { - return { - statusCode: response.status, - headers: { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }, - }; + return suppressTracing(() => { + return fetch(options.url, requestOptions).then(response => { + return { + statusCode: response.status, + headers: { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }, + }; + }); }); } catch (e) { return rejectedSyncPromise(e); diff --git a/packages/vercel-edge/src/transports/index.ts b/packages/vercel-edge/src/transports/index.ts index 4e8a35ac7c39..b938647b4415 100644 --- a/packages/vercel-edge/src/transports/index.ts +++ b/packages/vercel-edge/src/transports/index.ts @@ -1,4 +1,4 @@ -import { createTransport } from '@sentry/core'; +import { createTransport, suppressTracing } from '@sentry/core'; import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types'; import { SentryError } from '@sentry/utils'; @@ -89,14 +89,16 @@ export function makeEdgeTransport(options: VercelEdgeTransportOptions): Transpor ...options.fetchOptions, }; - return fetch(options.url, requestOptions).then(response => { - return { - statusCode: response.status, - headers: { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }, - }; + return suppressTracing(() => { + return fetch(options.url, requestOptions).then(response => { + return { + statusCode: response.status, + headers: { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }, + }; + }); }); } From c29c1906ef81c7ccf27329112a2476f4c0c6f60a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 28 Aug 2024 09:07:16 +0000 Subject: [PATCH 4/6] format & lint --- packages/node/src/integrations/http.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 5e0b8d311546..0d5b2d4814d1 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -9,7 +9,6 @@ import { getCapturedScopesOnSpan, getCurrentScope, getIsolationScope, - isSentryRequestUrl, setCapturedScopesOnSpan, } from '@sentry/core'; import { getClient } from '@sentry/opentelemetry'; From 5aedd74285621d53f7793b092019a1366b6b9b69 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 28 Aug 2024 09:11:43 +0000 Subject: [PATCH 5/6] Don't suppress for browser SDK --- packages/browser/src/transports/fetch.ts | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts index 243c30791e36..1e75f47458a4 100644 --- a/packages/browser/src/transports/fetch.ts +++ b/packages/browser/src/transports/fetch.ts @@ -47,18 +47,17 @@ export function makeFetchTransport( } try { - return suppressTracing(() => { - return nativeFetch(options.url, requestOptions).then(response => { - pendingBodySize -= requestSize; - pendingCount--; - return { - statusCode: response.status, - headers: { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }, - }; - }); + // TODO: This may need a `suppresTracing` call in the future when we switch the browser SDK to OTEL + return nativeFetch(options.url, requestOptions).then(response => { + pendingBodySize -= requestSize; + pendingCount--; + return { + statusCode: response.status, + headers: { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }, + }; }); } catch (e) { clearCachedImplementation('fetch'); From 691b52750661b6d19e476c7d6b57a612df57965a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 28 Aug 2024 11:38:15 +0000 Subject: [PATCH 6/6] fix --- packages/browser/src/transports/fetch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts index 1e75f47458a4..f9a7c258d4ff 100644 --- a/packages/browser/src/transports/fetch.ts +++ b/packages/browser/src/transports/fetch.ts @@ -1,5 +1,5 @@ import { clearCachedImplementation, getNativeImplementation } from '@sentry-internal/browser-utils'; -import { createTransport, suppressTracing } from '@sentry/core'; +import { createTransport } from '@sentry/core'; import type { Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types'; import { rejectedSyncPromise } from '@sentry/utils'; import type { WINDOW } from '../helpers';