From 8bcd1369989ec9df870846b268633b7b874cee51 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 31 Jan 2024 13:04:42 +0100 Subject: [PATCH] ref(vercel-edge): Replace `WinterCGFetch` with `winterCGFetchIntegration` (#10436) This has not been converted yet. --- packages/nextjs/src/index.types.ts | 1 + packages/vercel-edge/src/index.ts | 6 +- .../src/integrations/wintercg-fetch.ts | 154 +++++++++++------- packages/vercel-edge/src/sdk.ts | 23 ++- .../vercel-edge/test/wintercg-fetch.test.ts | 83 ++++++---- 5 files changed, 159 insertions(+), 108 deletions(-) diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index 21d51a01ee56..2328208e28c5 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -23,6 +23,7 @@ export declare function init( // eslint-disable-next-line deprecation/deprecation export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations & + // eslint-disable-next-line deprecation/deprecation typeof edgeSdk.Integrations; export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index 2ff971fde287..8937d35d38c8 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -99,12 +99,12 @@ export { import { Integrations as CoreIntegrations, RequestData } from '@sentry/core'; import { WinterCGFetch } from './integrations/wintercg-fetch'; +export { winterCGFetchIntegration } from './integrations/wintercg-fetch'; -const INTEGRATIONS = { +/** @deprecated Import the integration function directly, e.g. `inboundFiltersIntegration()` instead of `new Integrations.InboundFilter(). */ +export const Integrations = { // eslint-disable-next-line deprecation/deprecation ...CoreIntegrations, WinterCGFetch, RequestData, }; - -export { INTEGRATIONS as Integrations }; diff --git a/packages/vercel-edge/src/integrations/wintercg-fetch.ts b/packages/vercel-edge/src/integrations/wintercg-fetch.ts index 5d3d662e5b4f..507a34aedab4 100644 --- a/packages/vercel-edge/src/integrations/wintercg-fetch.ts +++ b/packages/vercel-edge/src/integrations/wintercg-fetch.ts @@ -1,14 +1,34 @@ import { instrumentFetchRequest } from '@sentry-internal/tracing'; -import { addBreadcrumb, getClient, isSentryRequestUrl } from '@sentry/core'; -import type { FetchBreadcrumbData, FetchBreadcrumbHint, HandlerDataFetch, Integration, Span } from '@sentry/types'; +import { + addBreadcrumb, + convertIntegrationFnToClass, + defineIntegration, + getClient, + isSentryRequestUrl, +} from '@sentry/core'; +import type { + Client, + FetchBreadcrumbData, + FetchBreadcrumbHint, + HandlerDataFetch, + Integration, + IntegrationClass, + IntegrationFn, + Span, +} from '@sentry/types'; import { LRUMap, addFetchInstrumentationHandler, stringMatchesSomePattern } from '@sentry/utils'; +const INTEGRATION_NAME = 'WinterCGFetch'; + +const HAS_CLIENT_MAP = new WeakMap(); + 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. @@ -16,63 +36,17 @@ export interface Options { 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; +const _winterCGFetch = ((options: Partial = {}) => { + const breadcrumbs = options.breadcrumbs === undefined ? true : options.breadcrumbs; + const shouldCreateSpanForRequest = options.shouldCreateSpanForRequest; - private readonly _createSpanUrlMap: LRUMap = new LRUMap(100); - private readonly _headersUrlMap: LRUMap = new LRUMap(100); + const _createSpanUrlMap = new LRUMap(100); + const _headersUrlMap = new LRUMap(100); - public constructor(_options: Partial = {}) { - this._options = { - breadcrumbs: _options.breadcrumbs === undefined ? true : _options.breadcrumbs, - shouldCreateSpanForRequest: _options.shouldCreateSpanForRequest, - }; - } - - /** - * @inheritDoc - */ - public setupOnce(): void { - const spans: Record = {}; - - addFetchInstrumentationHandler(handlerData => { - if (!getClient()?.getIntegrationByName?.('WinterCGFetch')) { - return; - } - - if (isSentryRequestUrl(handlerData.fetchData.url, getClient())) { - return; - } - - instrumentFetchRequest( - handlerData, - this._shouldCreateSpan.bind(this), - this._shouldAttachTraceData.bind(this), - spans, - 'auto.http.wintercg_fetch', - ); - - if (this._options.breadcrumbs) { - createBreadcrumb(handlerData); - } - }); - } + const spans: Record = {}; /** Decides whether to attach trace data to the outgoing fetch request */ - private _shouldAttachTraceData(url: string): boolean { + function _shouldAttachTraceData(url: string): boolean { const client = getClient(); if (!client) { @@ -85,32 +59,86 @@ export class WinterCGFetch implements Integration { return true; } - const cachedDecision = this._headersUrlMap.get(url); + const cachedDecision = _headersUrlMap.get(url); if (cachedDecision !== undefined) { return cachedDecision; } const decision = stringMatchesSomePattern(url, clientOptions.tracePropagationTargets); - this._headersUrlMap.set(url, decision); + _headersUrlMap.set(url, decision); return decision; } /** Helper that wraps shouldCreateSpanForRequest option */ - private _shouldCreateSpan(url: string): boolean { - if (this._options.shouldCreateSpanForRequest === undefined) { + function _shouldCreateSpan(url: string): boolean { + if (shouldCreateSpanForRequest === undefined) { return true; } - const cachedDecision = this._createSpanUrlMap.get(url); + const cachedDecision = _createSpanUrlMap.get(url); if (cachedDecision !== undefined) { return cachedDecision; } - const decision = this._options.shouldCreateSpanForRequest(url); - this._createSpanUrlMap.set(url, decision); + const decision = shouldCreateSpanForRequest(url); + _createSpanUrlMap.set(url, decision); return decision; } -} + + return { + name: INTEGRATION_NAME, + // TODO v8: Remove this again + // eslint-disable-next-line @typescript-eslint/no-empty-function + setupOnce() { + addFetchInstrumentationHandler(handlerData => { + const client = getClient(); + if (!client || !HAS_CLIENT_MAP.get(client)) { + return; + } + + if (isSentryRequestUrl(handlerData.fetchData.url, client)) { + return; + } + + instrumentFetchRequest( + handlerData, + _shouldCreateSpan, + _shouldAttachTraceData, + spans, + 'auto.http.wintercg_fetch', + ); + + if (breadcrumbs) { + createBreadcrumb(handlerData); + } + }); + }, + setup(client) { + HAS_CLIENT_MAP.set(client, true); + }, + }; +}) satisfies IntegrationFn; + +export const winterCGFetchIntegration = defineIntegration(_winterCGFetch); + +/** + * Creates spans and attaches tracing headers to fetch requests on WinterCG runtimes. + * + * @deprecated Use `winterCGFetchIntegration()` instead. + */ +// eslint-disable-next-line deprecation/deprecation +export const WinterCGFetch = convertIntegrationFnToClass( + INTEGRATION_NAME, + winterCGFetchIntegration, +) as IntegrationClass void }> & { + new (options?: { + breadcrumbs: boolean; + shouldCreateSpanForRequest?: (url: string) => boolean; + }): Integration; +}; + +// eslint-disable-next-line deprecation/deprecation +export type WinterCGFetch = typeof WinterCGFetch; function createBreadcrumb(handlerData: HandlerDataFetch): void { const { startTimestamp, endTimestamp } = handlerData; diff --git a/packages/vercel-edge/src/sdk.ts b/packages/vercel-edge/src/sdk.ts index ae263b7b01b7..fe806ccfc282 100644 --- a/packages/vercel-edge/src/sdk.ts +++ b/packages/vercel-edge/src/sdk.ts @@ -1,17 +1,17 @@ import { - FunctionToString, - InboundFilters, - LinkedErrors, - RequestData, + functionToStringIntegration, getIntegrationsToSetup, + inboundFiltersIntegration, initAndBind, + linkedErrorsIntegration, + requestDataIntegration, } from '@sentry/core'; import type { Integration, Options } from '@sentry/types'; import { GLOBAL_OBJ, createStackParser, nodeStackLineParser, stackParserFromStackParserOptions } from '@sentry/utils'; import { setAsyncLocalStorageAsyncContextStrategy } from './async'; import { VercelEdgeClient } from './client'; -import { WinterCGFetch } from './integrations/wintercg-fetch'; +import { winterCGFetchIntegration } from './integrations/wintercg-fetch'; import { makeEdgeTransport } from './transports'; import type { VercelEdgeClientOptions, VercelEdgeOptions } from './types'; import { getVercelEnv } from './utils/vercel'; @@ -24,12 +24,10 @@ const nodeStackParser = createStackParser(nodeStackLineParser()); /** @deprecated Use `getDefaultIntegrations(options)` instead. */ export const defaultIntegrations = [ - /* eslint-disable deprecation/deprecation */ - new InboundFilters(), - new FunctionToString(), - new LinkedErrors(), - /* eslint-enable deprecation/deprecation */ - new WinterCGFetch(), + inboundFiltersIntegration(), + functionToStringIntegration(), + linkedErrorsIntegration(), + winterCGFetchIntegration(), ]; /** Get the default integrations for the browser SDK. */ @@ -37,8 +35,7 @@ export function getDefaultIntegrations(options: Options): Integration[] { return [ // eslint-disable-next-line deprecation/deprecation ...defaultIntegrations, - // eslint-disable-next-line deprecation/deprecation - ...(options.sendDefaultPii ? [new RequestData()] : []), + ...(options.sendDefaultPii ? [requestDataIntegration()] : []), ]; } diff --git a/packages/vercel-edge/test/wintercg-fetch.test.ts b/packages/vercel-edge/test/wintercg-fetch.test.ts index e690e6785c79..2121f037e479 100644 --- a/packages/vercel-edge/test/wintercg-fetch.test.ts +++ b/packages/vercel-edge/test/wintercg-fetch.test.ts @@ -5,11 +5,11 @@ import * as sentryUtils from '@sentry/utils'; import { createStackParser } from '@sentry/utils'; import { VercelEdgeClient } from '../src/index'; -import { WinterCGFetch } from '../src/integrations/wintercg-fetch'; +import { winterCGFetchIntegration } from '../src/integrations/wintercg-fetch'; class FakeClient extends VercelEdgeClient { public getIntegrationByName(name: string): T | undefined { - return name === 'WinterCGFetch' ? (new WinterCGFetch() as unknown as T) : undefined; + return name === 'WinterCGFetch' ? (winterCGFetchIntegration() as Integration as T) : undefined; } } @@ -17,31 +17,34 @@ const addFetchInstrumentationHandlerSpy = jest.spyOn(sentryUtils, 'addFetchInstr const instrumentFetchRequestSpy = jest.spyOn(internalTracing, 'instrumentFetchRequest'); const addBreadcrumbSpy = jest.spyOn(sentryCore, 'addBreadcrumb'); -beforeEach(() => { - jest.clearAllMocks(); - - const client = new FakeClient({ - 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(), - }); +describe('WinterCGFetch instrumentation', () => { + let client: FakeClient; + + beforeEach(() => { + jest.clearAllMocks(); + + client = new FakeClient({ + 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, 'getClient').mockImplementation(() => client); -}); + jest.spyOn(sentryCore, 'getClient').mockImplementation(() => client); + }); -describe('WinterCGFetch instrumentation', () => { it('should call `instrumentFetchRequest` for outgoing fetch requests', () => { - const integration = new WinterCGFetch(); addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + const integration = winterCGFetchIntegration(); integration.setupOnce(); + integration.setup!(client); const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined(); @@ -70,11 +73,32 @@ describe('WinterCGFetch instrumentation', () => { expect(shouldCreateSpan('https://www.3rd-party-website.at/')).toBe(true); }); + it('should not instrument if client is not setup', () => { + addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + + const integration = winterCGFetchIntegration(); + integration.setupOnce(); + // integration.setup!(client) is not called! + + const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; + 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).not.toHaveBeenCalled(); + }); + it('should call `instrumentFetchRequest` for outgoing fetch requests to Sentry', () => { - const integration = new WinterCGFetch(); addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + const integration = winterCGFetchIntegration(); integration.setupOnce(); + integration.setup!(client); const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined(); @@ -90,14 +114,15 @@ describe('WinterCGFetch instrumentation', () => { }); it('should properly apply the `shouldCreateSpanForRequest` option', () => { - const integration = new WinterCGFetch({ + addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + + const integration = winterCGFetchIntegration({ shouldCreateSpanForRequest(url) { return url === 'http://only-acceptable-url.com/'; }, }); - addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); - integration.setupOnce(); + integration.setup!(client); const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined(); @@ -117,10 +142,11 @@ describe('WinterCGFetch instrumentation', () => { }); it('should create a breadcrumb for an outgoing request', () => { - const integration = new WinterCGFetch(); addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + const integration = winterCGFetchIntegration(); integration.setupOnce(); + integration.setup!(client); const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined(); @@ -153,12 +179,11 @@ describe('WinterCGFetch instrumentation', () => { }); it('should not create a breadcrumb for an outgoing request if `breadcrumbs: false` is set', () => { - const integration = new WinterCGFetch({ - breadcrumbs: false, - }); addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + const integration = winterCGFetchIntegration({ breadcrumbs: false }); integration.setupOnce(); + integration.setup!(client); const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined();