From d84f341972b63743afe82192ff048ccefe8d0e8b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 7 Nov 2023 13:49:46 +0000 Subject: [PATCH 01/16] ref: Hoist performance fetch instrumentation into core --- packages/core/src/fetch.ts | 191 +++++++++++++++++++++++++++++++ packages/core/src/index.ts | 1 + packages/types/src/instrument.ts | 18 ++- 3 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 packages/core/src/fetch.ts diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts new file mode 100644 index 000000000000..827ca8d9e87f --- /dev/null +++ b/packages/core/src/fetch.ts @@ -0,0 +1,191 @@ +import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types'; +import { + BAGGAGE_HEADER_NAME, + dynamicSamplingContextToSentryBaggageHeader, + generateSentryTraceHeader, + isInstanceOf, +} from '@sentry/utils'; + +import { getCurrentHub } from './hub'; +import { getDynamicSamplingContextFromClient } from './tracing'; +import { hasTracingEnabled } from './utils/hasTracingEnabled'; + +type PolymorphicRequestHeaders = + | Record + | Array<[string, string]> + // the below is not preicsely the Header type used in Request, but it'll pass duck-typing + | { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: any; + append: (key: string, value: string) => void; + get: (key: string) => string | null | undefined; + }; + +/** + * Create and track fetch request spans for usage in combination with `addInstrumentationHandler`. + * + * @returns Span if a span was created, otherwise void. + */ +export function instrumentFetchRequest( + handlerData: HandlerDataFetch, + shouldCreateSpan: (url: string) => boolean, + shouldAttachHeaders: (url: string) => boolean, + spans: Record, + spanOrigin: SpanOrigin = 'auto.http.browser', +): Span | undefined { + if (!hasTracingEnabled() || !handlerData.fetchData) { + return undefined; + } + + const shouldCreateSpanResult = shouldCreateSpan(handlerData.fetchData.url); + + if (handlerData.endTimestamp && shouldCreateSpanResult) { + const spanId = handlerData.fetchData.__span; + if (!spanId) return; + + const span = spans[spanId]; + if (span) { + if (handlerData.response) { + span.setHttpStatus(handlerData.response.status); + + const contentLength = + handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length'); + + if (contentLength) { + const contentLengthNum = parseInt(contentLength); + if (contentLengthNum > 0) { + span.setData('http.response_content_length', contentLengthNum); + } + } + } else if (handlerData.error) { + span.setStatus('internal_error'); + } + span.finish(); + + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete spans[spanId]; + } + return undefined; + } + + const hub = getCurrentHub(); + const scope = hub.getScope(); + const client = hub.getClient(); + const parentSpan = scope.getSpan(); + + const { method, url } = handlerData.fetchData; + + const span = + shouldCreateSpanResult && parentSpan + ? parentSpan.startChild({ + data: { + url, + type: 'fetch', + 'http.method': method, + }, + description: `${method} ${url}`, + op: 'http.client', + origin: spanOrigin, + }) + : undefined; + + if (span) { + handlerData.fetchData.__span = span.spanId; + spans[span.spanId] = span; + } + + if (shouldAttachHeaders(handlerData.fetchData.url) && client) { + const request: string | Request = handlerData.args[0]; + + // In case the user hasn't set the second argument of a fetch call we default it to `{}`. + handlerData.args[1] = handlerData.args[1] || {}; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const options: { [key: string]: any } = handlerData.args[1]; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + options.headers = addTracingHeadersToFetchRequest(request, client, scope, options, span); + } + + return span; +} + +/** + * Adds sentry-trace and baggage headers to the various forms of fetch headers + */ +export function addTracingHeadersToFetchRequest( + request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package, + client: Client, + scope: Scope, + options: { + headers?: + | { + [key: string]: string[] | string | undefined; + } + | PolymorphicRequestHeaders; + }, + requestSpan?: Span, +): PolymorphicRequestHeaders | undefined { + const span = requestSpan || scope.getSpan(); + + const transaction = span && span.transaction; + + const { traceId, sampled, dsc } = scope.getPropagationContext(); + + const sentryTraceHeader = span ? span.toTraceparent() : generateSentryTraceHeader(traceId, undefined, sampled); + const dynamicSamplingContext = transaction + ? transaction.getDynamicSamplingContext() + : dsc + ? dsc + : getDynamicSamplingContextFromClient(traceId, client, scope); + + const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); + + const headers = + typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : options.headers; + + if (!headers) { + return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader }; + } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) { + const newHeaders = new Headers(headers as Headers); + + newHeaders.append('sentry-trace', sentryTraceHeader); + + if (sentryBaggageHeader) { + // If the same header is appended multiple times the browser will merge the values into a single request header. + // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. + newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader); + } + + return newHeaders as PolymorphicRequestHeaders; + } else if (Array.isArray(headers)) { + const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]]; + + if (sentryBaggageHeader) { + // If there are multiple entries with the same key, the browser will merge the values into a single request header. + // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. + newHeaders.push([BAGGAGE_HEADER_NAME, sentryBaggageHeader]); + } + + return newHeaders as PolymorphicRequestHeaders; + } else { + const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined; + const newBaggageHeaders: string[] = []; + + if (Array.isArray(existingBaggageHeader)) { + newBaggageHeaders.push(...existingBaggageHeader); + } else if (existingBaggageHeader) { + newBaggageHeaders.push(existingBaggageHeader); + } + + if (sentryBaggageHeader) { + newBaggageHeaders.push(sentryBaggageHeader); + } + + return { + ...(headers as Exclude), + 'sentry-trace': sentryTraceHeader, + baggage: newBaggageHeaders.length > 0 ? newBaggageHeaders.join(',') : undefined, + }; + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a912475e6c41..c6b39ffd81b1 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -55,6 +55,7 @@ export { hasTracingEnabled } from './utils/hasTracingEnabled'; export { isSentryRequestUrl } from './utils/isSentryRequestUrl'; export { DEFAULT_ENVIRONMENT } from './constants'; export { ModuleMetadata } from './integrations/metadata'; +export { instrumentFetchRequest } from './fetch'; import * as Integrations from './integrations'; export { Integrations }; diff --git a/packages/types/src/instrument.ts b/packages/types/src/instrument.ts index b47426e69049..b6bf1132d7ab 100644 --- a/packages/types/src/instrument.ts +++ b/packages/types/src/instrument.ts @@ -31,6 +31,8 @@ interface SentryFetchData { url: string; request_body_size?: number; response_body_size?: number; + // span_id for the fetch request + __span?: string; } export interface HandlerDataFetch { @@ -38,6 +40,18 @@ export interface HandlerDataFetch { fetchData: SentryFetchData; startTimestamp: number; endTimestamp?: number; - // This is actually `Response`, make sure to cast this where needed (not available in Node) - response?: unknown; + // This is actually `Response` - Note: this type is not complete. Add to it if necessary. + response?: { + readonly ok: boolean; + readonly status: number; + readonly url: string; + headers: { + append(name: string, value: string): void; + delete(name: string): void; + get(name: string): string | null; + has(name: string): boolean; + set(name: string, value: string): void; + }; + }; + error?: unknown; } From a660935f8d139d520da44f19eac82d82427cd170 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 7 Nov 2023 15:19:08 +0000 Subject: [PATCH 02/16] Remove old --- packages/core/src/fetch.ts | 3 +- .../tracing-internal/src/browser/request.ts | 126 ++---------------- 2 files changed, 14 insertions(+), 115 deletions(-) diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index 827ca8d9e87f..83958535c49b 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -8,7 +8,6 @@ import { import { getCurrentHub } from './hub'; import { getDynamicSamplingContextFromClient } from './tracing'; -import { hasTracingEnabled } from './utils/hasTracingEnabled'; type PolymorphicRequestHeaders = | Record @@ -33,7 +32,7 @@ export function instrumentFetchRequest( spans: Record, spanOrigin: SpanOrigin = 'auto.http.browser', ): Span | undefined { - if (!hasTracingEnabled() || !handlerData.fetchData) { + if (!handlerData.fetchData) { return undefined; } diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 551524cb2208..7107460482b7 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -1,6 +1,11 @@ /* eslint-disable max-lines */ -import { getCurrentHub, getDynamicSamplingContextFromClient, hasTracingEnabled } from '@sentry/core'; -import type { Client, Scope, Span } from '@sentry/types'; +import { + getCurrentHub, + getDynamicSamplingContextFromClient, + hasTracingEnabled, + instrumentFetchRequest, +} from '@sentry/core'; +import type { Client, HandlerDataFetch, Scope, Span } from '@sentry/types'; import { addInstrumentationHandler, BAGGAGE_HEADER_NAME, @@ -66,26 +71,6 @@ export interface RequestInstrumentationOptions { shouldCreateSpanForRequest?(this: void, url: string): boolean; } -/** Data returned from fetch callback */ -export interface FetchData { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - args: any[]; // the arguments passed to the fetch call itself - fetchData?: { - method: string; - url: string; - // span_id - __span?: string; - }; - - // TODO Should this be unknown instead? If we vendor types, make it a Response - // eslint-disable-next-line @typescript-eslint/no-explicit-any - response?: any; - error?: unknown; - - startTimestamp: number; - endTimestamp?: number; -} - /** Data returned from XHR request */ export interface XHRData { xhr?: { @@ -154,8 +139,12 @@ export function instrumentOutgoingRequests(_options?: Partial = {}; if (traceFetch) { - addInstrumentationHandler('fetch', (handlerData: FetchData) => { - const createdSpan = fetchCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); + addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch) => { + if (!hasTracingEnabled()) { + return; + } + + const createdSpan = instrumentFetchRequest(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); if (enableHTTPTimings && createdSpan) { addHTTPTimings(createdSpan); } @@ -276,95 +265,6 @@ export function shouldAttachHeaders(url: string, tracePropagationTargets: (strin return stringMatchesSomePattern(url, tracePropagationTargets || DEFAULT_TRACE_PROPAGATION_TARGETS); } -/** - * Create and track fetch request spans - * - * @returns Span if a span was created, otherwise void. - */ -export function fetchCallback( - handlerData: FetchData, - shouldCreateSpan: (url: string) => boolean, - shouldAttachHeaders: (url: string) => boolean, - spans: Record, -): Span | undefined { - if (!hasTracingEnabled() || !handlerData.fetchData) { - return undefined; - } - - const shouldCreateSpanResult = shouldCreateSpan(handlerData.fetchData.url); - - if (handlerData.endTimestamp && shouldCreateSpanResult) { - const spanId = handlerData.fetchData.__span; - if (!spanId) return; - - const span = spans[spanId]; - if (span) { - if (handlerData.response) { - // TODO (kmclb) remove this once types PR goes through - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - span.setHttpStatus(handlerData.response.status); - - const contentLength: string = - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length'); - - const contentLengthNum = parseInt(contentLength); - if (contentLengthNum > 0) { - span.setData('http.response_content_length', contentLengthNum); - } - } else if (handlerData.error) { - span.setStatus('internal_error'); - } - span.finish(); - - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete spans[spanId]; - } - return undefined; - } - - const hub = getCurrentHub(); - const scope = hub.getScope(); - const client = hub.getClient(); - const parentSpan = scope.getSpan(); - - const { method, url } = handlerData.fetchData; - - const span = - shouldCreateSpanResult && parentSpan - ? parentSpan.startChild({ - data: { - url, - type: 'fetch', - 'http.method': method, - }, - description: `${method} ${url}`, - op: 'http.client', - origin: 'auto.http.browser', - }) - : undefined; - - if (span) { - handlerData.fetchData.__span = span.spanId; - spans[span.spanId] = span; - } - - if (shouldAttachHeaders(handlerData.fetchData.url) && client) { - const request: string | Request = handlerData.args[0]; - - // In case the user hasn't set the second argument of a fetch call we default it to `{}`. - handlerData.args[1] = handlerData.args[1] || {}; - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const options: { [key: string]: any } = handlerData.args[1]; - - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access - options.headers = addTracingHeadersToFetchRequest(request, client, scope, options, span); - } - - return span; -} - /** * Adds sentry-trace and baggage headers to the various forms of fetch headers */ From 6ec75e2b6e9f16d34ffea36604e4ac4ea5552989 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Nov 2023 10:29:28 +0000 Subject: [PATCH 03/16] wtf --- packages/core/src/index.ts | 1 - .../tracing-internal/src/browser/request.ts | 8 ++--- .../src/common}/fetch.ts | 4 +-- .../test/browser/request.test.ts | 34 +++++++++---------- 4 files changed, 20 insertions(+), 27 deletions(-) rename packages/{core/src => tracing-internal/src/common}/fetch.ts (98%) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c6b39ffd81b1..a912475e6c41 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -55,7 +55,6 @@ export { hasTracingEnabled } from './utils/hasTracingEnabled'; export { isSentryRequestUrl } from './utils/isSentryRequestUrl'; export { DEFAULT_ENVIRONMENT } from './constants'; export { ModuleMetadata } from './integrations/metadata'; -export { instrumentFetchRequest } from './fetch'; import * as Integrations from './integrations'; export { Integrations }; diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 7107460482b7..423d0b8c5d90 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -1,10 +1,5 @@ /* eslint-disable max-lines */ -import { - getCurrentHub, - getDynamicSamplingContextFromClient, - hasTracingEnabled, - instrumentFetchRequest, -} from '@sentry/core'; +import { getCurrentHub, getDynamicSamplingContextFromClient, hasTracingEnabled } from '@sentry/core'; import type { Client, HandlerDataFetch, Scope, Span } from '@sentry/types'; import { addInstrumentationHandler, @@ -17,6 +12,7 @@ import { stringMatchesSomePattern, } from '@sentry/utils'; +import { instrumentFetchRequest } from '../common/fetch'; import { addPerformanceInstrumentationHandler } from './instrument'; export const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^\/(?!\/)/]; diff --git a/packages/core/src/fetch.ts b/packages/tracing-internal/src/common/fetch.ts similarity index 98% rename from packages/core/src/fetch.ts rename to packages/tracing-internal/src/common/fetch.ts index 83958535c49b..ccb9334896f0 100644 --- a/packages/core/src/fetch.ts +++ b/packages/tracing-internal/src/common/fetch.ts @@ -1,3 +1,4 @@ +import { getCurrentHub, getDynamicSamplingContextFromClient } from '@sentry/core'; import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types'; import { BAGGAGE_HEADER_NAME, @@ -6,9 +7,6 @@ import { isInstanceOf, } from '@sentry/utils'; -import { getCurrentHub } from './hub'; -import { getDynamicSamplingContextFromClient } from './tracing'; - type PolymorphicRequestHeaders = | Record | Array<[string, string]> diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index 2db0ec839372..47d3e41fa8c9 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -1,19 +1,20 @@ /* eslint-disable deprecation/deprecation */ import * as sentryCore from '@sentry/core'; +import type { HandlerDataFetch } from '@sentry/types'; import * as utils from '@sentry/utils'; import { SENTRY_XHR_DATA_KEY } from '@sentry/utils'; import type { Transaction } from '../../../tracing/src'; import { addExtensionMethods, Span, spanStatusfromHttpCode } from '../../../tracing/src'; import { getDefaultBrowserClientOptions } from '../../../tracing/test/testutils'; -import type { FetchData, XHRData } from '../../src/browser/request'; +import type { XHRData } from '../../src/browser/request'; import { extractNetworkProtocol, - fetchCallback, instrumentOutgoingRequests, shouldAttachHeaders, xhrCallback, } from '../../src/browser/request'; +import { instrumentFetchRequest } from '../../src/common/fetch'; import { TestClient } from '../utils/TestClient'; beforeAll(() => { @@ -76,7 +77,7 @@ describe('callbacks', () => { }); describe('fetchCallback()', () => { - let fetchHandlerData: FetchData; + let fetchHandlerData: HandlerDataFetch; const fetchSpan = { data: { @@ -109,7 +110,7 @@ describe('callbacks', () => { ])( 'span creation/header attachment interaction - shouldCreateSpan: %s, shouldAttachHeaders: %s', (shouldCreateSpanReturnValue, shouldAttachHeadersReturnValue, expectedSpan, expectedHeaderKeys) => { - fetchCallback( + instrumentFetchRequest( fetchHandlerData, () => shouldCreateSpanReturnValue, () => shouldAttachHeadersReturnValue, @@ -126,10 +127,9 @@ describe('callbacks', () => { ); it('adds neither fetch request spans nor fetch request headers if there is no fetch data in handler data', () => { - delete fetchHandlerData.fetchData; const spans = {}; - fetchCallback(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); expect(spans).toEqual({}); @@ -141,7 +141,7 @@ describe('callbacks', () => { hasTracingEnabled.mockReturnValueOnce(false); const spans = {}; - fetchCallback(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); expect(spans).toEqual({}); @@ -153,7 +153,7 @@ describe('callbacks', () => { const spans = {}; // triggered by request being sent - fetchCallback(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); const newSpan = transaction.spanRecorder?.spans[1] as Span; @@ -174,7 +174,7 @@ describe('callbacks', () => { }; // triggered by response coming back - fetchCallback(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); expect(newSpan.endTimestamp).toBeDefined(); }); @@ -183,7 +183,7 @@ describe('callbacks', () => { const spans: Record = {}; // triggered by request being sent - fetchCallback(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); const newSpan = transaction.spanRecorder?.spans[1] as Span; @@ -196,7 +196,7 @@ describe('callbacks', () => { }; // triggered by response coming back - fetchCallback(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); expect(newSpan.status).toBe(spanStatusfromHttpCode(404)); }); @@ -211,7 +211,7 @@ describe('callbacks', () => { }; // in that case, the response coming back will be ignored - fetchCallback(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, {}); + instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, {}); const newSpan = transaction.spanRecorder?.spans[1]; @@ -221,7 +221,7 @@ describe('callbacks', () => { it('uses active span to generate sentry-trace header', () => { const spans: Record = {}; // triggered by request being sent - fetchCallback(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); const activeSpan = transaction.spanRecorder?.spans[1] as Span; @@ -232,7 +232,7 @@ describe('callbacks', () => { }; // triggered by response coming back - fetchCallback(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); const headers = (fetchHandlerData.args[1].headers as Record) || {}; expect(headers['sentry-trace']).toEqual(`${activeSpan.traceId}-${activeSpan.spanId}-1`); @@ -242,7 +242,7 @@ describe('callbacks', () => { const spans: Record = {}; // triggered by request being sent - fetchCallback(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); const newSpan = transaction.spanRecorder?.spans[1] as Span; @@ -252,10 +252,10 @@ describe('callbacks', () => { ...fetchHandlerData, endTimestamp, response: { status: 404, headers: { get: () => 123 } }, - }; + } as unknown as HandlerDataFetch; // triggered by response coming back - fetchCallback(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); const finishedSpan = transaction.spanRecorder?.spans[1] as Span; From 79a72e724632038c3264d5feb12c06bb051c8733 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Nov 2023 10:34:58 +0000 Subject: [PATCH 04/16] feat(vercel-edge): Add fetch instrumentation --- packages/nextjs/src/edge/index.ts | 8 ++ .../src/integrations/edge-fetch.ts | 123 ++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 packages/vercel-edge/src/integrations/edge-fetch.ts diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index 07c6de85e92c..acb68f6466e5 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -9,10 +9,18 @@ export type EdgeOptions = VercelEdgeOptions; const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & { __rewriteFramesDistDir__?: string; + fetch: (...args: unknown[]) => unknown; }; /** Inits the Sentry NextJS SDK on the Edge Runtime. */ export function init(options: VercelEdgeOptions = {}): void { + globalWithInjectedValues.fetch = new Proxy(globalWithInjectedValues.fetch, { + apply(target, thisArg, argArray) { + console.log({ target, thisArg, argArray }); + return target.apply(thisArg, argArray); + }, + }); + const opts = { _metadata: {} as SdkMetadata, ...options, diff --git a/packages/vercel-edge/src/integrations/edge-fetch.ts b/packages/vercel-edge/src/integrations/edge-fetch.ts new file mode 100644 index 000000000000..799d585acb35 --- /dev/null +++ b/packages/vercel-edge/src/integrations/edge-fetch.ts @@ -0,0 +1,123 @@ +import { getCurrentHub, getDynamicSamplingContextFromClient, isSentryRequestUrl } from '@sentry/core'; +import type { EventProcessor, HandlerDataFetch, Integration, Span } from '@sentry/types'; +import { + addInstrumentationHandler, + dynamicSamplingContextToSentryBaggageHeader, + generateSentryTraceHeader, + LRUMap, + stringMatchesSomePattern, +} from '@sentry/utils'; + +import type { VercelEdgeClient } from '../client'; + +export interface Options { + /** + * Whether breadcrumbs should be recorded for requests + * Defaults to true + */ + breadcrumbs: boolean; + /** + * Function determining whether or not to create spans to track outgoing requests to the given URL. + * By default, spans will be created for all outgoing requests. + */ + shouldCreateSpanForRequest?: (url: string) => boolean; +} + +/** + * Instruments outgoing HTTP requests made with the `undici` package via + * Node's `diagnostics_channel` API. + * + * Supports Undici 4.7.0 or higher. + * + * Requires Node 16.17.0 or higher. + */ +export class EdgeFetch implements Integration { + /** + * @inheritDoc + */ + public static id: string = 'EdgeFetch'; + + /** + * @inheritDoc + */ + public name: string = EdgeFetch.id; + + private readonly _options: Options; + + private readonly _createSpanUrlMap: LRUMap = new LRUMap(100); + private readonly _headersUrlMap: LRUMap = new LRUMap(100); + + public constructor(_options: Partial = {}) { + this._options = { + breadcrumbs: _options.breadcrumbs === undefined ? true : _options.breadcrumbs, + shouldCreateSpanForRequest: _options.shouldCreateSpanForRequest, + }; + } + + /** + * @inheritDoc + */ + public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void): void { + const spans: Record = {}; + + addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch) => { + const hub = getCurrentHub(); + if (!hub.getIntegration(EdgeFetch)) { + return; + } + + // TODO: Ignore if sentry request + + createFetchSpan( + handlerData, + this._shouldCreateSpan.bind(this), + this._shouldAttachTraceData.bind(this), + spans, + 'auto.http.vercel_edge', + ); + + // TODO: Breadcrumbs + }); + } + + /** TODO */ + private _shouldAttachTraceData(url: string): boolean { + const hub = getCurrentHub(); + const client = hub.getClient(); + + if (!client) { + return false; + } + + const clientOptions = client.getOptions(); + + if (clientOptions.tracePropagationTargets === undefined) { + return true; + } + + const cachedDecision = this._headersUrlMap.get(url); + if (cachedDecision !== undefined) { + return cachedDecision; + } + + const decision = stringMatchesSomePattern(url, clientOptions.tracePropagationTargets); + this._headersUrlMap.set(url, decision); + return decision; + } + + /** Helper that wraps shouldCreateSpanForRequest option */ + private _shouldCreateSpan(url: string): boolean { + if (this._options.shouldCreateSpanForRequest === undefined) { + return true; + } + + const cachedDecision = this._createSpanUrlMap.get(url); + if (cachedDecision !== undefined) { + return cachedDecision; + } + + const decision = this._options.shouldCreateSpanForRequest(url); + this._createSpanUrlMap.set(url, decision); + return decision; + } +} From 1d1afe1c7b7f73ff5943c244d9fc8ed95d3ff385 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Nov 2023 10:40:42 +0000 Subject: [PATCH 05/16] export --- .../tracing-internal/src/browser/index.ts | 6 +- .../tracing-internal/src/browser/request.ts | 80 ------------------- packages/tracing-internal/src/index.ts | 3 +- 3 files changed, 3 insertions(+), 86 deletions(-) diff --git a/packages/tracing-internal/src/browser/index.ts b/packages/tracing-internal/src/browser/index.ts index 31c65aa303b4..5b30bc519404 100644 --- a/packages/tracing-internal/src/browser/index.ts +++ b/packages/tracing-internal/src/browser/index.ts @@ -3,11 +3,7 @@ export * from '../exports'; export type { RequestInstrumentationOptions } from './request'; export { BrowserTracing, BROWSER_TRACING_INTEGRATION_ID } from './browsertracing'; -export { - instrumentOutgoingRequests, - defaultRequestInstrumentationOptions, - addTracingHeadersToFetchRequest, -} from './request'; +export { instrumentOutgoingRequests, defaultRequestInstrumentationOptions } from './request'; export { addPerformanceInstrumentationHandler, diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 423d0b8c5d90..ab4dbb6884ed 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -261,86 +261,6 @@ export function shouldAttachHeaders(url: string, tracePropagationTargets: (strin return stringMatchesSomePattern(url, tracePropagationTargets || DEFAULT_TRACE_PROPAGATION_TARGETS); } -/** - * Adds sentry-trace and baggage headers to the various forms of fetch headers - */ -export function addTracingHeadersToFetchRequest( - request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package, - client: Client, - scope: Scope, - options: { - headers?: - | { - [key: string]: string[] | string | undefined; - } - | PolymorphicRequestHeaders; - }, - requestSpan?: Span, -): PolymorphicRequestHeaders | undefined { - const span = requestSpan || scope.getSpan(); - - const transaction = span && span.transaction; - - const { traceId, sampled, dsc } = scope.getPropagationContext(); - - const sentryTraceHeader = span ? span.toTraceparent() : generateSentryTraceHeader(traceId, undefined, sampled); - const dynamicSamplingContext = transaction - ? transaction.getDynamicSamplingContext() - : dsc - ? dsc - : getDynamicSamplingContextFromClient(traceId, client, scope); - - const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); - - const headers = - typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : options.headers; - - if (!headers) { - return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader }; - } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) { - const newHeaders = new Headers(headers as Headers); - - newHeaders.append('sentry-trace', sentryTraceHeader); - - if (sentryBaggageHeader) { - // If the same header is appended multiple times the browser will merge the values into a single request header. - // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. - newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader); - } - - return newHeaders as PolymorphicRequestHeaders; - } else if (Array.isArray(headers)) { - const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]]; - - if (sentryBaggageHeader) { - // If there are multiple entries with the same key, the browser will merge the values into a single request header. - // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. - newHeaders.push([BAGGAGE_HEADER_NAME, sentryBaggageHeader]); - } - - return newHeaders as PolymorphicRequestHeaders; - } else { - const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined; - const newBaggageHeaders: string[] = []; - - if (Array.isArray(existingBaggageHeader)) { - newBaggageHeaders.push(...existingBaggageHeader); - } else if (existingBaggageHeader) { - newBaggageHeaders.push(existingBaggageHeader); - } - - if (sentryBaggageHeader) { - newBaggageHeaders.push(sentryBaggageHeader); - } - - return { - ...(headers as Exclude), - 'sentry-trace': sentryTraceHeader, - baggage: newBaggageHeaders.length > 0 ? newBaggageHeaders.join(',') : undefined, - }; - } -} - /** * Create and track xhr request spans * diff --git a/packages/tracing-internal/src/index.ts b/packages/tracing-internal/src/index.ts index 50474994624e..495d8dbb26b9 100644 --- a/packages/tracing-internal/src/index.ts +++ b/packages/tracing-internal/src/index.ts @@ -17,13 +17,14 @@ export { BROWSER_TRACING_INTEGRATION_ID, instrumentOutgoingRequests, defaultRequestInstrumentationOptions, - addTracingHeadersToFetchRequest, addPerformanceInstrumentationHandler, addClsInstrumentationHandler, addFidInstrumentationHandler, addLcpInstrumentationHandler, } from './browser'; +export { addTracingHeadersToFetchRequest, instrumentFetchRequest } from './common/fetch'; + export type { RequestInstrumentationOptions } from './browser'; export { addExtensionMethods } from './extensions'; From abdbf9a36b8b811824f47842fd26a2dc40645f28 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Nov 2023 12:27:22 +0000 Subject: [PATCH 06/16] . --- packages/nextjs/src/edge/index.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index acb68f6466e5..3f114c500673 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -14,13 +14,6 @@ const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & { /** Inits the Sentry NextJS SDK on the Edge Runtime. */ export function init(options: VercelEdgeOptions = {}): void { - globalWithInjectedValues.fetch = new Proxy(globalWithInjectedValues.fetch, { - apply(target, thisArg, argArray) { - console.log({ target, thisArg, argArray }); - return target.apply(thisArg, argArray); - }, - }); - const opts = { _metadata: {} as SdkMetadata, ...options, From 9762348601bd3f23b6f958938046de5ce3415448 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Nov 2023 12:38:26 +0000 Subject: [PATCH 07/16] =?UTF-8?q?Why=20don't=20you=20work=20on=20vercel=20?= =?UTF-8?q?=F0=9F=98=A2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../node/src/integrations/undici/index.ts | 7 +- .../node/src/integrations/undici/types.ts | 1 - packages/utils/src/supports.ts | 6 + packages/vercel-edge/src/index.ts | 3 + .../src/integrations/edge-fetch.ts | 188 +++++++++++++++++- packages/vercel-edge/src/sdk.ts | 2 + 6 files changed, 200 insertions(+), 7 deletions(-) diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts index ff08d1df0f65..6ffec69d7d24 100644 --- a/packages/node/src/integrations/undici/index.ts +++ b/packages/node/src/integrations/undici/index.ts @@ -272,7 +272,10 @@ function setHeadersOnRequest( sentryTrace: string, sentryBaggageHeader: string | undefined, ): void { - if (request.__sentry_has_headers__) { + const headerLines = request.headers.split('\r\n'); + const hasSentryHeaders = headerLines.some(headerLine => headerLine.startsWith('sentry-trace:')); + + if (hasSentryHeaders) { return; } @@ -280,8 +283,6 @@ function setHeadersOnRequest( if (sentryBaggageHeader) { request.addHeader('baggage', sentryBaggageHeader); } - - request.__sentry_has_headers__ = true; } function createRequestSpan( diff --git a/packages/node/src/integrations/undici/types.ts b/packages/node/src/integrations/undici/types.ts index d885984671bf..231c49aebf1f 100644 --- a/packages/node/src/integrations/undici/types.ts +++ b/packages/node/src/integrations/undici/types.ts @@ -236,7 +236,6 @@ export interface UndiciResponse { export interface RequestWithSentry extends UndiciRequest { __sentry_span__?: Span; - __sentry_has_headers__?: boolean; } export interface RequestCreateMessage { diff --git a/packages/utils/src/supports.ts b/packages/utils/src/supports.ts index ebaa633acd7b..2e555ee2efb5 100644 --- a/packages/utils/src/supports.ts +++ b/packages/utils/src/supports.ts @@ -4,6 +4,8 @@ import { getGlobalObject } from './worldwide'; // eslint-disable-next-line deprecation/deprecation const WINDOW = getGlobalObject(); +declare const EdgeRuntime: string | undefined; + export { supportsHistory } from './vendor/supportsHistory'; /** @@ -89,6 +91,10 @@ export function isNativeFetch(func: Function): boolean { * @returns true if `window.fetch` is natively implemented, false otherwise */ export function supportsNativeFetch(): boolean { + if (typeof EdgeRuntime === 'string') { + return true; + } + if (!supportsFetch()) { return false; } diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index 47e1b9fd7924..42fef4975132 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -69,8 +69,11 @@ export { defaultIntegrations, init } from './sdk'; import { Integrations as CoreIntegrations } from '@sentry/core'; +import { EdgeFetch } from './integrations/edge-fetch'; + const INTEGRATIONS = { ...CoreIntegrations, + ...EdgeFetch, }; export { INTEGRATIONS as Integrations }; diff --git a/packages/vercel-edge/src/integrations/edge-fetch.ts b/packages/vercel-edge/src/integrations/edge-fetch.ts index 799d585acb35..4ce0c75ab910 100644 --- a/packages/vercel-edge/src/integrations/edge-fetch.ts +++ b/packages/vercel-edge/src/integrations/edge-fetch.ts @@ -1,9 +1,11 @@ -import { getCurrentHub, getDynamicSamplingContextFromClient, isSentryRequestUrl } from '@sentry/core'; -import type { EventProcessor, HandlerDataFetch, Integration, Span } from '@sentry/types'; +import { getCurrentHub, getDynamicSamplingContextFromClient } from '@sentry/core'; +import type { Client, EventProcessor, HandlerDataFetch, Integration, Scope, Span, SpanOrigin } from '@sentry/types'; import { addInstrumentationHandler, + BAGGAGE_HEADER_NAME, dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, + isInstanceOf, LRUMap, stringMatchesSomePattern, } from '@sentry/utils'; @@ -68,7 +70,7 @@ export class EdgeFetch implements Integration { // TODO: Ignore if sentry request - createFetchSpan( + instrumentFetchRequest( handlerData, this._shouldCreateSpan.bind(this), this._shouldAttachTraceData.bind(this), @@ -121,3 +123,183 @@ export class EdgeFetch implements Integration { return decision; } } + +type PolymorphicRequestHeaders = + | Record + | Array<[string, string]> + // the below is not preicsely the Header type used in Request, but it'll pass duck-typing + | { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: any; + append: (key: string, value: string) => void; + get: (key: string) => string | null | undefined; + }; + +/** + * Create and track fetch request spans for usage in combination with `addInstrumentationHandler`. + * + * @returns Span if a span was created, otherwise void. + */ +export function instrumentFetchRequest( + handlerData: HandlerDataFetch, + shouldCreateSpan: (url: string) => boolean, + shouldAttachHeaders: (url: string) => boolean, + spans: Record, + spanOrigin: SpanOrigin = 'auto.http.browser', +): Span | undefined { + if (!handlerData.fetchData) { + return undefined; + } + + const shouldCreateSpanResult = shouldCreateSpan(handlerData.fetchData.url); + + if (handlerData.endTimestamp && shouldCreateSpanResult) { + const spanId = handlerData.fetchData.__span; + if (!spanId) return; + + const span = spans[spanId]; + if (span) { + if (handlerData.response) { + span.setHttpStatus(handlerData.response.status); + + const contentLength = + handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length'); + + if (contentLength) { + const contentLengthNum = parseInt(contentLength); + if (contentLengthNum > 0) { + span.setData('http.response_content_length', contentLengthNum); + } + } + } else if (handlerData.error) { + span.setStatus('internal_error'); + } + span.finish(); + + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete spans[spanId]; + } + return undefined; + } + + const hub = getCurrentHub(); + const scope = hub.getScope(); + const client = hub.getClient(); + const parentSpan = scope.getSpan(); + + const { method, url } = handlerData.fetchData; + + const span = + shouldCreateSpanResult && parentSpan + ? parentSpan.startChild({ + data: { + url, + type: 'fetch', + 'http.method': method, + }, + description: `${method} ${url}`, + op: 'http.client', + origin: spanOrigin, + }) + : undefined; + + if (span) { + handlerData.fetchData.__span = span.spanId; + spans[span.spanId] = span; + } + + if (shouldAttachHeaders(handlerData.fetchData.url) && client) { + const request: string | Request = handlerData.args[0]; + + // In case the user hasn't set the second argument of a fetch call we default it to `{}`. + handlerData.args[1] = handlerData.args[1] || {}; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const options: { [key: string]: any } = handlerData.args[1]; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + options.headers = addTracingHeadersToFetchRequest(request, client, scope, options, span); + } + + return span; +} + +/** + * Adds sentry-trace and baggage headers to the various forms of fetch headers + */ +export function addTracingHeadersToFetchRequest( + request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package, + client: Client, + scope: Scope, + options: { + headers?: + | { + [key: string]: string[] | string | undefined; + } + | PolymorphicRequestHeaders; + }, + requestSpan?: Span, +): PolymorphicRequestHeaders | undefined { + const span = requestSpan || scope.getSpan(); + + const transaction = span && span.transaction; + + const { traceId, sampled, dsc } = scope.getPropagationContext(); + + const sentryTraceHeader = span ? span.toTraceparent() : generateSentryTraceHeader(traceId, undefined, sampled); + const dynamicSamplingContext = transaction + ? transaction.getDynamicSamplingContext() + : dsc + ? dsc + : getDynamicSamplingContextFromClient(traceId, client, scope); + + const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); + + const headers = + typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : options.headers; + + if (!headers) { + return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader }; + } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) { + const newHeaders = new Headers(headers as Headers); + + newHeaders.append('sentry-trace', sentryTraceHeader); + + if (sentryBaggageHeader) { + // If the same header is appended multiple times the browser will merge the values into a single request header. + // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. + newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader); + } + + return newHeaders as PolymorphicRequestHeaders; + } else if (Array.isArray(headers)) { + const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]]; + + if (sentryBaggageHeader) { + // If there are multiple entries with the same key, the browser will merge the values into a single request header. + // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. + newHeaders.push([BAGGAGE_HEADER_NAME, sentryBaggageHeader]); + } + + return newHeaders as PolymorphicRequestHeaders; + } else { + const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined; + const newBaggageHeaders: string[] = []; + + if (Array.isArray(existingBaggageHeader)) { + newBaggageHeaders.push(...existingBaggageHeader); + } else if (existingBaggageHeader) { + newBaggageHeaders.push(existingBaggageHeader); + } + + if (sentryBaggageHeader) { + newBaggageHeaders.push(sentryBaggageHeader); + } + + return { + ...(headers as Exclude), + 'sentry-trace': sentryTraceHeader, + baggage: newBaggageHeaders.length > 0 ? newBaggageHeaders.join(',') : undefined, + }; + } +} diff --git a/packages/vercel-edge/src/sdk.ts b/packages/vercel-edge/src/sdk.ts index 417c55bf809e..d58a41668f80 100644 --- a/packages/vercel-edge/src/sdk.ts +++ b/packages/vercel-edge/src/sdk.ts @@ -3,6 +3,7 @@ import { createStackParser, GLOBAL_OBJ, nodeStackLineParser, stackParserFromStac import { setAsyncLocalStorageAsyncContextStrategy } from './async'; import { VercelEdgeClient } from './client'; +import { EdgeFetch } from './integrations/edge-fetch'; import { makeEdgeTransport } from './transports'; import type { VercelEdgeClientOptions, VercelEdgeOptions } from './types'; import { getVercelEnv } from './utils/vercel'; @@ -17,6 +18,7 @@ export const defaultIntegrations = [ new CoreIntegrations.InboundFilters(), new CoreIntegrations.FunctionToString(), new CoreIntegrations.LinkedErrors(), + new EdgeFetch(), ]; /** Inits the Sentry NextJS SDK on the Edge Runtime. */ From 9344a37cf27378fa42b81617dceea32d90f98f6f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Nov 2023 10:40:00 +0000 Subject: [PATCH 08/16] Lint --- packages/tracing-internal/src/browser/request.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index ab4dbb6884ed..a730bc8955db 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -1,13 +1,12 @@ /* eslint-disable max-lines */ import { getCurrentHub, getDynamicSamplingContextFromClient, hasTracingEnabled } from '@sentry/core'; -import type { Client, HandlerDataFetch, Scope, Span } from '@sentry/types'; +import type { HandlerDataFetch, Span } from '@sentry/types'; import { addInstrumentationHandler, BAGGAGE_HEADER_NAME, browserPerformanceTimeOrigin, dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, - isInstanceOf, SENTRY_XHR_DATA_KEY, stringMatchesSomePattern, } from '@sentry/utils'; From 22c16d80070592e5c83b29f940c5a31c091443a5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Nov 2023 10:59:08 +0000 Subject: [PATCH 09/16] Undo stupid change and fix tests --- packages/tracing-internal/src/browser/request.ts | 4 ---- packages/tracing-internal/src/common/fetch.ts | 4 ++-- .../tracing-internal/test/browser/request.test.ts | 11 ----------- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index a730bc8955db..8178f698ed5a 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -135,10 +135,6 @@ export function instrumentOutgoingRequests(_options?: Partial { - if (!hasTracingEnabled()) { - return; - } - const createdSpan = instrumentFetchRequest(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); if (enableHTTPTimings && createdSpan) { addHTTPTimings(createdSpan); diff --git a/packages/tracing-internal/src/common/fetch.ts b/packages/tracing-internal/src/common/fetch.ts index ccb9334896f0..95e63d983935 100644 --- a/packages/tracing-internal/src/common/fetch.ts +++ b/packages/tracing-internal/src/common/fetch.ts @@ -1,4 +1,4 @@ -import { getCurrentHub, getDynamicSamplingContextFromClient } from '@sentry/core'; +import { getCurrentHub, getDynamicSamplingContextFromClient, hasTracingEnabled } from '@sentry/core'; import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types'; import { BAGGAGE_HEADER_NAME, @@ -30,7 +30,7 @@ export function instrumentFetchRequest( spans: Record, spanOrigin: SpanOrigin = 'auto.http.browser', ): Span | undefined { - if (!handlerData.fetchData) { + if (!hasTracingEnabled() || !handlerData.fetchData) { return undefined; } diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index 47d3e41fa8c9..b1e46bd05abe 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -126,17 +126,6 @@ describe('callbacks', () => { }, ); - it('adds neither fetch request spans nor fetch request headers if there is no fetch data in handler data', () => { - const spans = {}; - - instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - expect(spans).toEqual({}); - - const headers = (fetchHandlerData.args[1].headers as Record) || {}; - expect(Object.keys(headers)).toEqual([]); - }); - it('adds neither fetch request spans nor fetch request headers if tracing is disabled', () => { hasTracingEnabled.mockReturnValueOnce(false); const spans = {}; From 064d809d970d38b18b05f604567f2371dcd6de80 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Nov 2023 11:01:47 +0000 Subject: [PATCH 10/16] . --- packages/tracing-internal/src/browser/request.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 8178f698ed5a..06d60f219530 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -85,17 +85,6 @@ export interface XHRData { endTimestamp?: number; } -type PolymorphicRequestHeaders = - | Record - | Array<[string, string]> - // the below is not preicsely the Header type used in Request, but it'll pass duck-typing - | { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - [key: string]: any; - append: (key: string, value: string) => void; - get: (key: string) => string | null | undefined; - }; - export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions = { traceFetch: true, traceXHR: true, From e60c11f978006f133cabc4f4881f2d4843ff0af4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Nov 2023 11:23:20 +0000 Subject: [PATCH 11/16] . --- .../src/node/integrations/express.ts | 4 +- packages/vercel-edge/package.json | 3 +- packages/vercel-edge/src/index.ts | 4 +- .../src/integrations/edge-fetch.ts | 305 ------------------ .../src/integrations/wintercg-fetch.ts | 111 +++++++ packages/vercel-edge/src/sdk.ts | 4 +- yarn.lock | 17 +- 7 files changed, 133 insertions(+), 315 deletions(-) delete mode 100644 packages/vercel-edge/src/integrations/edge-fetch.ts create mode 100644 packages/vercel-edge/src/integrations/wintercg-fetch.ts diff --git a/packages/tracing-internal/src/node/integrations/express.ts b/packages/tracing-internal/src/node/integrations/express.ts index f7ff20fa9986..b8874ec6f27a 100644 --- a/packages/tracing-internal/src/node/integrations/express.ts +++ b/packages/tracing-internal/src/node/integrations/express.ts @@ -3,6 +3,7 @@ import type { Hub, Integration, PolymorphicRequest, Transaction } from '@sentry/ import { extractPathForTransaction, getNumberOfUrlSegments, + GLOBAL_OBJ, isRegExp, logger, stripUrlQueryAndFragment, @@ -485,7 +486,8 @@ function getLayerRoutePathInfo(layer: Layer): LayerRoutePathInfo { if (!lrp) { // parse node.js major version - const [major] = process.versions.node.split('.').map(Number); + // Next.js will complain if we directly use `proces.versions` here because of edge runtime. + const [major] = (GLOBAL_OBJ as unknown as NodeJS.Global).process.versions.node.split('.').map(Number); // allow call extractOriginalRoute only if node version support Regex d flag, node 16+ if (major >= 16) { diff --git a/packages/vercel-edge/package.json b/packages/vercel-edge/package.json index 0f945d2016a1..080a6cd9804a 100644 --- a/packages/vercel-edge/package.json +++ b/packages/vercel-edge/package.json @@ -25,7 +25,8 @@ "dependencies": { "@sentry/core": "7.80.1", "@sentry/types": "7.80.1", - "@sentry/utils": "7.80.1" + "@sentry/utils": "7.80.1", + "@sentry-internal/tracing": "7.80.1" }, "devDependencies": { "@edge-runtime/jest-environment": "2.2.3", diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index 85f63181b983..4f9b81345203 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -70,11 +70,11 @@ export { defaultIntegrations, init } from './sdk'; import { Integrations as CoreIntegrations } from '@sentry/core'; -import { EdgeFetch } from './integrations/edge-fetch'; +import { WinterCGFetch } from './integrations/wintercg-fetch'; const INTEGRATIONS = { ...CoreIntegrations, - ...EdgeFetch, + ...WinterCGFetch, }; export { INTEGRATIONS as Integrations }; diff --git a/packages/vercel-edge/src/integrations/edge-fetch.ts b/packages/vercel-edge/src/integrations/edge-fetch.ts deleted file mode 100644 index 4ce0c75ab910..000000000000 --- a/packages/vercel-edge/src/integrations/edge-fetch.ts +++ /dev/null @@ -1,305 +0,0 @@ -import { getCurrentHub, getDynamicSamplingContextFromClient } from '@sentry/core'; -import type { Client, EventProcessor, HandlerDataFetch, Integration, Scope, Span, SpanOrigin } from '@sentry/types'; -import { - addInstrumentationHandler, - BAGGAGE_HEADER_NAME, - dynamicSamplingContextToSentryBaggageHeader, - generateSentryTraceHeader, - isInstanceOf, - LRUMap, - stringMatchesSomePattern, -} from '@sentry/utils'; - -import type { VercelEdgeClient } from '../client'; - -export interface Options { - /** - * Whether breadcrumbs should be recorded for requests - * Defaults to true - */ - breadcrumbs: boolean; - /** - * Function determining whether or not to create spans to track outgoing requests to the given URL. - * By default, spans will be created for all outgoing requests. - */ - shouldCreateSpanForRequest?: (url: string) => boolean; -} - -/** - * Instruments outgoing HTTP requests made with the `undici` package via - * Node's `diagnostics_channel` API. - * - * Supports Undici 4.7.0 or higher. - * - * Requires Node 16.17.0 or higher. - */ -export class EdgeFetch implements Integration { - /** - * @inheritDoc - */ - public static id: string = 'EdgeFetch'; - - /** - * @inheritDoc - */ - public name: string = EdgeFetch.id; - - private readonly _options: Options; - - private readonly _createSpanUrlMap: LRUMap = new LRUMap(100); - private readonly _headersUrlMap: LRUMap = new LRUMap(100); - - public constructor(_options: Partial = {}) { - this._options = { - breadcrumbs: _options.breadcrumbs === undefined ? true : _options.breadcrumbs, - shouldCreateSpanForRequest: _options.shouldCreateSpanForRequest, - }; - } - - /** - * @inheritDoc - */ - public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void): void { - const spans: Record = {}; - - addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch) => { - const hub = getCurrentHub(); - if (!hub.getIntegration(EdgeFetch)) { - return; - } - - // TODO: Ignore if sentry request - - instrumentFetchRequest( - handlerData, - this._shouldCreateSpan.bind(this), - this._shouldAttachTraceData.bind(this), - spans, - 'auto.http.vercel_edge', - ); - - // TODO: Breadcrumbs - }); - } - - /** TODO */ - private _shouldAttachTraceData(url: string): boolean { - const hub = getCurrentHub(); - const client = hub.getClient(); - - if (!client) { - return false; - } - - const clientOptions = client.getOptions(); - - if (clientOptions.tracePropagationTargets === undefined) { - return true; - } - - const cachedDecision = this._headersUrlMap.get(url); - if (cachedDecision !== undefined) { - return cachedDecision; - } - - const decision = stringMatchesSomePattern(url, clientOptions.tracePropagationTargets); - this._headersUrlMap.set(url, decision); - return decision; - } - - /** Helper that wraps shouldCreateSpanForRequest option */ - private _shouldCreateSpan(url: string): boolean { - if (this._options.shouldCreateSpanForRequest === undefined) { - return true; - } - - const cachedDecision = this._createSpanUrlMap.get(url); - if (cachedDecision !== undefined) { - return cachedDecision; - } - - const decision = this._options.shouldCreateSpanForRequest(url); - this._createSpanUrlMap.set(url, decision); - return decision; - } -} - -type PolymorphicRequestHeaders = - | Record - | Array<[string, string]> - // the below is not preicsely the Header type used in Request, but it'll pass duck-typing - | { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - [key: string]: any; - append: (key: string, value: string) => void; - get: (key: string) => string | null | undefined; - }; - -/** - * Create and track fetch request spans for usage in combination with `addInstrumentationHandler`. - * - * @returns Span if a span was created, otherwise void. - */ -export function instrumentFetchRequest( - handlerData: HandlerDataFetch, - shouldCreateSpan: (url: string) => boolean, - shouldAttachHeaders: (url: string) => boolean, - spans: Record, - spanOrigin: SpanOrigin = 'auto.http.browser', -): Span | undefined { - if (!handlerData.fetchData) { - return undefined; - } - - const shouldCreateSpanResult = shouldCreateSpan(handlerData.fetchData.url); - - if (handlerData.endTimestamp && shouldCreateSpanResult) { - const spanId = handlerData.fetchData.__span; - if (!spanId) return; - - const span = spans[spanId]; - if (span) { - if (handlerData.response) { - span.setHttpStatus(handlerData.response.status); - - const contentLength = - handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length'); - - if (contentLength) { - const contentLengthNum = parseInt(contentLength); - if (contentLengthNum > 0) { - span.setData('http.response_content_length', contentLengthNum); - } - } - } else if (handlerData.error) { - span.setStatus('internal_error'); - } - span.finish(); - - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete spans[spanId]; - } - return undefined; - } - - const hub = getCurrentHub(); - const scope = hub.getScope(); - const client = hub.getClient(); - const parentSpan = scope.getSpan(); - - const { method, url } = handlerData.fetchData; - - const span = - shouldCreateSpanResult && parentSpan - ? parentSpan.startChild({ - data: { - url, - type: 'fetch', - 'http.method': method, - }, - description: `${method} ${url}`, - op: 'http.client', - origin: spanOrigin, - }) - : undefined; - - if (span) { - handlerData.fetchData.__span = span.spanId; - spans[span.spanId] = span; - } - - if (shouldAttachHeaders(handlerData.fetchData.url) && client) { - const request: string | Request = handlerData.args[0]; - - // In case the user hasn't set the second argument of a fetch call we default it to `{}`. - handlerData.args[1] = handlerData.args[1] || {}; - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const options: { [key: string]: any } = handlerData.args[1]; - - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access - options.headers = addTracingHeadersToFetchRequest(request, client, scope, options, span); - } - - return span; -} - -/** - * Adds sentry-trace and baggage headers to the various forms of fetch headers - */ -export function addTracingHeadersToFetchRequest( - request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package, - client: Client, - scope: Scope, - options: { - headers?: - | { - [key: string]: string[] | string | undefined; - } - | PolymorphicRequestHeaders; - }, - requestSpan?: Span, -): PolymorphicRequestHeaders | undefined { - const span = requestSpan || scope.getSpan(); - - const transaction = span && span.transaction; - - const { traceId, sampled, dsc } = scope.getPropagationContext(); - - const sentryTraceHeader = span ? span.toTraceparent() : generateSentryTraceHeader(traceId, undefined, sampled); - const dynamicSamplingContext = transaction - ? transaction.getDynamicSamplingContext() - : dsc - ? dsc - : getDynamicSamplingContextFromClient(traceId, client, scope); - - const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); - - const headers = - typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : options.headers; - - if (!headers) { - return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader }; - } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) { - const newHeaders = new Headers(headers as Headers); - - newHeaders.append('sentry-trace', sentryTraceHeader); - - if (sentryBaggageHeader) { - // If the same header is appended multiple times the browser will merge the values into a single request header. - // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. - newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader); - } - - return newHeaders as PolymorphicRequestHeaders; - } else if (Array.isArray(headers)) { - const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]]; - - if (sentryBaggageHeader) { - // If there are multiple entries with the same key, the browser will merge the values into a single request header. - // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. - newHeaders.push([BAGGAGE_HEADER_NAME, sentryBaggageHeader]); - } - - return newHeaders as PolymorphicRequestHeaders; - } else { - const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined; - const newBaggageHeaders: string[] = []; - - if (Array.isArray(existingBaggageHeader)) { - newBaggageHeaders.push(...existingBaggageHeader); - } else if (existingBaggageHeader) { - newBaggageHeaders.push(existingBaggageHeader); - } - - if (sentryBaggageHeader) { - newBaggageHeaders.push(sentryBaggageHeader); - } - - return { - ...(headers as Exclude), - 'sentry-trace': sentryTraceHeader, - baggage: newBaggageHeaders.length > 0 ? newBaggageHeaders.join(',') : undefined, - }; - } -} diff --git a/packages/vercel-edge/src/integrations/wintercg-fetch.ts b/packages/vercel-edge/src/integrations/wintercg-fetch.ts new file mode 100644 index 000000000000..080d65e99e74 --- /dev/null +++ b/packages/vercel-edge/src/integrations/wintercg-fetch.ts @@ -0,0 +1,111 @@ +import { instrumentFetchRequest } from '@sentry-internal/tracing'; +import { getCurrentHub } from '@sentry/core'; +import type { EventProcessor, HandlerDataFetch, Integration, Span } from '@sentry/types'; +import { addInstrumentationHandler, LRUMap, stringMatchesSomePattern } from '@sentry/utils'; + +export interface Options { + /** + * Whether breadcrumbs should be recorded for requests + * Defaults to true + */ + breadcrumbs: boolean; + /** + * Function determining whether or not to create spans to track outgoing requests to the given URL. + * By default, spans will be created for all outgoing requests. + */ + shouldCreateSpanForRequest?: (url: string) => boolean; +} + +/** + * Creates spans and attaches tracing headers to fetch requests on WinterCG runtimes. + */ +export class WinterCGFetch implements Integration { + /** + * @inheritDoc + */ + public static id: string = 'WinterCGFetch'; + + /** + * @inheritDoc + */ + public name: string = WinterCGFetch.id; + + private readonly _options: Options; + + private readonly _createSpanUrlMap: LRUMap = new LRUMap(100); + private readonly _headersUrlMap: LRUMap = new LRUMap(100); + + public constructor(_options: Partial = {}) { + this._options = { + breadcrumbs: _options.breadcrumbs === undefined ? true : _options.breadcrumbs, + shouldCreateSpanForRequest: _options.shouldCreateSpanForRequest, + }; + } + + /** + * @inheritDoc + */ + public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void): void { + const spans: Record = {}; + + addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch) => { + const hub = getCurrentHub(); + if (!hub.getIntegration(WinterCGFetch)) { + return; + } + + // TODO: Ignore if sentry request + + instrumentFetchRequest( + handlerData, + this._shouldCreateSpan.bind(this), + this._shouldAttachTraceData.bind(this), + spans, + 'auto.http.wintercg_fetch', + ); + + // TODO: Breadcrumbs + }); + } + + /** TODO */ + private _shouldAttachTraceData(url: string): boolean { + const hub = getCurrentHub(); + const client = hub.getClient(); + + if (!client) { + return false; + } + + const clientOptions = client.getOptions(); + + if (clientOptions.tracePropagationTargets === undefined) { + return true; + } + + const cachedDecision = this._headersUrlMap.get(url); + if (cachedDecision !== undefined) { + return cachedDecision; + } + + const decision = stringMatchesSomePattern(url, clientOptions.tracePropagationTargets); + this._headersUrlMap.set(url, decision); + return decision; + } + + /** Helper that wraps shouldCreateSpanForRequest option */ + private _shouldCreateSpan(url: string): boolean { + if (this._options.shouldCreateSpanForRequest === undefined) { + return true; + } + + const cachedDecision = this._createSpanUrlMap.get(url); + if (cachedDecision !== undefined) { + return cachedDecision; + } + + const decision = this._options.shouldCreateSpanForRequest(url); + this._createSpanUrlMap.set(url, decision); + return decision; + } +} diff --git a/packages/vercel-edge/src/sdk.ts b/packages/vercel-edge/src/sdk.ts index d58a41668f80..f49115452a5d 100644 --- a/packages/vercel-edge/src/sdk.ts +++ b/packages/vercel-edge/src/sdk.ts @@ -3,7 +3,7 @@ import { createStackParser, GLOBAL_OBJ, nodeStackLineParser, stackParserFromStac import { setAsyncLocalStorageAsyncContextStrategy } from './async'; import { VercelEdgeClient } from './client'; -import { EdgeFetch } from './integrations/edge-fetch'; +import { WinterCGFetch } from './integrations/wintercg-fetch'; import { makeEdgeTransport } from './transports'; import type { VercelEdgeClientOptions, VercelEdgeOptions } from './types'; import { getVercelEnv } from './utils/vercel'; @@ -18,7 +18,7 @@ export const defaultIntegrations = [ new CoreIntegrations.InboundFilters(), new CoreIntegrations.FunctionToString(), new CoreIntegrations.LinkedErrors(), - new EdgeFetch(), + new WinterCGFetch(), ]; /** Inits the Sentry NextJS SDK on the Edge Runtime. */ diff --git a/yarn.lock b/yarn.lock index 1ee62d9e6a00..c165b2135951 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8290,10 +8290,10 @@ aws4@^1.8.0: resolved "https://registry.yarnpkg.com/aws4/-/aws4-1.11.0.tgz#d61f46d83b2519250e2784daf5b09479a8b41c59" integrity sha512-xh1Rl34h6Fi1DC2WWKfxUTVqRsNnr6LsKz2+hfwDxQJWmrx8+c7ylaqBMcHfl1U1r2dsifOvKX3LQuLNZ+XSvA== -axios@1.3.4, axios@^1.2.2: - version "1.3.4" - resolved "https://registry.yarnpkg.com/axios/-/axios-1.3.4.tgz#f5760cefd9cfb51fd2481acf88c05f67c4523024" - integrity sha512-toYm+Bsyl6VC5wSkfkbbNB6ROv7KY93PEBBL6xyDczaIHasAiv4wPqQ/c4RjoQzipxRD2W5g21cOqQulZ7rHwQ== +axios@1.6.0: + version "1.6.0" + resolved "https://registry.yarnpkg.com/axios/-/axios-1.6.0.tgz#f1e5292f26b2fd5c2e66876adc5b06cdbd7d2102" + integrity sha512-EZ1DYihju9pwVB+jg67ogm+Tmqc6JmhamRN6I4Zt8DfZu5lbcQGw3ozH9lFejSJgs/ibaef3A9PMXPLeefFGJg== dependencies: follow-redirects "^1.15.0" form-data "^4.0.0" @@ -8316,6 +8316,15 @@ axios@^1.0.0: form-data "^4.0.0" proxy-from-env "^1.1.0" +axios@^1.2.2: + version "1.3.4" + resolved "https://registry.yarnpkg.com/axios/-/axios-1.3.4.tgz#f5760cefd9cfb51fd2481acf88c05f67c4523024" + integrity sha512-toYm+Bsyl6VC5wSkfkbbNB6ROv7KY93PEBBL6xyDczaIHasAiv4wPqQ/c4RjoQzipxRD2W5g21cOqQulZ7rHwQ== + dependencies: + follow-redirects "^1.15.0" + form-data "^4.0.0" + proxy-from-env "^1.1.0" + b4a@^1.6.4: version "1.6.4" resolved "https://registry.yarnpkg.com/b4a/-/b4a-1.6.4.tgz#ef1c1422cae5ce6535ec191baeed7567443f36c9" From a9ffcfba0da8d2da80479bc885d3686d5cfc3fb0 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Nov 2023 11:45:23 +0000 Subject: [PATCH 12/16] Don't trace sentry reqs and create breadcrumbs --- packages/types/src/instrument.ts | 2 +- .../src/integrations/wintercg-fetch.ts | 67 +++++++++++++++++-- 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/packages/types/src/instrument.ts b/packages/types/src/instrument.ts index b6bf1132d7ab..d3581d694312 100644 --- a/packages/types/src/instrument.ts +++ b/packages/types/src/instrument.ts @@ -37,7 +37,7 @@ interface SentryFetchData { export interface HandlerDataFetch { args: any[]; - fetchData: SentryFetchData; + fetchData: SentryFetchData; // This data is among other things dumped directly onto the fetch breadcrumb data startTimestamp: number; endTimestamp?: number; // This is actually `Response` - Note: this type is not complete. Add to it if necessary. diff --git a/packages/vercel-edge/src/integrations/wintercg-fetch.ts b/packages/vercel-edge/src/integrations/wintercg-fetch.ts index 080d65e99e74..ecd429e4cd26 100644 --- a/packages/vercel-edge/src/integrations/wintercg-fetch.ts +++ b/packages/vercel-edge/src/integrations/wintercg-fetch.ts @@ -1,6 +1,13 @@ import { instrumentFetchRequest } from '@sentry-internal/tracing'; -import { getCurrentHub } from '@sentry/core'; -import type { EventProcessor, HandlerDataFetch, Integration, Span } from '@sentry/types'; +import { getCurrentHub, isSentryRequestUrl } from '@sentry/core'; +import type { + EventProcessor, + FetchBreadcrumbData, + FetchBreadcrumbHint, + HandlerDataFetch, + Integration, + Span, +} from '@sentry/types'; import { addInstrumentationHandler, LRUMap, stringMatchesSomePattern } from '@sentry/utils'; export interface Options { @@ -54,7 +61,9 @@ export class WinterCGFetch implements Integration { return; } - // TODO: Ignore if sentry request + if (isSentryRequestUrl(handlerData.fetchData.url, hub)) { + return; + } instrumentFetchRequest( handlerData, @@ -64,7 +73,9 @@ export class WinterCGFetch implements Integration { 'auto.http.wintercg_fetch', ); - // TODO: Breadcrumbs + if (this._options.breadcrumbs) { + createBreadcrumb(handlerData); + } }); } @@ -109,3 +120,51 @@ export class WinterCGFetch implements Integration { return decision; } } + +function createBreadcrumb(handlerData: HandlerDataFetch): void { + const { startTimestamp, endTimestamp } = handlerData; + + // We only capture complete fetch requests + if (!endTimestamp) { + return; + } + + if (handlerData.error) { + const data = handlerData.fetchData; + const hint: FetchBreadcrumbHint = { + data: handlerData.error, + input: handlerData.args, + startTimestamp, + endTimestamp, + }; + + getCurrentHub().addBreadcrumb( + { + category: 'fetch', + data, + level: 'error', + type: 'http', + }, + hint, + ); + } else { + const data: FetchBreadcrumbData = { + ...handlerData.fetchData, + status_code: handlerData.response && handlerData.response.status, + }; + const hint: FetchBreadcrumbHint = { + input: handlerData.args, + response: handlerData.response, + startTimestamp, + endTimestamp, + }; + getCurrentHub().addBreadcrumb( + { + category: 'fetch', + data, + type: 'http', + }, + hint, + ); + } +} From e3648591ef38b53ad6017a83035aa68d0b69854e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 15 Nov 2023 11:47:44 +0000 Subject: [PATCH 13/16] . --- packages/vercel-edge/src/integrations/wintercg-fetch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vercel-edge/src/integrations/wintercg-fetch.ts b/packages/vercel-edge/src/integrations/wintercg-fetch.ts index ecd429e4cd26..f5127e2857ba 100644 --- a/packages/vercel-edge/src/integrations/wintercg-fetch.ts +++ b/packages/vercel-edge/src/integrations/wintercg-fetch.ts @@ -79,7 +79,7 @@ export class WinterCGFetch implements Integration { }); } - /** TODO */ + /** Decides whether to attach trace data to the outgoing fetch request */ private _shouldAttachTraceData(url: string): boolean { const hub = getCurrentHub(); const client = hub.getClient(); From 4c96af83c500fda17c223d57551bb3996ae571c2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 17 Nov 2023 12:11:27 +0000 Subject: [PATCH 14/16] Add tests --- .../src/integrations/wintercg-fetch.ts | 11 +- .../vercel-edge/test/wintercg-fetch.test.ts | 192 ++++++++++++++++++ 2 files changed, 194 insertions(+), 9 deletions(-) create mode 100644 packages/vercel-edge/test/wintercg-fetch.test.ts diff --git a/packages/vercel-edge/src/integrations/wintercg-fetch.ts b/packages/vercel-edge/src/integrations/wintercg-fetch.ts index f5127e2857ba..3ecffad83b33 100644 --- a/packages/vercel-edge/src/integrations/wintercg-fetch.ts +++ b/packages/vercel-edge/src/integrations/wintercg-fetch.ts @@ -1,13 +1,6 @@ import { instrumentFetchRequest } from '@sentry-internal/tracing'; import { getCurrentHub, isSentryRequestUrl } from '@sentry/core'; -import type { - EventProcessor, - FetchBreadcrumbData, - FetchBreadcrumbHint, - HandlerDataFetch, - Integration, - Span, -} from '@sentry/types'; +import type { FetchBreadcrumbData, FetchBreadcrumbHint, HandlerDataFetch, Integration, Span } from '@sentry/types'; import { addInstrumentationHandler, LRUMap, stringMatchesSomePattern } from '@sentry/utils'; export interface Options { @@ -52,7 +45,7 @@ export class WinterCGFetch implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void): void { + public setupOnce(): void { const spans: Record = {}; addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch) => { diff --git a/packages/vercel-edge/test/wintercg-fetch.test.ts b/packages/vercel-edge/test/wintercg-fetch.test.ts new file mode 100644 index 000000000000..699fb4891257 --- /dev/null +++ b/packages/vercel-edge/test/wintercg-fetch.test.ts @@ -0,0 +1,192 @@ +import * as internalTracing from '@sentry-internal/tracing'; +import * as sentryCore from '@sentry/core'; +import type { HandlerDataFetch, Integration, IntegrationClass } from '@sentry/types'; +import * as sentryUtils from '@sentry/utils'; +import { createStackParser } from '@sentry/utils'; + +import { VercelEdgeClient } from '../src/index'; +import { WinterCGFetch } from '../src/integrations/wintercg-fetch'; + +class FakeHub extends sentryCore.Hub { + getIntegration(integration: IntegrationClass): T | null { + return new integration(); + } +} + +const fakeHubInstance = new FakeHub( + new VercelEdgeClient({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + enableTracing: true, + tracesSampleRate: 1, + integrations: [], + transport: () => ({ + send: () => Promise.resolve(undefined), + flush: () => Promise.resolve(true), + }), + tracePropagationTargets: ['http://my-website.com/'], + stackParser: createStackParser(), + }), +); + +jest.spyOn(sentryCore, 'getCurrentHub').mockImplementation(() => fakeHubInstance); + +const addInstrumentationHandlerSpy = jest.spyOn(sentryUtils, 'addInstrumentationHandler'); +const instrumentFetchRequestSpy = jest.spyOn(internalTracing, 'instrumentFetchRequest'); +const addBreadcrumbSpy = jest.spyOn(fakeHubInstance, 'addBreadcrumb'); + +beforeEach(() => { + jest.clearAllMocks(); +}); + +describe('WinterCGFetch instrumentation', () => { + it('should call `instrumentFetchRequest` for outgoing fetch requests', () => { + const integration = new WinterCGFetch(); + addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + + integration.setupOnce(); + + const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = + addInstrumentationHandlerSpy.mock.calls[0]; + expect(fetchInstrumentationHandlerType).toBe('fetch'); + expect(fetchInstrumentationHandlerCallback).toBeDefined(); + + const startHandlerData: HandlerDataFetch = { + fetchData: { url: 'http://my-website.com/', method: 'POST' }, + args: ['http://my-website.com/'], + startTimestamp: Date.now(), + }; + fetchInstrumentationHandlerCallback(startHandlerData); + + expect(instrumentFetchRequestSpy).toHaveBeenCalledWith( + startHandlerData, + expect.any(Function), + expect.any(Function), + expect.any(Object), + 'auto.http.wintercg_fetch', + ); + + const [, shouldCreateSpan, shouldAttachTraceData] = instrumentFetchRequestSpy.mock.calls[0]; + + expect(shouldAttachTraceData('http://my-website.com/')).toBe(true); + expect(shouldAttachTraceData('https://www.3rd-party-website.at/')).toBe(false); + + expect(shouldCreateSpan('http://my-website.com/')).toBe(true); + expect(shouldCreateSpan('https://www.3rd-party-website.at/')).toBe(true); + }); + + it('should call `instrumentFetchRequest` for outgoing fetch requests to Sentry', () => { + const integration = new WinterCGFetch(); + addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + + integration.setupOnce(); + + const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = + addInstrumentationHandlerSpy.mock.calls[0]; + expect(fetchInstrumentationHandlerType).toBe('fetch'); + expect(fetchInstrumentationHandlerCallback).toBeDefined(); + + const startHandlerData: HandlerDataFetch = { + fetchData: { url: 'https://dsn.ingest.sentry.io/1337', method: 'POST' }, + args: ['https://dsn.ingest.sentry.io/1337'], + startTimestamp: Date.now(), + }; + fetchInstrumentationHandlerCallback(startHandlerData); + + expect(instrumentFetchRequestSpy).not.toHaveBeenCalled(); + }); + + it('should properly apply the `shouldCreateSpanForRequest` option', () => { + const integration = new WinterCGFetch({ + shouldCreateSpanForRequest(url) { + return url === 'http://only-acceptable-url.com/'; + }, + }); + addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + + integration.setupOnce(); + + const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = + addInstrumentationHandlerSpy.mock.calls[0]; + expect(fetchInstrumentationHandlerType).toBe('fetch'); + expect(fetchInstrumentationHandlerCallback).toBeDefined(); + + const startHandlerData: HandlerDataFetch = { + fetchData: { url: 'http://my-website.com/', method: 'POST' }, + args: ['http://my-website.com/'], + startTimestamp: Date.now(), + }; + fetchInstrumentationHandlerCallback(startHandlerData); + + const [, shouldCreateSpan] = instrumentFetchRequestSpy.mock.calls[0]; + + expect(shouldCreateSpan('http://only-acceptable-url.com/')).toBe(true); + expect(shouldCreateSpan('http://my-website.com/')).toBe(false); + expect(shouldCreateSpan('https://www.3rd-party-website.at/')).toBe(false); + }); + + it('should create a breadcrumb for an outgoing request', () => { + const integration = new WinterCGFetch(); + addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + + integration.setupOnce(); + + const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = + addInstrumentationHandlerSpy.mock.calls[0]; + expect(fetchInstrumentationHandlerType).toBe('fetch'); + expect(fetchInstrumentationHandlerCallback).toBeDefined(); + + const startTimestamp = Date.now(); + const endTimestamp = Date.now() + 100; + + const startHandlerData: HandlerDataFetch = { + fetchData: { url: 'http://my-website.com/', method: 'POST' }, + args: ['http://my-website.com/'], + response: { ok: true, status: 201, url: 'http://my-website.com/' } as Response, + startTimestamp, + endTimestamp, + }; + fetchInstrumentationHandlerCallback(startHandlerData); + + expect(addBreadcrumbSpy).toBeCalledWith( + { + category: 'fetch', + data: { method: 'POST', status_code: 201, url: 'http://my-website.com/' }, + type: 'http', + }, + { + endTimestamp, + input: ['http://my-website.com/'], + response: { ok: true, status: 201, url: 'http://my-website.com/' }, + startTimestamp, + }, + ); + }); + + it('should not create a breadcrumb for an outgoing request if `breadcrumbs: false` is set', () => { + const integration = new WinterCGFetch({ + breadcrumbs: false, + }); + addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + + integration.setupOnce(); + + const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = + addInstrumentationHandlerSpy.mock.calls[0]; + expect(fetchInstrumentationHandlerType).toBe('fetch'); + expect(fetchInstrumentationHandlerCallback).toBeDefined(); + + const startTimestamp = Date.now(); + const endTimestamp = Date.now() + 100; + + const startHandlerData: HandlerDataFetch = { + fetchData: { url: 'http://my-website.com/', method: 'POST' }, + args: ['http://my-website.com/'], + response: { ok: true, status: 201, url: 'http://my-website.com/' } as Response, + startTimestamp, + endTimestamp, + }; + fetchInstrumentationHandlerCallback(startHandlerData); + + expect(addBreadcrumbSpy).not.toHaveBeenCalled(); + }); +}); From 77fdb4ee8219f20c9cb66fcd742103fbb55746e4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 17 Nov 2023 12:41:19 +0000 Subject: [PATCH 15/16] Add E2E test --- .../nextjs-app-dir/middleware.ts | 6 ++- .../nextjs-app-dir/tests/middleware.test.ts | 41 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts index bb9db27b50d7..a491ccde0a91 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts @@ -1,11 +1,15 @@ import { NextResponse } from 'next/server'; import type { NextRequest } from 'next/server'; -export function middleware(request: NextRequest) { +export async function middleware(request: NextRequest) { if (request.headers.has('x-should-throw')) { throw new Error('Middleware Error'); } + if (request.headers.has('x-should-make-request')) { + await fetch('http://localhost:3030/'); + } + return NextResponse.next(); } diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts index 7cf989236360..4211d7e640ff 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts @@ -45,3 +45,44 @@ test('Records exceptions happening in middleware', async ({ request }) => { expect(await errorEventPromise).toBeDefined(); }); + +test('Should trace outgoing fetch requests inside middleware and create breadcrumbs for it', async ({ request }) => { + const middlewareTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return ( + transactionEvent?.transaction === 'middleware' && + !!transactionEvent.spans?.find(span => span.op === 'http.client') + ); + }); + + request.get('/api/endpoint-behind-middleware', { headers: { 'x-should-make-request': '1' } }); + + const middlewareTransaction = await middlewareTransactionPromise; + + expect(middlewareTransaction.spans).toEqual( + expect.arrayContaining([ + { + data: { 'http.method': 'GET', 'http.response.status_code': 200, type: 'fetch', url: 'http://localhost:3030/' }, + description: 'GET http://localhost:3030/', + op: 'http.client', + origin: 'auto.http.wintercg_fetch', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + tags: { 'http.status_code': '200' }, + timestamp: expect.any(Number), + trace_id: expect.any(String), + }, + ]), + ); + expect(middlewareTransaction.breadcrumbs).toEqual( + expect.arrayContaining([ + { + category: 'fetch', + data: { __span: expect.any(String), method: 'GET', status_code: 200, url: 'http://localhost:3030/' }, + timestamp: expect.any(Number), + type: 'http', + }, + ]), + ); +}); From cb7d1f8b1d61e9b56210f7526902a865946c0164 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 17 Nov 2023 12:49:50 +0000 Subject: [PATCH 16/16] Lil fix --- .../test-applications/nextjs-app-dir/tests/middleware.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts index 4211d7e640ff..b7bacd9d4ce4 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts @@ -54,7 +54,9 @@ test('Should trace outgoing fetch requests inside middleware and create breadcru ); }); - request.get('/api/endpoint-behind-middleware', { headers: { 'x-should-make-request': '1' } }); + request.get('/api/endpoint-behind-middleware', { headers: { 'x-should-make-request': '1' } }).catch(() => { + // Noop + }); const middlewareTransaction = await middlewareTransactionPromise;