From c5dcb5d1687866c502d30d8187b4d98ddc0ab657 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 23 Jan 2024 14:44:14 -0500 Subject: [PATCH 01/15] feat(browser): Export functional integrations & deprecate classes (#10267) Also deprecate the `Integrations` hash in browser SDKs. I also forgot to add the integrations from `@sentry/integrations` to the migration.md doc, so adding those as well! --- MIGRATION.md | 36 +++++++++++++------ packages/astro/src/index.types.ts | 3 ++ packages/browser/src/exports.ts | 9 +++++ packages/browser/src/index.ts | 2 ++ .../browser/src/integrations/breadcrumbs.ts | 8 +++-- packages/browser/src/integrations/dedupe.ts | 11 ++++-- .../src/integrations/globalhandlers.ts | 11 ++++-- .../browser/src/integrations/httpcontext.ts | 11 ++++-- packages/browser/src/integrations/index.ts | 1 + .../browser/src/integrations/linkederrors.ts | 11 ++++-- packages/browser/src/integrations/trycatch.ts | 11 ++++-- packages/browser/src/sdk.ts | 28 ++++++++------- .../unit/integrations/breadcrumbs.test.ts | 1 + packages/deno/src/index.ts | 1 + packages/deno/src/sdk.ts | 16 ++++----- packages/nextjs/src/client/index.ts | 2 ++ packages/nextjs/src/index.types.ts | 3 ++ packages/nextjs/test/clientSdk.test.ts | 8 ++--- packages/remix/src/index.types.ts | 3 ++ packages/sveltekit/src/index.types.ts | 3 ++ 20 files changed, 125 insertions(+), 54 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 0253907e3278..4c0ea3eddc91 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -34,17 +34,31 @@ integrations from the `Integrations.XXX` hash, is deprecated in favor of using t The following list shows how integrations should be migrated: -| Old | New | Packages | -| ------------------------ | ------------------------------- | ------------------------------------------------------------------------------------------------------- | -| `new InboundFilters()` | `inboundFiltersIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | -| `new FunctionToString()` | `functionToStringIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | -| `new LinkedErrors()` | `linkedErrorsIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | -| `new ModuleMetadata()` | `moduleMetadataIntegration()` | `@sentry/core`, `@sentry/browser` | -| `new RequestData()` | `requestDataIntegration()` | `@sentry/core`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | -| `new Wasm() ` | `wasmIntegration()` | `@sentry/wasm` | -| `new Replay()` | `replayIntegration()` | `@sentry/browser` | -| `new ReplayCanvas()` | `replayCanvasIntegration()` | `@sentry/browser` | -| `new Feedback()` | `feedbackIntegration()` | `@sentry/browser` | +| Old | New | Packages | +| ------------------------- | -------------------------------- | ------------------------------------------------------------------------------------------------------- | +| `new InboundFilters()` | `inboundFiltersIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | +| `new FunctionToString()` | `functionToStringIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | +| `new LinkedErrors()` | `linkedErrorsIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | +| `new ModuleMetadata()` | `moduleMetadataIntegration()` | `@sentry/core`, `@sentry/browser` | +| `new RequestData()` | `requestDataIntegration()` | `@sentry/core`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | +| `new Wasm() ` | `wasmIntegration()` | `@sentry/wasm` | +| `new Replay()` | `replayIntegration()` | `@sentry/browser` | +| `new ReplayCanvas()` | `replayCanvasIntegration()` | `@sentry/browser` | +| `new Feedback()` | `feedbackIntegration()` | `@sentry/browser` | +| `new CaptureConsole()` | `captureConsoleIntegration()` | `@sentry/integrations` | +| `new Debug()` | `debugIntegration()` | `@sentry/integrations` | +| `new Dedupe()` | `dedupeIntegration()` | `@sentry/browser`, `@sentry/integrations`, `@sentry/deno` | +| `new ExtraErrorData()` | `extraErrorDataIntegration()` | `@sentry/integrations` | +| `new ReportingObserver()` | `reportingObserverIntegration()` | `@sentry/integrations` | +| `new RewriteFrames()` | `rewriteFramesIntegration()` | `@sentry/integrations` | +| `new SessionTiming()` | `sessionTimingIntegration()` | `@sentry/integrations` | +| `new HttpClient()` | `httpClientIntegration()` | `@sentry/integrations` | +| `new ContextLines()` | `contextLinesIntegration()` | `@sentry/browser` | +| `new Breadcrumbs()` | `breadcrumbsIntegration()` | `@sentry/browser`, `@sentry/deno` | +| `new GlobalHandlers()` | `globalHandlersIntegration()` | `@sentry/browser` | +| `new HttpContext()` | `httpContextIntegration()` | `@sentry/browser` | +| `new TryCatch()` | `browserApiErrorsIntegration()` | `@sentry/browser`, `@sentry/deno` | +| `new VueIntegration()` | `vueIntegration()` | `@sentry/vue` | ## Deprecate `hub.bindClient()` and `makeMain()` diff --git a/packages/astro/src/index.types.ts b/packages/astro/src/index.types.ts index 146d6a6e23b0..026321e8ab3d 100644 --- a/packages/astro/src/index.types.ts +++ b/packages/astro/src/index.types.ts @@ -14,8 +14,11 @@ import sentryAstro from './index.server'; export declare function init(options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions): void; // We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere. +// eslint-disable-next-line deprecation/deprecation export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations; +export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; + export declare const defaultIntegrations: Integration[]; export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 5d012bdea2b7..e9e947de8559 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -102,4 +102,13 @@ export { // eslint-disable-next-line deprecation/deprecation wrap, } from './sdk'; + +export { breadcrumbsIntegration } from './integrations/breadcrumbs'; +export { dedupeIntegration } from './integrations/dedupe'; +export { globalHandlersIntegration } from './integrations/globalhandlers'; +export { httpContextIntegration } from './integrations/httpcontext'; +export { linkedErrorsIntegration } from './integrations/linkederrors'; +export { browserApiErrorsIntegration } from './integrations/trycatch'; + +// eslint-disable-next-line deprecation/deprecation export { GlobalHandlers, TryCatch, Breadcrumbs, LinkedErrors, HttpContext, Dedupe } from './integrations'; diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 6c91e141149a..19c377fc5931 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -12,6 +12,7 @@ if (WINDOW.Sentry && WINDOW.Sentry.Integrations) { windowIntegrations = WINDOW.Sentry.Integrations; } +/** @deprecated Import the integration function directly, e.g. `inboundFiltersIntegration()` instead of `new Integrations.InboundFilter(). */ const INTEGRATIONS = { ...windowIntegrations, // eslint-disable-next-line deprecation/deprecation @@ -19,6 +20,7 @@ const INTEGRATIONS = { ...BrowserIntegrations, }; +// eslint-disable-next-line deprecation/deprecation export { INTEGRATIONS as Integrations }; export { diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts index 485b19d3b711..1cbd8321346e 100644 --- a/packages/browser/src/integrations/breadcrumbs.ts +++ b/packages/browser/src/integrations/breadcrumbs.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines */ -import { addBreadcrumb, convertIntegrationFnToClass, getClient } from '@sentry/core'; +import { addBreadcrumb, convertIntegrationFnToClass, defineIntegration, getClient } from '@sentry/core'; import type { Client, Event as SentryEvent, @@ -57,7 +57,7 @@ const MAX_ALLOWED_STRING_LENGTH = 1024; const INTEGRATION_NAME = 'Breadcrumbs'; -const breadcrumbsIntegration = ((options: Partial = {}) => { +const _breadcrumbsIntegration = ((options: Partial = {}) => { const _options = { console: true, dom: true, @@ -95,8 +95,12 @@ const breadcrumbsIntegration = ((options: Partial = {}) => { }; }) satisfies IntegrationFn; +export const breadcrumbsIntegration = defineIntegration(_breadcrumbsIntegration); + /** * Default Breadcrumbs instrumentations + * + * @deprecated Use `breadcrumbsIntegration()` instead. */ // eslint-disable-next-line deprecation/deprecation export const Breadcrumbs = convertIntegrationFnToClass(INTEGRATION_NAME, breadcrumbsIntegration) as IntegrationClass< diff --git a/packages/browser/src/integrations/dedupe.ts b/packages/browser/src/integrations/dedupe.ts index 394d5c5ae1e3..f4ace6011b19 100644 --- a/packages/browser/src/integrations/dedupe.ts +++ b/packages/browser/src/integrations/dedupe.ts @@ -1,4 +1,4 @@ -import { convertIntegrationFnToClass } from '@sentry/core'; +import { convertIntegrationFnToClass, defineIntegration } from '@sentry/core'; import type { Event, Exception, Integration, IntegrationClass, IntegrationFn, StackFrame } from '@sentry/types'; import { logger } from '@sentry/utils'; @@ -6,7 +6,7 @@ import { DEBUG_BUILD } from '../debug-build'; const INTEGRATION_NAME = 'Dedupe'; -const dedupeIntegration = (() => { +const _dedupeIntegration = (() => { let previousEvent: Event | undefined; return { @@ -33,7 +33,12 @@ const dedupeIntegration = (() => { }; }) satisfies IntegrationFn; -/** Deduplication filter */ +export const dedupeIntegration = defineIntegration(_dedupeIntegration); + +/** + * Deduplication filter. + * @deprecated Use `dedupeIntegration()` instead. + */ // eslint-disable-next-line deprecation/deprecation export const Dedupe = convertIntegrationFnToClass(INTEGRATION_NAME, dedupeIntegration) as IntegrationClass< Integration & { processEvent: (event: Event) => Event } diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index 01ef16a02851..c1096e7c2132 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ -import { captureEvent, convertIntegrationFnToClass, getClient } from '@sentry/core'; +import { captureEvent, convertIntegrationFnToClass, defineIntegration, getClient } from '@sentry/core'; import type { Client, Event, @@ -30,7 +30,7 @@ type GlobalHandlersIntegrations = Record = {}) => { +const _globalHandlersIntegration = ((options: Partial = {}) => { const _options = { onerror: true, onunhandledrejection: true, @@ -55,7 +55,12 @@ const globalHandlersIntegration = ((options: Partial }; }) satisfies IntegrationFn; -/** Global handlers */ +export const globalHandlersIntegration = defineIntegration(_globalHandlersIntegration); + +/** + * Global handlers. + * @deprecated Use `globalHandlersIntegration()` instead. + */ // eslint-disable-next-line deprecation/deprecation export const GlobalHandlers = convertIntegrationFnToClass( INTEGRATION_NAME, diff --git a/packages/browser/src/integrations/httpcontext.ts b/packages/browser/src/integrations/httpcontext.ts index 569eeffab45c..1a5e95f2eee3 100644 --- a/packages/browser/src/integrations/httpcontext.ts +++ b/packages/browser/src/integrations/httpcontext.ts @@ -1,11 +1,11 @@ -import { convertIntegrationFnToClass } from '@sentry/core'; +import { convertIntegrationFnToClass, defineIntegration } from '@sentry/core'; import type { Event, Integration, IntegrationClass, IntegrationFn } from '@sentry/types'; import { WINDOW } from '../helpers'; const INTEGRATION_NAME = 'HttpContext'; -const httpContextIntegration = (() => { +const _httpContextIntegration = (() => { return { name: INTEGRATION_NAME, // TODO v8: Remove this @@ -33,7 +33,12 @@ const httpContextIntegration = (() => { }; }) satisfies IntegrationFn; -/** HttpContext integration collects information about HTTP request headers */ +export const httpContextIntegration = defineIntegration(_httpContextIntegration); + +/** + * HttpContext integration collects information about HTTP request headers. + * @deprecated Use `httpContextIntegration()` instead. + */ // eslint-disable-next-line deprecation/deprecation export const HttpContext = convertIntegrationFnToClass(INTEGRATION_NAME, httpContextIntegration) as IntegrationClass< Integration & { preprocessEvent: (event: Event) => void } diff --git a/packages/browser/src/integrations/index.ts b/packages/browser/src/integrations/index.ts index e029422f363c..17ac85b31232 100644 --- a/packages/browser/src/integrations/index.ts +++ b/packages/browser/src/integrations/index.ts @@ -1,3 +1,4 @@ +/* eslint-disable deprecation/deprecation */ export { GlobalHandlers } from './globalhandlers'; export { TryCatch } from './trycatch'; export { Breadcrumbs } from './breadcrumbs'; diff --git a/packages/browser/src/integrations/linkederrors.ts b/packages/browser/src/integrations/linkederrors.ts index 8a166e7667d9..b03855303c58 100644 --- a/packages/browser/src/integrations/linkederrors.ts +++ b/packages/browser/src/integrations/linkederrors.ts @@ -1,4 +1,4 @@ -import { convertIntegrationFnToClass } from '@sentry/core'; +import { convertIntegrationFnToClass, defineIntegration } from '@sentry/core'; import type { Client, Event, EventHint, Integration, IntegrationClass, IntegrationFn } from '@sentry/types'; import { applyAggregateErrorsToEvent } from '@sentry/utils'; import { exceptionFromError } from '../eventbuilder'; @@ -13,7 +13,7 @@ const DEFAULT_LIMIT = 5; const INTEGRATION_NAME = 'LinkedErrors'; -const linkedErrorsIntegration = ((options: LinkedErrorsOptions = {}) => { +const _linkedErrorsIntegration = ((options: LinkedErrorsOptions = {}) => { const limit = options.limit || DEFAULT_LIMIT; const key = options.key || DEFAULT_KEY; @@ -38,7 +38,12 @@ const linkedErrorsIntegration = ((options: LinkedErrorsOptions = {}) => { }; }) satisfies IntegrationFn; -/** Aggregrate linked errors in an event. */ +export const linkedErrorsIntegration = defineIntegration(_linkedErrorsIntegration); + +/** + * Aggregrate linked errors in an event. + * @deprecated Use `linkedErrorsIntegration()` instead. + */ // eslint-disable-next-line deprecation/deprecation export const LinkedErrors = convertIntegrationFnToClass(INTEGRATION_NAME, linkedErrorsIntegration) as IntegrationClass< Integration & { preprocessEvent: (event: Event, hint: EventHint, client: Client) => void } diff --git a/packages/browser/src/integrations/trycatch.ts b/packages/browser/src/integrations/trycatch.ts index 2f5f06592805..bd7c50331022 100644 --- a/packages/browser/src/integrations/trycatch.ts +++ b/packages/browser/src/integrations/trycatch.ts @@ -1,4 +1,4 @@ -import { convertIntegrationFnToClass } from '@sentry/core'; +import { convertIntegrationFnToClass, defineIntegration } from '@sentry/core'; import type { Integration, IntegrationClass, IntegrationFn, WrappedFunction } from '@sentry/types'; import { fill, getFunctionName, getOriginalFunction } from '@sentry/utils'; @@ -50,7 +50,7 @@ interface TryCatchOptions { eventTarget: boolean | string[]; } -const browserApiErrorsIntegration = ((options: Partial = {}) => { +const _browserApiErrorsIntegration = ((options: Partial = {}) => { const _options = { XMLHttpRequest: true, eventTarget: true, @@ -90,7 +90,12 @@ const browserApiErrorsIntegration = ((options: Partial = {}) => }; }) satisfies IntegrationFn; -/** Wrap timer functions and event targets to catch errors and provide better meta data */ +export const browserApiErrorsIntegration = defineIntegration(_browserApiErrorsIntegration); + +/** + * Wrap timer functions and event targets to catch errors and provide better meta data. + * @deprecated Use `browserApiErrorsIntegration()` instead. + */ // eslint-disable-next-line deprecation/deprecation export const TryCatch = convertIntegrationFnToClass( INTEGRATION_NAME, diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 6332cdac9d4b..1c57f534867c 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -1,7 +1,6 @@ import type { Hub } from '@sentry/core'; +import { functionToStringIntegration, inboundFiltersIntegration } from '@sentry/core'; import { - FunctionToString, - InboundFilters, captureSession, getClient, getCurrentHub, @@ -23,22 +22,25 @@ import { BrowserClient } from './client'; import { DEBUG_BUILD } from './debug-build'; import type { ReportDialogOptions } from './helpers'; import { WINDOW, wrap as internalWrap } from './helpers'; -import { Breadcrumbs, Dedupe, GlobalHandlers, HttpContext, LinkedErrors, TryCatch } from './integrations'; +import { breadcrumbsIntegration } from './integrations/breadcrumbs'; +import { dedupeIntegration } from './integrations/dedupe'; +import { globalHandlersIntegration } from './integrations/globalhandlers'; +import { httpContextIntegration } from './integrations/httpcontext'; +import { linkedErrorsIntegration } from './integrations/linkederrors'; +import { browserApiErrorsIntegration } from './integrations/trycatch'; import { defaultStackParser } from './stack-parsers'; import { makeFetchTransport, makeXHRTransport } from './transports'; /** @deprecated Use `getDefaultIntegrations(options)` instead. */ export const defaultIntegrations = [ - /* eslint-disable deprecation/deprecation */ - new InboundFilters(), - new FunctionToString(), - /* eslint-enable deprecation/deprecation */ - new TryCatch(), - new Breadcrumbs(), - new GlobalHandlers(), - new LinkedErrors(), - new Dedupe(), - new HttpContext(), + inboundFiltersIntegration(), + functionToStringIntegration(), + browserApiErrorsIntegration(), + breadcrumbsIntegration(), + globalHandlersIntegration(), + linkedErrorsIntegration(), + dedupeIntegration(), + httpContextIntegration(), ]; /** Get the default integrations for the browser SDK. */ diff --git a/packages/browser/test/unit/integrations/breadcrumbs.test.ts b/packages/browser/test/unit/integrations/breadcrumbs.test.ts index c2aea2ee170f..28764c2cddfa 100644 --- a/packages/browser/test/unit/integrations/breadcrumbs.test.ts +++ b/packages/browser/test/unit/integrations/breadcrumbs.test.ts @@ -8,6 +8,7 @@ describe('Breadcrumbs', () => { const client = new BrowserClient({ ...getDefaultBrowserClientOptions(), dsn: 'https://username@domain/123', + // eslint-disable-next-line deprecation/deprecation integrations: [new Breadcrumbs()], }); diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index 7adfcae9a894..65d82a0e6779 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -94,6 +94,7 @@ export { init, } from './sdk'; +export { breadcrumbsIntegration, dedupeIntegration } from '@sentry/browser'; import { Integrations as CoreIntegrations } from '@sentry/core'; import * as DenoIntegrations from './integrations'; diff --git a/packages/deno/src/sdk.ts b/packages/deno/src/sdk.ts index 81834a8b0081..990eb8146039 100644 --- a/packages/deno/src/sdk.ts +++ b/packages/deno/src/sdk.ts @@ -1,6 +1,6 @@ -import { Breadcrumbs, Dedupe } from '@sentry/browser'; +import { breadcrumbsIntegration, dedupeIntegration } from '@sentry/browser'; import type { ServerRuntimeClientOptions } from '@sentry/core'; -import { FunctionToString, InboundFilters, LinkedErrors } from '@sentry/core'; +import { functionToStringIntegration, inboundFiltersIntegration, linkedErrorsIntegration } from '@sentry/core'; import { getIntegrationsToSetup, initAndBind } from '@sentry/core'; import type { Integration, Options, StackParser } from '@sentry/types'; import { createStackParser, nodeStackLineParser, stackParserFromStackParserOptions } from '@sentry/utils'; @@ -12,15 +12,13 @@ import type { DenoOptions } from './types'; /** @deprecated Use `getDefaultIntegrations(options)` instead. */ export const defaultIntegrations = [ - /* eslint-disable deprecation/deprecation */ // Common - new InboundFilters(), - new FunctionToString(), - new LinkedErrors(), - /* eslint-enable deprecation/deprecation */ + inboundFiltersIntegration(), + functionToStringIntegration(), + linkedErrorsIntegration(), // From Browser - new Dedupe(), - new Breadcrumbs({ + dedupeIntegration(), + breadcrumbsIntegration({ dom: false, history: false, xhr: false, diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index 14c788a44307..d1d5e1db7ff5 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -18,7 +18,9 @@ export * from '@sentry/react'; export { nextRouterInstrumentation } from './routing/nextRoutingInstrumentation'; export { captureUnderscoreErrorException } from '../common/_error'; +/** @deprecated Import the integration function directly, e.g. `inboundFiltersIntegration()` instead of `new Integrations.InboundFilter(). */ export const Integrations = { + // eslint-disable-next-line deprecation/deprecation ...OriginalIntegrations, BrowserTracing, }; diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index b15bb6c40bab..21d51a01ee56 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -20,10 +20,13 @@ export declare function init( ): void; // We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere. +// eslint-disable-next-line deprecation/deprecation export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations & typeof edgeSdk.Integrations; +export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; + export declare const defaultIntegrations: Integration[]; export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts index d0f0de959170..464b7db14dc7 100644 --- a/packages/nextjs/test/clientSdk.test.ts +++ b/packages/nextjs/test/clientSdk.test.ts @@ -6,7 +6,7 @@ import type { Integration } from '@sentry/types'; import { logger } from '@sentry/utils'; import { JSDOM } from 'jsdom'; -import { BrowserTracing, Integrations, init, nextRouterInstrumentation } from '../src/client'; +import { BrowserTracing, breadcrumbsIntegration, init, nextRouterInstrumentation } from '../src/client'; const reactInit = jest.spyOn(SentryReact, 'init'); const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent'); @@ -108,12 +108,12 @@ describe('Client init()', () => { type ModifiedInitOptionsIntegrationArray = { defaultIntegrations: Integration[]; integrations: Integration[] }; it('supports passing unrelated integrations through options', () => { - init({ integrations: [new Integrations.Breadcrumbs({ console: false })] }); + init({ integrations: [breadcrumbsIntegration({ console: false })] }); const reactInitOptions = reactInit.mock.calls[0][0] as ModifiedInitOptionsIntegrationArray; - const breadcrumbsIntegration = findIntegrationByName(reactInitOptions.integrations, 'Breadcrumbs'); + const installedBreadcrumbsIntegration = findIntegrationByName(reactInitOptions.integrations, 'Breadcrumbs'); - expect(breadcrumbsIntegration).toBeDefined(); + expect(installedBreadcrumbsIntegration).toBeDefined(); }); describe('`BrowserTracing` integration', () => { diff --git a/packages/remix/src/index.types.ts b/packages/remix/src/index.types.ts index 1a7c7fb847e5..0abe77c7a20d 100644 --- a/packages/remix/src/index.types.ts +++ b/packages/remix/src/index.types.ts @@ -13,8 +13,11 @@ import type { RemixOptions } from './utils/remixOptions'; export declare function init(options: RemixOptions): void; // We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere. +// eslint-disable-next-line deprecation/deprecation export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations; +export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; + export declare const defaultIntegrations: Integration[]; export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index 3e57a08c2538..6a5d3e3883e9 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/packages/sveltekit/src/index.types.ts @@ -37,8 +37,11 @@ export declare function handleErrorWithSentry any>(origLoad: T): T; // We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere. +// eslint-disable-next-line deprecation/deprecation export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations; +export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; + export declare const defaultIntegrations: Integration[]; export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; From 6fe349fb672bb5c065d040a62c4f282cdd2c6b52 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 23 Jan 2024 20:38:45 +0000 Subject: [PATCH 02/15] feat(deps): bump @sentry/cli from 2.25.0 to 2.26.0 (#10287) --- yarn.lock | 88 +++++++++++++++++++++++++++---------------------------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/yarn.lock b/yarn.lock index c02f5da0ff3c..2a082acd3aa5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5292,40 +5292,40 @@ magic-string "0.27.0" unplugin "1.0.1" -"@sentry/cli-darwin@2.25.0": - version "2.25.0" - resolved "https://registry.yarnpkg.com/@sentry/cli-darwin/-/cli-darwin-2.25.0.tgz#a46c84852fbecdbd16948548f4c58302cb5471b9" - integrity sha512-OgBioypi9S+cooC4mPj/gYyvjw3oP9TH9ACgzobL0oP9gCpyF36iv044SWHLgeFUb45cPpVZ7f7WeSbufItzCQ== - -"@sentry/cli-linux-arm64@2.25.0": - version "2.25.0" - resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm64/-/cli-linux-arm64-2.25.0.tgz#dcf61eac6adc6dcc5aee2eaebff2e901370abc75" - integrity sha512-GqxP3s0qHBgch3WI1my5P/h4YeEtNEar+jOGTPg66Bt042rUEHIlYuhULriu3v5rLnmlTuQ5i+LGr4Kq5SFW0Q== - -"@sentry/cli-linux-arm@2.25.0": - version "2.25.0" - resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm/-/cli-linux-arm-2.25.0.tgz#bbea30b44b6b37c7e58d5e47b393d1139cdfe5ef" - integrity sha512-EZT//Dnajc03juqBTRUlU7x/1R1ODq5w6ZC9zO5tJfURxljUJ/kkAScHpfHiqzhPMNArK0gu7vYrOS4CTA7eBw== - -"@sentry/cli-linux-i686@2.25.0": - version "2.25.0" - resolved "https://registry.yarnpkg.com/@sentry/cli-linux-i686/-/cli-linux-i686-2.25.0.tgz#b11c4bc6679c253dbd0e000a92198f3ccb41e53c" - integrity sha512-w25QuABMK7FDjlOgpWgJOhQdVQguOhz81DPoeXNWiDLcTHFsYDXxT88exaUQxrLhMNcRrQnS0rDhwd5y0cYh0g== - -"@sentry/cli-linux-x64@2.25.0": - version "2.25.0" - resolved "https://registry.yarnpkg.com/@sentry/cli-linux-x64/-/cli-linux-x64-2.25.0.tgz#c2c3be3db81ee08185557fa502ecfb5e516995ab" - integrity sha512-7Pr3JZTPWqSeLiG67v/7uR9prpCfNAW2naf/SSZOMg2ZTXSgG+kgXf6/ADI3WP1LtF3GVhexGtJ5eyFVYxfLsQ== - -"@sentry/cli-win32-i686@2.25.0": - version "2.25.0" - resolved "https://registry.yarnpkg.com/@sentry/cli-win32-i686/-/cli-win32-i686-2.25.0.tgz#968f5093e3a74401fe15233808aee68311d15496" - integrity sha512-AuHBpFB2DZr19KE3g7qejaVmGb0d7E4ZN2cBKX1Vixb+KTi9/bEcRrWaQ2PpqLTVb2Wwglf/VlZgsxOjfhp7Ag== - -"@sentry/cli-win32-x64@2.25.0": - version "2.25.0" - resolved "https://registry.yarnpkg.com/@sentry/cli-win32-x64/-/cli-win32-x64-2.25.0.tgz#9bb6b271642e1cc2b571acd395bece2683a3faed" - integrity sha512-EFGg2L4Wm8YNRV/yAy0bmztc2jkkhy0SfaQtxrHW22IRqfl2jXyKcHHmcEjwoYfCvIW+c5I0ftHhjueMuuRcXw== +"@sentry/cli-darwin@2.26.0": + version "2.26.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-darwin/-/cli-darwin-2.26.0.tgz#6b44500dd549415c5f8b7228a3f4aef18a2e6766" + integrity sha512-SJ4ts9VELoLdOx1g034Tv2nGqhjutBYNAI3WMsjBaQG3tqNPJkQJKGrOqfpL6kTdO2tqQIAYeVw60yqWuHU3FA== + +"@sentry/cli-linux-arm64@2.26.0": + version "2.26.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm64/-/cli-linux-arm64-2.26.0.tgz#dfe28a7caeffd8bd68476b709ba5f5f50fa74244" + integrity sha512-tAsK5pWrLyU+zqoW0uwylfLB7udOV8FtU8xqcfMsYGxt44zviiuxzKeDnaUdHsZcvk03aTAyf1Dxqn0u32A0MA== + +"@sentry/cli-linux-arm@2.26.0": + version "2.26.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm/-/cli-linux-arm-2.26.0.tgz#cae4cf7db31a0cb4dba5243fce826abfa71dc404" + integrity sha512-qNqKLf3eGowhm+4gg47jGLfova5SLgC0wvWX181U+w94oVGp4onuSjbqpy7wbSA9nsfTXllMhEFI5jA4CMmZVw== + +"@sentry/cli-linux-i686@2.26.0": + version "2.26.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-i686/-/cli-linux-i686-2.26.0.tgz#29cd6617d2e764dec4cff377ede58762b87e9a66" + integrity sha512-+dSFR9rK6o6F0gBxoU0mrHw18qVgF1t27Y0jvdItMtDuCuduBuXTffmsbBwbPFWBgWuLPG+ojB1LuoBt5qVMng== + +"@sentry/cli-linux-x64@2.26.0": + version "2.26.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-x64/-/cli-linux-x64-2.26.0.tgz#a54dbcdd5c377ba97f7d3cc8115f25e68c3aded4" + integrity sha512-oY86ECWVQuk434K+enUVZnn28T8qxjJTpxN079xvz7SIWOxQ609tMva91Ywo0gExcu07AZ0pg71XFsEQ9WhZgA== + +"@sentry/cli-win32-i686@2.26.0": + version "2.26.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-win32-i686/-/cli-win32-i686-2.26.0.tgz#42fdf7006e9e420cb0b3e6b70d9f44f6807906e9" + integrity sha512-vLju9NFl4venKEVpuFJpxaCwa2NdG6C9mhYNqxRvZAPrXWMdMd697qBDOMepAPT7CI8EWiyXUwMli0WjGXGMeQ== + +"@sentry/cli-win32-x64@2.26.0": + version "2.26.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-win32-x64/-/cli-win32-x64-2.26.0.tgz#805cb184652d4a608128b70f9e19c1117f25c6f6" + integrity sha512-r3ZaxdHGC6OyJhOxO5ADzAitpGcgT/PkqQzOzKXBOebHj5jzwY27JWjdNhpT6sJZDII13HxqwISRedVWftZgRw== "@sentry/cli@^1.74.4", "@sentry/cli@^1.77.1": version "1.77.1" @@ -5340,9 +5340,9 @@ which "^2.0.2" "@sentry/cli@^2.17.0", "@sentry/cli@^2.21.2", "@sentry/cli@^2.23.0": - version "2.25.0" - resolved "https://registry.yarnpkg.com/@sentry/cli/-/cli-2.25.0.tgz#2b142b763d21def99a3473f0cd3169f6b04f6235" - integrity sha512-N7k3NdiiEyQkQ43hRDAVqMf+Lg3GTWevO+ndg4yZ8Zv+J1jEVD6ZbqNnshSwWOx9qzcWQ+V/8ZgjmNuHbcNRxg== + version "2.26.0" + resolved "https://registry.yarnpkg.com/@sentry/cli/-/cli-2.26.0.tgz#94cee60c89f457318540f74f8f1158357c2dd706" + integrity sha512-WRrY9nkjLLUvyo+l8KE0x0Q+0NtCd2U8HYJzh3kyJHyyfKWiSH7ZhExcsb2MoSIjlzbKjjrIJzxhklZABkidDw== dependencies: https-proxy-agent "^5.0.0" node-fetch "^2.6.7" @@ -5350,13 +5350,13 @@ proxy-from-env "^1.1.0" which "^2.0.2" optionalDependencies: - "@sentry/cli-darwin" "2.25.0" - "@sentry/cli-linux-arm" "2.25.0" - "@sentry/cli-linux-arm64" "2.25.0" - "@sentry/cli-linux-i686" "2.25.0" - "@sentry/cli-linux-x64" "2.25.0" - "@sentry/cli-win32-i686" "2.25.0" - "@sentry/cli-win32-x64" "2.25.0" + "@sentry/cli-darwin" "2.26.0" + "@sentry/cli-linux-arm" "2.26.0" + "@sentry/cli-linux-arm64" "2.26.0" + "@sentry/cli-linux-i686" "2.26.0" + "@sentry/cli-linux-x64" "2.26.0" + "@sentry/cli-win32-i686" "2.26.0" + "@sentry/cli-win32-x64" "2.26.0" "@sentry/vite-plugin@^0.6.1": version "0.6.1" From 3b4650c5a876c255c217b8d1defe1577539826ae Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 24 Jan 2024 09:04:02 -0500 Subject: [PATCH 03/15] feat(tracing): Deprecate transaction-related options for `BrowserTracing` (#10303) These are mostly naming changes to use `span` instead of `transaction` for options. Missing is a new handling for `routingInstrumentation`, which I will tackle in a separate PR. --- MIGRATION.md | 10 + packages/core/src/tracing/trace.ts | 113 +----------- .../src/browser/browsertracing.ts | 120 +++++++++--- .../test/browser/browsertracing.test.ts | 174 ++++++++++++------ packages/types/src/index.ts | 1 + packages/types/src/startSpanOptions.ts | 104 +++++++++++ 6 files changed, 326 insertions(+), 196 deletions(-) create mode 100644 packages/types/src/startSpanOptions.ts diff --git a/MIGRATION.md b/MIGRATION.md index 4c0ea3eddc91..241697d54586 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -10,6 +10,16 @@ npx @sentry/migr8@latest This will let you select which updates to run, and automatically update your code. Make sure to still review all code changes! +## Deprecate transaction-related options to `BrowserTracing` + +When configuring the `BrowserTracing` integration, some options have been renamed: + +- `startTransactionOnPageLoad` --> `spanOnPageLoad` +- `startTransactionOnLocationChange` --> `spanOnLocationChange` +- `markBackgroundTransactions` --> `markBackgroundSpan` + +Also, `beforeNavigate` is replaced with a `beforeStartSpan` callback, which receives `StartSpanOptions`. + ## Deprecate using `getClient()` to check if the SDK was initialized In v8, `getClient()` will stop returning `undefined` if `Sentry.init()` was not called. For cases where this may be used diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index eb070510a3b7..885cbd7c9d08 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -1,15 +1,5 @@ -import type { - Instrumenter, - Primitive, - Scope, - Span, - SpanTimeInput, - TransactionContext, - TransactionMetadata, -} from '@sentry/types'; -import type { SpanAttributes } from '@sentry/types'; -import type { SpanOrigin } from '@sentry/types'; -import type { TransactionSource } from '@sentry/types'; +import type { Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types'; + import { dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; @@ -20,105 +10,6 @@ import { handleCallbackErrors } from '../utils/handleCallbackErrors'; import { hasTracingEnabled } from '../utils/hasTracingEnabled'; import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; -interface StartSpanOptions extends TransactionContext { - /** A manually specified start time for the created `Span` object. */ - startTime?: SpanTimeInput; - - /** If defined, start this span off this scope instead off the current scope. */ - scope?: Scope; - - /** The name of the span. */ - name: string; - - /** An op for the span. This is a categorization for spans. */ - op?: string; - - /** The origin of the span - if it comes from auto instrumenation or manual instrumentation. */ - origin?: SpanOrigin; - - /** Attributes for the span. */ - attributes?: SpanAttributes; - - // All remaining fields are deprecated - - /** - * @deprecated Manually set the end timestamp instead. - */ - trimEnd?: boolean; - - /** - * @deprecated This cannot be set manually anymore. - */ - parentSampled?: boolean; - - /** - * @deprecated Use attributes or set data on scopes instead. - */ - metadata?: Partial; - - /** - * The name thingy. - * @deprecated Use `name` instead. - */ - description?: string; - - /** - * @deprecated Use `span.setStatus()` instead. - */ - status?: string; - - /** - * @deprecated Use `scope` instead. - */ - parentSpanId?: string; - - /** - * @deprecated You cannot manually set the span to sampled anymore. - */ - sampled?: boolean; - - /** - * @deprecated You cannot manually set the spanId anymore. - */ - spanId?: string; - - /** - * @deprecated You cannot manually set the traceId anymore. - */ - traceId?: string; - - /** - * @deprecated Use an attribute instead. - */ - source?: TransactionSource; - - /** - * @deprecated Use attributes or set tags on the scope instead. - */ - tags?: { [key: string]: Primitive }; - - /** - * @deprecated Use attributes instead. - */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - data?: { [key: string]: any }; - - /** - * @deprecated Use `startTime` instead. - */ - startTimestamp?: number; - - /** - * @deprecated Use `span.end()` instead. - */ - endTimestamp?: number; - - /** - * @deprecated You cannot set the instrumenter manually anymore. - */ - instrumenter?: Instrumenter; -} - /** * Wraps a function with a transaction/span and finishes the span after the function is done. * diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index e9f61c73c0f3..a2d89b5c7c28 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -1,4 +1,4 @@ -/* eslint-disable max-lines */ +/* eslint-disable max-lines, complexity */ import type { Hub, IdleTransaction } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, @@ -9,7 +9,14 @@ import { spanToJSON, startIdleTransaction, } from '@sentry/core'; -import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; +import type { + EventProcessor, + Integration, + StartSpanOptions, + Transaction, + TransactionContext, + TransactionSource, +} from '@sentry/types'; import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; import { DEBUG_BUILD } from '../common/debug-build'; @@ -60,27 +67,48 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { heartbeatInterval: number; /** - * Flag to enable/disable creation of `navigation` transaction on history changes. + * If a span should be created on location (history) changes. + * Default: true + */ + spanOnLocationChange: boolean; + + /** + * If a span should be created on pageload. + * Default: true + */ + spanOnPageLoad: boolean; + + /** + * Flag spans where tabs moved to background with "cancelled". Browser background tab timing is + * not suited towards doing precise measurements of operations. By default, we recommend that this option + * be enabled as background transactions can mess up your statistics in nondeterministic ways. * * Default: true */ - startTransactionOnLocationChange: boolean; + markBackgroundSpan: boolean; + + /** + * Flag to enable/disable creation of `navigation` transaction on history changes. + * Default: true + * @deprecated Configure `spanOnLocationChange` instead. + */ + startTransactionOnLocationChange?: boolean; /** * Flag to enable/disable creation of `pageload` transaction on first pageload. - * * Default: true + * @deprecated Configure `spanOnPageLoad` instead. */ - startTransactionOnPageLoad: boolean; + startTransactionOnPageLoad?: boolean; /** * Flag Transactions where tabs moved to background with "cancelled". Browser background tab timing is * not suited towards doing precise measurements of operations. By default, we recommend that this option * be enabled as background transactions can mess up your statistics in nondeterministic ways. - * * Default: true + * @deprecated Configure `markBackgroundSpan` instead. */ - markBackgroundTransactions: boolean; + markBackgroundTransactions?: boolean; /** * If true, Sentry will capture long tasks and add them to the corresponding transaction. @@ -117,6 +145,12 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { onStartRouteTransaction: (t: Transaction | undefined, ctx: TransactionContext, getCurrentHub: () => Hub) => void; }>; + /** + * A callback which is called before a span for a pageload or navigation is started. + * It receives the options passed to `startSpan`, and expects to return an updated options object. + */ + beforeStartSpan?: (options: StartSpanOptions) => StartSpanOptions; + /** * beforeNavigate is called before a pageload/navigation transaction is created and allows users to modify transaction * context data, or drop the transaction entirely (by setting `sampled = false` in the context). @@ -126,6 +160,8 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { * @param context: The context data which will be passed to `startTransaction` by default * * @returns A (potentially) modified context object, with `sampled = false` if the transaction should be dropped. + * + * @deprecated Use `beforeStartSpan` instead. */ beforeNavigate?(this: void, context: TransactionContext): TransactionContext | undefined; @@ -143,10 +179,10 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { ...TRACING_DEFAULTS, - markBackgroundTransactions: true, routingInstrumentation: instrumentRoutingWithDefaults, - startTransactionOnLocationChange: true, - startTransactionOnPageLoad: true, + spanOnLocationChange: true, + spanOnPageLoad: true, + markBackgroundSpan: true, enableLongTask: true, _experiments: {}, ...defaultRequestInstrumentationOptions, @@ -182,7 +218,7 @@ export class BrowserTracing implements Integration { private _hasSetTracePropagationTargets: boolean; - public constructor(_options?: Partial) { + public constructor(_options: Partial = {}) { this.name = BROWSER_TRACING_INTEGRATION_ID; this._hasSetTracePropagationTargets = false; @@ -190,12 +226,34 @@ export class BrowserTracing implements Integration { if (DEBUG_BUILD) { this._hasSetTracePropagationTargets = !!( - _options && // eslint-disable-next-line deprecation/deprecation (_options.tracePropagationTargets || _options.tracingOrigins) ); } + // Migrate legacy options + // TODO v8: Remove this + /* eslint-disable deprecation/deprecation */ + if (typeof _options.startTransactionOnPageLoad === 'boolean') { + _options.spanOnPageLoad = _options.startTransactionOnPageLoad; + } + if (typeof _options.startTransactionOnLocationChange === 'boolean') { + _options.spanOnLocationChange = _options.startTransactionOnLocationChange; + } + if (typeof _options.markBackgroundTransactions === 'boolean') { + _options.markBackgroundSpan = _options.markBackgroundTransactions; + } + /* eslint-enable deprecation/deprecation */ + + // TODO (v8): remove this block after tracingOrigins is removed + // Set tracePropagationTargets to tracingOrigins if specified by the user + // In case both are specified, tracePropagationTargets takes precedence + // eslint-disable-next-line deprecation/deprecation + if (!_options.tracePropagationTargets && _options.tracingOrigins) { + // eslint-disable-next-line deprecation/deprecation + _options.tracePropagationTargets = _options.tracingOrigins; + } + this.options = { ...DEFAULT_BROWSER_TRACING_OPTIONS, ..._options, @@ -207,15 +265,6 @@ export class BrowserTracing implements Integration { this.options.enableLongTask = this.options._experiments.enableLongTask; } - // TODO (v8): remove this block after tracingOrigins is removed - // Set tracePropagationTargets to tracingOrigins if specified by the user - // In case both are specified, tracePropagationTargets takes precedence - // eslint-disable-next-line deprecation/deprecation - if (_options && !_options.tracePropagationTargets && _options.tracingOrigins) { - // eslint-disable-next-line deprecation/deprecation - this.options.tracePropagationTargets = _options.tracingOrigins; - } - this._collectWebVitals = startTrackingWebVitals(); if (this.options.enableLongTask) { startTrackingLongTasks(); @@ -237,9 +286,9 @@ export class BrowserTracing implements Integration { const { routingInstrumentation: instrumentRouting, - startTransactionOnLocationChange, - startTransactionOnPageLoad, - markBackgroundTransactions, + spanOnPageLoad, + spanOnLocationChange, + markBackgroundSpan, traceFetch, traceXHR, shouldCreateSpanForRequest, @@ -275,11 +324,11 @@ export class BrowserTracing implements Integration { return transaction; }, - startTransactionOnPageLoad, - startTransactionOnLocationChange, + spanOnPageLoad, + spanOnLocationChange, ); - if (markBackgroundTransactions) { + if (markBackgroundSpan) { registerBackgroundTabDetection(); } @@ -306,7 +355,14 @@ export class BrowserTracing implements Integration { const hub = this._getCurrentHub(); - const { beforeNavigate, idleTimeout, finalTimeout, heartbeatInterval } = this.options; + const { + // eslint-disable-next-line deprecation/deprecation + beforeNavigate, + beforeStartSpan, + idleTimeout, + finalTimeout, + heartbeatInterval, + } = this.options; const isPageloadTransaction = context.op === 'pageload'; @@ -328,7 +384,11 @@ export class BrowserTracing implements Integration { trimEnd: true, }; - const modifiedContext = typeof beforeNavigate === 'function' ? beforeNavigate(expandedContext) : expandedContext; + const contextAfterProcessing = beforeStartSpan ? beforeStartSpan(expandedContext) : expandedContext; + + const modifiedContext = + // eslint-disable-next-line deprecation/deprecation + typeof beforeNavigate === 'function' ? beforeNavigate(contextAfterProcessing) : contextAfterProcessing; // For backwards compatibility reasons, beforeNavigate can return undefined to "drop" the transaction (prevent it // from being sent to Sentry). diff --git a/packages/tracing-internal/test/browser/browsertracing.test.ts b/packages/tracing-internal/test/browser/browsertracing.test.ts index b9830b8d754c..0892169c28a2 100644 --- a/packages/tracing-internal/test/browser/browsertracing.test.ts +++ b/packages/tracing-internal/test/browser/browsertracing.test.ts @@ -85,61 +85,6 @@ conditionalTest({ min: 10 })('BrowserTracing', () => { return instance; } - // These are important enough to check with a test as incorrect defaults could - // break a lot of users' configurations. - it('is created with default settings', () => { - const browserTracing = createBrowserTracing(); - - expect(browserTracing.options).toEqual({ - enableLongTask: true, - _experiments: {}, - ...TRACING_DEFAULTS, - markBackgroundTransactions: true, - routingInstrumentation: instrumentRoutingWithDefaults, - startTransactionOnLocationChange: true, - startTransactionOnPageLoad: true, - ...defaultRequestInstrumentationOptions, - }); - }); - - it('is allows to disable enableLongTask via _experiments', () => { - const browserTracing = createBrowserTracing(false, { - _experiments: { - enableLongTask: false, - }, - }); - - expect(browserTracing.options).toEqual({ - enableLongTask: false, - ...TRACING_DEFAULTS, - markBackgroundTransactions: true, - routingInstrumentation: instrumentRoutingWithDefaults, - startTransactionOnLocationChange: true, - startTransactionOnPageLoad: true, - ...defaultRequestInstrumentationOptions, - _experiments: { - enableLongTask: false, - }, - }); - }); - - it('is allows to disable enableLongTask', () => { - const browserTracing = createBrowserTracing(false, { - enableLongTask: false, - }); - - expect(browserTracing.options).toEqual({ - enableLongTask: false, - _experiments: {}, - ...TRACING_DEFAULTS, - markBackgroundTransactions: true, - routingInstrumentation: instrumentRoutingWithDefaults, - startTransactionOnLocationChange: true, - startTransactionOnPageLoad: true, - ...defaultRequestInstrumentationOptions, - }); - }); - /** * All of these tests under `describe('route transaction')` are tested with * `browserTracing.options = { routingInstrumentation: customInstrumentRouting }`, @@ -387,6 +332,71 @@ conditionalTest({ min: 10 })('BrowserTracing', () => { }); }); + describe('beforeStartSpan', () => { + it('is called on transaction creation', () => { + const beforeStartSpan = jest.fn().mockReturnValue({ name: 'here/is/my/path' }); + createBrowserTracing(true, { + beforeStartSpan, + routingInstrumentation: customInstrumentRouting, + }); + const transaction = getActiveTransaction(hub) as IdleTransaction; + expect(transaction).toBeDefined(); + + expect(beforeStartSpan).toHaveBeenCalledTimes(1); + }); + + it('can override default context values', () => { + const beforeStartSpan = jest.fn(ctx => ({ + ...ctx, + op: 'not-pageload', + })); + createBrowserTracing(true, { + beforeStartSpan, + routingInstrumentation: customInstrumentRouting, + }); + const transaction = getActiveTransaction(hub) as IdleTransaction; + expect(transaction).toBeDefined(); + expect(transaction.op).toBe('not-pageload'); + + expect(beforeStartSpan).toHaveBeenCalledTimes(1); + }); + + it("sets transaction name source to `'custom'` if name is changed", () => { + const beforeStartSpan = jest.fn(ctx => ({ + ...ctx, + name: 'newName', + })); + createBrowserTracing(true, { + beforeStartSpan, + routingInstrumentation: customInstrumentRouting, + }); + const transaction = getActiveTransaction(hub) as IdleTransaction; + expect(transaction).toBeDefined(); + expect(transaction.name).toBe('newName'); + expect(transaction.metadata.source).toBe('custom'); + + expect(beforeStartSpan).toHaveBeenCalledTimes(1); + }); + + it('sets transaction name source to default `url` if name is not changed', () => { + const beforeStartSpan = jest.fn(ctx => ({ + ...ctx, + })); + createBrowserTracing(true, { + beforeStartSpan, + routingInstrumentation: (customStartTransaction: (obj: any) => void) => { + customStartTransaction({ name: 'a/path', op: 'pageload', metadata: { source: 'url' } }); + }, + }); + const transaction = getActiveTransaction(hub) as IdleTransaction; + expect(transaction).toBeDefined(); + expect(transaction.name).toBe('a/path'); + expect(transaction.metadata.source).toBe('url'); + + expect(beforeStartSpan).toHaveBeenCalledTimes(1); + }); + }); + it('sets transaction context from sentry-trace header', () => { const name = 'sentry-trace'; const content = '126de09502ae4e0fb26c6967190756a4-b6e54397b12a2a0f-1'; @@ -699,4 +709,58 @@ conditionalTest({ min: 10 })('BrowserTracing', () => { ); }); }); + + describe('options', () => { + // These are important enough to check with a test as incorrect defaults could + // break a lot of users' configurations. + it('is created with default settings', () => { + const browserTracing = createBrowserTracing(); + + expect(browserTracing.options).toEqual({ + enableLongTask: true, + _experiments: {}, + ...TRACING_DEFAULTS, + routingInstrumentation: instrumentRoutingWithDefaults, + spanOnLocationChange: true, + spanOnPageLoad: true, + markBackgroundSpan: true, + ...defaultRequestInstrumentationOptions, + }); + }); + + it('handles legacy `startTransactionOnLocationChange` option', () => { + const integration = new BrowserTracing({ startTransactionOnLocationChange: false }); + expect(integration.options.spanOnLocationChange).toBe(false); + }); + + it('handles legacy `startTransactionOnPageLoad` option', () => { + const integration = new BrowserTracing({ startTransactionOnPageLoad: false }); + expect(integration.options.spanOnPageLoad).toBe(false); + }); + + it('handles legacy `markBackgroundTransactions` option', () => { + const integration = new BrowserTracing({ markBackgroundTransactions: false }); + expect(integration.options.markBackgroundSpan).toBe(false); + }); + + it('allows to disable enableLongTask via _experiments', () => { + const browserTracing = createBrowserTracing(false, { + _experiments: { + enableLongTask: false, + }, + }); + + expect(browserTracing.options.enableLongTask).toBe(false); + expect(browserTracing.options._experiments.enableLongTask).toBe(false); + }); + + it('allows to disable enableLongTask', () => { + const browserTracing = createBrowserTracing(false, { + enableLongTask: false, + }); + + expect(browserTracing.options.enableLongTask).toBe(false); + expect(browserTracing.options._experiments.enableLongTask).toBe(undefined); + }); + }); }); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 7f9d66c904fa..ca4c976db1c8 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -100,6 +100,7 @@ export type { SpanContextData, TraceFlag, } from './span'; +export type { StartSpanOptions } from './startSpanOptions'; export type { StackFrame } from './stackframe'; export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace'; export type { TextEncoderInternal } from './textencoder'; diff --git a/packages/types/src/startSpanOptions.ts b/packages/types/src/startSpanOptions.ts new file mode 100644 index 000000000000..49f81250008c --- /dev/null +++ b/packages/types/src/startSpanOptions.ts @@ -0,0 +1,104 @@ +import type { Instrumenter } from './instrumenter'; +import type { Primitive } from './misc'; +import type { Scope } from './scope'; +import type { SpanAttributes, SpanOrigin, SpanTimeInput } from './span'; +import type { TransactionContext, TransactionMetadata, TransactionSource } from './transaction'; + +export interface StartSpanOptions extends TransactionContext { + /** A manually specified start time for the created `Span` object. */ + startTime?: SpanTimeInput; + + /** If defined, start this span off this scope instead off the current scope. */ + scope?: Scope; + + /** The name of the span. */ + name: string; + + /** An op for the span. This is a categorization for spans. */ + op?: string; + + /** The origin of the span - if it comes from auto instrumenation or manual instrumentation. */ + origin?: SpanOrigin; + + /** Attributes for the span. */ + attributes?: SpanAttributes; + + // All remaining fields are deprecated + + /** + * @deprecated Manually set the end timestamp instead. + */ + trimEnd?: boolean; + + /** + * @deprecated This cannot be set manually anymore. + */ + parentSampled?: boolean; + + /** + * @deprecated Use attributes or set data on scopes instead. + */ + metadata?: Partial; + + /** + * The name thingy. + * @deprecated Use `name` instead. + */ + description?: string; + + /** + * @deprecated Use `span.setStatus()` instead. + */ + status?: string; + + /** + * @deprecated Use `scope` instead. + */ + parentSpanId?: string; + + /** + * @deprecated You cannot manually set the span to sampled anymore. + */ + sampled?: boolean; + + /** + * @deprecated You cannot manually set the spanId anymore. + */ + spanId?: string; + + /** + * @deprecated You cannot manually set the traceId anymore. + */ + traceId?: string; + + /** + * @deprecated Use an attribute instead. + */ + source?: TransactionSource; + + /** + * @deprecated Use attributes or set tags on the scope instead. + */ + tags?: { [key: string]: Primitive }; + + /** + * @deprecated Use attributes instead. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + data?: { [key: string]: any }; + + /** + * @deprecated Use `startTime` instead. + */ + startTimestamp?: number; + + /** + * @deprecated Use `span.end()` instead. + */ + endTimestamp?: number; + + /** + * @deprecated You cannot set the instrumenter manually anymore. + */ + instrumenter?: Instrumenter; +} From a2f0bac1fad007bd7acea4b5983ff19ee4c697e0 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 24 Jan 2024 09:04:44 -0500 Subject: [PATCH 04/15] feat(browser): Deprecate `BrowserTracing` in favor of `browserTracingIntegration` (#10304) Step by step... --- MIGRATION.md | 1 + packages/astro/src/client/sdk.ts | 4 +-- packages/astro/test/client/sdk.test.ts | 7 ++--- packages/browser/src/index.bundle.feedback.ts | 11 +++++++- packages/browser/src/index.bundle.replay.ts | 4 +++ .../index.bundle.tracing.replay.feedback.ts | 5 +++- .../src/index.bundle.tracing.replay.ts | 5 +++- packages/browser/src/index.bundle.tracing.ts | 5 +++- packages/browser/src/index.bundle.ts | 4 +++ packages/browser/src/index.ts | 2 ++ .../test/unit/profiling/integration.test.ts | 2 +- .../sentry-performance.ts | 4 +-- packages/ember/addon/types.ts | 4 +-- packages/gatsby/src/utils/integrations.ts | 6 ++--- packages/gatsby/test/gatsby-browser.test.ts | 4 +-- packages/gatsby/test/sdk.test.ts | 10 +++---- .../integration-shims/src/BrowserTracing.ts | 19 ++++++++++++- packages/integration-shims/src/index.ts | 9 ++++++- .../src/client/browserTracingIntegration.ts | 13 +++++++++ packages/nextjs/src/client/index.ts | 15 ++++++++--- packages/nextjs/test/clientSdk.test.ts | 27 ++++++++++++------- .../src/client/browserTracingIntegration.ts | 13 +++++++++ packages/sveltekit/src/client/sdk.ts | 8 +++--- packages/sveltekit/test/client/sdk.test.ts | 6 +++-- .../src/browser/browsertracing.ts | 8 ++++++ .../tracing-internal/src/browser/index.ts | 7 ++++- packages/tracing-internal/src/index.ts | 2 ++ packages/tracing/src/index.ts | 2 ++ 28 files changed, 162 insertions(+), 45 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 241697d54586..063788881242 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -46,6 +46,7 @@ The following list shows how integrations should be migrated: | Old | New | Packages | | ------------------------- | -------------------------------- | ------------------------------------------------------------------------------------------------------- | +| `new BrowserTracing()` | `browserTracingIntegration()` | `@sentry/browser` | | `new InboundFilters()` | `inboundFiltersIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | | `new FunctionToString()` | `functionToStringIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | | `new LinkedErrors()` | `linkedErrorsIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | diff --git a/packages/astro/src/client/sdk.ts b/packages/astro/src/client/sdk.ts index 8d2b70ee6751..a289296c2ab2 100644 --- a/packages/astro/src/client/sdk.ts +++ b/packages/astro/src/client/sdk.ts @@ -1,6 +1,6 @@ import type { BrowserOptions } from '@sentry/browser'; import { - BrowserTracing, + browserTracingIntegration, getDefaultIntegrations as getBrowserDefaultIntegrations, init as initBrowserSdk, setTag, @@ -34,7 +34,7 @@ function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefi // in which case everything inside will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - return [...getBrowserDefaultIntegrations(options), new BrowserTracing()]; + return [...getBrowserDefaultIntegrations(options), browserTracingIntegration()]; } } diff --git a/packages/astro/test/client/sdk.test.ts b/packages/astro/test/client/sdk.test.ts index 2e10d4210953..4996df46ca39 100644 --- a/packages/astro/test/client/sdk.test.ts +++ b/packages/astro/test/client/sdk.test.ts @@ -1,7 +1,7 @@ -import type { BrowserClient } from '@sentry/browser'; +import type { BrowserClient, BrowserTracing } from '@sentry/browser'; import { getCurrentScope } from '@sentry/browser'; import * as SentryBrowser from '@sentry/browser'; -import { BrowserTracing, SDK_VERSION, WINDOW, getClient } from '@sentry/browser'; +import { SDK_VERSION, WINDOW, browserTracingIntegration, getClient } from '@sentry/browser'; import { vi } from 'vitest'; import { init } from '../../../astro/src/client/sdk'; @@ -103,12 +103,13 @@ describe('Sentry client SDK', () => { it('Overrides the automatically default BrowserTracing instance with a a user-provided instance', () => { init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [new BrowserTracing({ finalTimeout: 10, startTransactionOnLocationChange: false })], + integrations: [browserTracingIntegration({ finalTimeout: 10, startTransactionOnLocationChange: false })], enableTracing: true, }); const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations; + // eslint-disable-next-line deprecation/deprecation const browserTracing = getClient()?.getIntegrationByName('BrowserTracing') as BrowserTracing; const options = browserTracing.options; diff --git a/packages/browser/src/index.bundle.feedback.ts b/packages/browser/src/index.bundle.feedback.ts index 5d3612106286..8e653c2d4757 100644 --- a/packages/browser/src/index.bundle.feedback.ts +++ b/packages/browser/src/index.bundle.feedback.ts @@ -1,6 +1,12 @@ // This is exported so the loader does not fail when switching off Replay/Tracing import { Feedback, feedbackIntegration } from '@sentry-internal/feedback'; -import { BrowserTracing, Replay, addTracingExtensions, replayIntegration } from '@sentry-internal/integration-shims'; +import { + BrowserTracing, + Replay, + addTracingExtensions, + browserTracingIntegration, + replayIntegration, +} from '@sentry-internal/integration-shims'; import * as Sentry from './index.bundle.base'; @@ -8,11 +14,14 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; export { + // eslint-disable-next-line deprecation/deprecation BrowserTracing, + browserTracingIntegration, addTracingExtensions, // eslint-disable-next-line deprecation/deprecation Replay, diff --git a/packages/browser/src/index.bundle.replay.ts b/packages/browser/src/index.bundle.replay.ts index 2609e7d9b48c..2e4619ab49ea 100644 --- a/packages/browser/src/index.bundle.replay.ts +++ b/packages/browser/src/index.bundle.replay.ts @@ -3,6 +3,7 @@ import { BrowserTracing, Feedback, addTracingExtensions, + browserTracingIntegration, feedbackIntegration, } from '@sentry-internal/integration-shims'; import { Replay, replayIntegration } from '@sentry/replay'; @@ -13,11 +14,14 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; export { + // eslint-disable-next-line deprecation/deprecation BrowserTracing, + browserTracingIntegration, addTracingExtensions, // eslint-disable-next-line deprecation/deprecation Replay, diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.ts index e17c7de4159a..5822a2379cc4 100644 --- a/packages/browser/src/index.bundle.tracing.replay.feedback.ts +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.ts @@ -1,5 +1,5 @@ import { Feedback, feedbackIntegration } from '@sentry-internal/feedback'; -import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; +import { BrowserTracing, Span, addExtensionMethods, browserTracingIntegration } from '@sentry-internal/tracing'; import { Replay, replayIntegration } from '@sentry/replay'; import * as Sentry from './index.bundle.base'; @@ -10,6 +10,7 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods @@ -22,7 +23,9 @@ export { Replay, feedbackIntegration, replayIntegration, + // eslint-disable-next-line deprecation/deprecation BrowserTracing, + browserTracingIntegration, Span, addExtensionMethods, }; diff --git a/packages/browser/src/index.bundle.tracing.replay.ts b/packages/browser/src/index.bundle.tracing.replay.ts index 5dc0537be064..41ea77b8d982 100644 --- a/packages/browser/src/index.bundle.tracing.replay.ts +++ b/packages/browser/src/index.bundle.tracing.replay.ts @@ -1,5 +1,5 @@ import { Feedback, feedbackIntegration } from '@sentry-internal/integration-shims'; -import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; +import { BrowserTracing, Span, addExtensionMethods, browserTracingIntegration } from '@sentry-internal/tracing'; import { Replay, replayIntegration } from '@sentry/replay'; import * as Sentry from './index.bundle.base'; @@ -10,6 +10,7 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods @@ -22,7 +23,9 @@ export { Replay, replayIntegration, feedbackIntegration, + // eslint-disable-next-line deprecation/deprecation BrowserTracing, + browserTracingIntegration, Span, addExtensionMethods, }; diff --git a/packages/browser/src/index.bundle.tracing.ts b/packages/browser/src/index.bundle.tracing.ts index f810b61b92a7..6607dacf992c 100644 --- a/packages/browser/src/index.bundle.tracing.ts +++ b/packages/browser/src/index.bundle.tracing.ts @@ -1,6 +1,6 @@ // This is exported so the loader does not fail when switching off Replay import { Feedback, Replay, feedbackIntegration, replayIntegration } from '@sentry-internal/integration-shims'; -import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; +import { BrowserTracing, Span, addExtensionMethods, browserTracingIntegration } from '@sentry-internal/tracing'; import * as Sentry from './index.bundle.base'; @@ -10,6 +10,7 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods @@ -22,7 +23,9 @@ export { Replay, feedbackIntegration, replayIntegration, + // eslint-disable-next-line deprecation/deprecation BrowserTracing, + browserTracingIntegration, Span, addExtensionMethods, }; diff --git a/packages/browser/src/index.bundle.ts b/packages/browser/src/index.bundle.ts index a92ff6bf66ec..3087d7d317ca 100644 --- a/packages/browser/src/index.bundle.ts +++ b/packages/browser/src/index.bundle.ts @@ -4,6 +4,7 @@ import { Feedback, Replay, addTracingExtensions, + browserTracingIntegration, feedbackIntegration, replayIntegration, } from '@sentry-internal/integration-shims'; @@ -14,16 +15,19 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; export { + // eslint-disable-next-line deprecation/deprecation BrowserTracing, addTracingExtensions, // eslint-disable-next-line deprecation/deprecation Replay, // eslint-disable-next-line deprecation/deprecation Feedback, + browserTracingIntegration, feedbackIntegration, replayIntegration, }; diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 19c377fc5931..8fbf60bf1bd7 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -54,7 +54,9 @@ export { } from '@sentry-internal/feedback'; export { + // eslint-disable-next-line deprecation/deprecation BrowserTracing, + browserTracingIntegration, defaultRequestInstrumentationOptions, instrumentOutgoingRequests, } from '@sentry-internal/tracing'; diff --git a/packages/browser/test/unit/profiling/integration.test.ts b/packages/browser/test/unit/profiling/integration.test.ts index b69d3a52d655..d7f13a3848f6 100644 --- a/packages/browser/test/unit/profiling/integration.test.ts +++ b/packages/browser/test/unit/profiling/integration.test.ts @@ -44,7 +44,7 @@ describe('BrowserProfilingIntegration', () => { send, }; }, - integrations: [new Sentry.BrowserTracing(), new BrowserProfilingIntegration()], + integrations: [Sentry.browserTracingIntegration(), new BrowserProfilingIntegration()], }); const client = Sentry.getClient(); diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index b25125b28da6..d6f5bf61a053 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -404,11 +404,11 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) // Maintaining backwards compatibility with config.browserTracingOptions, but passing it with Sentry options is preferred. const browserTracingOptions = config.browserTracingOptions || config.sentry.browserTracingOptions || {}; - const { BrowserTracing } = await import('@sentry/browser'); + const { browserTracingIntegration } = await import('@sentry/browser'); const idleTimeout = config.transitionTimeout || 5000; - const browserTracing = new BrowserTracing({ + const browserTracing = browserTracingIntegration({ routingInstrumentation: (customStartTransaction, startTransactionOnPageLoad) => { // eslint-disable-next-line ember/no-private-routing-service const routerMain = appInstance.lookup('router:main') as EmberRouterMain; diff --git a/packages/ember/addon/types.ts b/packages/ember/addon/types.ts index 787eecc7e4cf..dbd14ff96f70 100644 --- a/packages/ember/addon/types.ts +++ b/packages/ember/addon/types.ts @@ -1,7 +1,7 @@ -import type { BrowserOptions, BrowserTracing } from '@sentry/browser'; +import type { BrowserOptions, browserTracingIntegration } from '@sentry/browser'; import type { Transaction, TransactionContext } from '@sentry/types'; -type BrowserTracingOptions = ConstructorParameters[0]; +type BrowserTracingOptions = Parameters[0]; export type EmberSentryConfig = { sentry: BrowserOptions & { browserTracingOptions?: BrowserTracingOptions }; diff --git a/packages/gatsby/src/utils/integrations.ts b/packages/gatsby/src/utils/integrations.ts index 94ef28f21272..7c61adee1d50 100644 --- a/packages/gatsby/src/utils/integrations.ts +++ b/packages/gatsby/src/utils/integrations.ts @@ -1,5 +1,5 @@ import { hasTracingEnabled } from '@sentry/core'; -import { BrowserTracing } from '@sentry/react'; +import { browserTracingIntegration } from '@sentry/react'; import type { Integration } from '@sentry/types'; import type { GatsbyOptions } from './types'; @@ -31,8 +31,8 @@ export function getIntegrationsFromOptions(options: GatsbyOptions): UserIntegrat * @param isTracingEnabled Whether the user has enabled tracing. */ function getIntegrationsFromArray(userIntegrations: Integration[], isTracingEnabled: boolean): Integration[] { - if (isTracingEnabled && !userIntegrations.some(integration => integration.name === BrowserTracing.name)) { - userIntegrations.push(new BrowserTracing()); + if (isTracingEnabled && !userIntegrations.some(integration => integration.name === 'BrowserTracing')) { + userIntegrations.push(browserTracingIntegration()); } return userIntegrations; } diff --git a/packages/gatsby/test/gatsby-browser.test.ts b/packages/gatsby/test/gatsby-browser.test.ts index b67305042c71..cf456a9d3e9d 100644 --- a/packages/gatsby/test/gatsby-browser.test.ts +++ b/packages/gatsby/test/gatsby-browser.test.ts @@ -2,7 +2,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { onClientEntry } from '../gatsby-browser'; -import { BrowserTracing } from '../src/index'; +import { browserTracingIntegration } from '../src/index'; (global as any).__SENTRY_RELEASE__ = '683f3a6ab819d47d23abfca9a914c81f0524d35b'; (global as any).__SENTRY_DSN__ = 'https://examplePublicKey@o0.ingest.sentry.io/0'; @@ -141,7 +141,7 @@ describe('onClientEntry', () => { }); it('only defines a single `BrowserTracing` integration', () => { - const integrations = [new BrowserTracing()]; + const integrations = [browserTracingIntegration()]; onClientEntry(undefined, { tracesSampleRate: 0.5, integrations }); expect(sentryInit).toHaveBeenLastCalledWith( diff --git a/packages/gatsby/test/sdk.test.ts b/packages/gatsby/test/sdk.test.ts index c3c95cdabc1f..28206d1ef6c5 100644 --- a/packages/gatsby/test/sdk.test.ts +++ b/packages/gatsby/test/sdk.test.ts @@ -1,4 +1,4 @@ -import { BrowserTracing, SDK_VERSION, init } from '@sentry/react'; +import { SDK_VERSION, browserTracingIntegration, init } from '@sentry/react'; import type { Integration } from '@sentry/types'; import { init as gatsbyInit } from '../src/sdk'; @@ -68,27 +68,27 @@ describe('Integrations from options', () => { [ 'tracing disabled, with BrowserTracing as an array', [], - { integrations: [new BrowserTracing()] }, + { integrations: [browserTracingIntegration()] }, ['BrowserTracing'], ], [ 'tracing disabled, with BrowserTracing as a function', [], { - integrations: () => [new BrowserTracing()], + integrations: () => [browserTracingIntegration()], }, ['BrowserTracing'], ], [ 'tracing enabled, with BrowserTracing as an array', [], - { tracesSampleRate: 1, integrations: [new BrowserTracing()] }, + { tracesSampleRate: 1, integrations: [browserTracingIntegration()] }, ['BrowserTracing'], ], [ 'tracing enabled, with BrowserTracing as a function', [], - { tracesSampleRate: 1, integrations: () => [new BrowserTracing()] }, + { tracesSampleRate: 1, integrations: () => [browserTracingIntegration()] }, ['BrowserTracing'], ], ] as TestArgs[])( diff --git a/packages/integration-shims/src/BrowserTracing.ts b/packages/integration-shims/src/BrowserTracing.ts index 310dc589afe9..94936b004903 100644 --- a/packages/integration-shims/src/BrowserTracing.ts +++ b/packages/integration-shims/src/BrowserTracing.ts @@ -5,6 +5,8 @@ import { consoleSandbox } from '@sentry/utils'; * This is a shim for the BrowserTracing integration. * It is needed in order for the CDN bundles to continue working when users add/remove tracing * from it, without changing their config. This is necessary for the loader mechanism. + * + * @deprecated Use `browserTracingIntegration()` instead. */ class BrowserTracingShim implements Integration { /** @@ -19,6 +21,7 @@ class BrowserTracingShim implements Integration { // eslint-disable-next-line @typescript-eslint/no-explicit-any public constructor(_options: any) { + // eslint-disable-next-line deprecation/deprecation this.name = BrowserTracingShim.id; consoleSandbox(() => { @@ -33,7 +36,21 @@ class BrowserTracingShim implements Integration { } } -export { BrowserTracingShim as BrowserTracing }; +/** + * This is a shim for the BrowserTracing integration. + * It is needed in order for the CDN bundles to continue working when users add/remove tracing + * from it, without changing their config. This is necessary for the loader mechanism. + */ +function browserTracingIntegrationShim(): Integration { + // eslint-disable-next-line deprecation/deprecation + return new BrowserTracingShim({}); +} + +export { + // eslint-disable-next-line deprecation/deprecation + BrowserTracingShim as BrowserTracing, + browserTracingIntegrationShim as browserTracingIntegration, +}; /** Shim function */ export function addTracingExtensions(): void { diff --git a/packages/integration-shims/src/index.ts b/packages/integration-shims/src/index.ts index 43243f69a194..bffdf82c99f7 100644 --- a/packages/integration-shims/src/index.ts +++ b/packages/integration-shims/src/index.ts @@ -3,9 +3,16 @@ export { Feedback, feedbackIntegration, } from './Feedback'; + export { // eslint-disable-next-line deprecation/deprecation Replay, replayIntegration, } from './Replay'; -export { BrowserTracing, addTracingExtensions } from './BrowserTracing'; + +export { + // eslint-disable-next-line deprecation/deprecation + BrowserTracing, + browserTracingIntegration, + addTracingExtensions, +} from './BrowserTracing'; diff --git a/packages/nextjs/src/client/browserTracingIntegration.ts b/packages/nextjs/src/client/browserTracingIntegration.ts index c3eb18887301..b7c6f33159eb 100644 --- a/packages/nextjs/src/client/browserTracingIntegration.ts +++ b/packages/nextjs/src/client/browserTracingIntegration.ts @@ -1,10 +1,14 @@ import { BrowserTracing as OriginalBrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/react'; +import type { Integration } from '@sentry/types'; import { nextRouterInstrumentation } from '../index.client'; /** * A custom BrowserTracing integration for Next.js. + * @deprecated Use `browserTracingIntegration()` instead. */ +// eslint-disable-next-line deprecation/deprecation export class BrowserTracing extends OriginalBrowserTracing { + // eslint-disable-next-line deprecation/deprecation public constructor(options?: ConstructorParameters[0]) { super({ // eslint-disable-next-line deprecation/deprecation @@ -24,3 +28,12 @@ export class BrowserTracing extends OriginalBrowserTracing { }); } } + +/** + * A custom BrowserTracing integration for Next.js. + */ +// eslint-disable-next-line deprecation/deprecation +export function browserTracingIntegration(options?: ConstructorParameters[0]): Integration { + // eslint-disable-next-line deprecation/deprecation + return new BrowserTracing(options); +} diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index d1d5e1db7ff5..60c1c5d2a7f8 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -10,7 +10,7 @@ import type { EventProcessor, Integration } from '@sentry/types'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; -import { BrowserTracing } from './browserTracingIntegration'; +import { BrowserTracing, browserTracingIntegration } from './browserTracingIntegration'; import { rewriteFramesIntegration } from './rewriteFramesIntegration'; import { applyTunnelRouteOption } from './tunnelRoute'; @@ -35,7 +35,12 @@ export const Integrations = { // // import { BrowserTracing } from '@sentry/nextjs'; // const instance = new BrowserTracing(); -export { BrowserTracing, rewriteFramesIntegration }; +export { + // eslint-disable-next-line deprecation/deprecation + BrowserTracing, + browserTracingIntegration, + rewriteFramesIntegration, +}; // Treeshakable guard to remove all code related to tracing declare const __SENTRY_TRACING__: boolean; @@ -90,13 +95,15 @@ function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Inte const browserTracing = integrations.find(integration => integration.name === 'BrowserTracing'); // If BrowserTracing was added, but it is not our forked version, // replace it with our forked version with the same options + // eslint-disable-next-line deprecation/deprecation if (browserTracing && !(browserTracing instanceof BrowserTracing)) { + // eslint-disable-next-line deprecation/deprecation const options: ConstructorParameters[0] = (browserTracing as BrowserTracing).options; // These two options are overwritten by the custom integration delete options.routingInstrumentation; // eslint-disable-next-line deprecation/deprecation delete options.tracingOrigins; - integrations[integrations.indexOf(browserTracing)] = new BrowserTracing(options); + integrations[integrations.indexOf(browserTracing)] = browserTracingIntegration(options); } return integrations; @@ -109,7 +116,7 @@ function getDefaultIntegrations(options: BrowserOptions): Integration[] { // will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - customDefaultIntegrations.push(new BrowserTracing()); + customDefaultIntegrations.push(browserTracingIntegration()); } } diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts index 464b7db14dc7..6de340dfc3e1 100644 --- a/packages/nextjs/test/clientSdk.test.ts +++ b/packages/nextjs/test/clientSdk.test.ts @@ -6,7 +6,8 @@ import type { Integration } from '@sentry/types'; import { logger } from '@sentry/utils'; import { JSDOM } from 'jsdom'; -import { BrowserTracing, breadcrumbsIntegration, init, nextRouterInstrumentation } from '../src/client'; +import type { BrowserTracing } from '../src/client'; +import { breadcrumbsIntegration, browserTracingIntegration, init, nextRouterInstrumentation } from '../src/client'; const reactInit = jest.spyOn(SentryReact, 'init'); const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent'); @@ -124,6 +125,7 @@ describe('Client init()', () => { }); const client = getClient()!; + // eslint-disable-next-line deprecation/deprecation const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeDefined(); @@ -141,6 +143,7 @@ describe('Client init()', () => { }); const client = getClient()!; + // eslint-disable-next-line deprecation/deprecation const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeDefined(); @@ -157,6 +160,7 @@ describe('Client init()', () => { }); const client = getClient()!; + // eslint-disable-next-line deprecation/deprecation const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeUndefined(); @@ -166,14 +170,15 @@ describe('Client init()', () => { init({ dsn: TEST_DSN, tracesSampleRate: 1.0, - integrations: [new BrowserTracing({ startTransactionOnLocationChange: false })], + integrations: [browserTracingIntegration({ startTransactionOnLocationChange: false })], }); const client = getClient()!; - const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + // eslint-disable-next-line deprecation/deprecation + const integration = client.getIntegrationByName('BrowserTracing'); - expect(browserTracingIntegration).toBeDefined(); - expect(browserTracingIntegration?.options).toEqual( + expect(integration).toBeDefined(); + expect(integration?.options).toEqual( expect.objectContaining({ routingInstrumentation: nextRouterInstrumentation, // This proves it's still the user's copy @@ -186,15 +191,19 @@ describe('Client init()', () => { init({ dsn: TEST_DSN, tracesSampleRate: 1.0, - integrations: defaults => [...defaults, new BrowserTracing({ startTransactionOnLocationChange: false })], + integrations: defaults => [ + ...defaults, + browserTracingIntegration({ startTransactionOnLocationChange: false }), + ], }); const client = getClient()!; - const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + // eslint-disable-next-line deprecation/deprecation + const integration = client.getIntegrationByName('BrowserTracing'); - expect(browserTracingIntegration).toBeDefined(); - expect(browserTracingIntegration?.options).toEqual( + expect(integration).toBeDefined(); + expect(integration?.options).toEqual( expect.objectContaining({ routingInstrumentation: nextRouterInstrumentation, // This proves it's still the user's copy diff --git a/packages/sveltekit/src/client/browserTracingIntegration.ts b/packages/sveltekit/src/client/browserTracingIntegration.ts index 9968f8b6de5f..d6b5e0bde95f 100644 --- a/packages/sveltekit/src/client/browserTracingIntegration.ts +++ b/packages/sveltekit/src/client/browserTracingIntegration.ts @@ -1,10 +1,14 @@ import { BrowserTracing as OriginalBrowserTracing } from '@sentry/svelte'; +import type { Integration } from '@sentry/types'; import { svelteKitRoutingInstrumentation } from './router'; /** * A custom BrowserTracing integration for Sveltekit. + * @deprecated Use `browserTracingIntegration()` instead. */ +// eslint-disable-next-line deprecation/deprecation export class BrowserTracing extends OriginalBrowserTracing { + // eslint-disable-next-line deprecation/deprecation public constructor(options?: ConstructorParameters[0]) { super({ routingInstrumentation: svelteKitRoutingInstrumentation, @@ -12,3 +16,12 @@ export class BrowserTracing extends OriginalBrowserTracing { }); } } + +/** + * A custom BrowserTracing integration for Sveltekit. + */ +// eslint-disable-next-line deprecation/deprecation +export function browserTracingIntegration(options?: ConstructorParameters[0]): Integration { + // eslint-disable-next-line deprecation/deprecation + return new BrowserTracing(options); +} diff --git a/packages/sveltekit/src/client/sdk.ts b/packages/sveltekit/src/client/sdk.ts index 7b9c608a862d..c8894605f53d 100644 --- a/packages/sveltekit/src/client/sdk.ts +++ b/packages/sveltekit/src/client/sdk.ts @@ -4,7 +4,7 @@ import { getDefaultIntegrations as getDefaultSvelteIntegrations } from '@sentry/ import { WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte'; import type { Integration } from '@sentry/types'; -import { BrowserTracing } from './browserTracingIntegration'; +import { BrowserTracing, browserTracingIntegration } from './browserTracingIntegration'; type WindowWithSentryFetchProxy = typeof WINDOW & { _sentryFetchProxy?: typeof fetch; @@ -65,11 +65,13 @@ function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Inte const browserTracing = integrations.find(integration => integration.name === 'BrowserTracing'); // If BrowserTracing was added, but it is not our forked version, // replace it with our forked version with the same options + // eslint-disable-next-line deprecation/deprecation if (browserTracing && !(browserTracing instanceof BrowserTracing)) { + // eslint-disable-next-line deprecation/deprecation const options: ConstructorParameters[0] = (browserTracing as BrowserTracing).options; // This option is overwritten by the custom integration delete options.routingInstrumentation; - integrations[integrations.indexOf(browserTracing)] = new BrowserTracing(options); + integrations[integrations.indexOf(browserTracing)] = browserTracingIntegration(options); } return integrations; @@ -80,7 +82,7 @@ function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefi // will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - return [...getDefaultSvelteIntegrations(options), new BrowserTracing()]; + return [...getDefaultSvelteIntegrations(options), browserTracingIntegration()]; } } diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts index 10292658bc54..0f643c9e9b14 100644 --- a/packages/sveltekit/test/client/sdk.test.ts +++ b/packages/sveltekit/test/client/sdk.test.ts @@ -4,7 +4,8 @@ import * as SentrySvelte from '@sentry/svelte'; import { SDK_VERSION, WINDOW } from '@sentry/svelte'; import { vi } from 'vitest'; -import { BrowserTracing, init } from '../../src/client'; +import type { BrowserTracing } from '../../src/client'; +import { browserTracingIntegration, init } from '../../src/client'; import { svelteKitRoutingInstrumentation } from '../../src/client/router'; const svelteInit = vi.spyOn(SentrySvelte, 'init'); @@ -100,10 +101,11 @@ describe('Sentry client SDK', () => { it('Merges a user-provided BrowserTracing integration with the automatically added one', () => { init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [new BrowserTracing({ finalTimeout: 10, startTransactionOnLocationChange: false })], + integrations: [browserTracingIntegration({ finalTimeout: 10, startTransactionOnLocationChange: false })], enableTracing: true, }); + // eslint-disable-next-line deprecation/deprecation const browserTracing = getClient()?.getIntegrationByName('BrowserTracing') as BrowserTracing; const options = browserTracing.options; diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index a2d89b5c7c28..28c10215118d 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -195,6 +195,14 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { * The integration can be configured with a variety of options, and can be extended to use * any routing library. This integration uses {@see IdleTransaction} to create transactions. */ +export function browserTracingIntegration(options?: Partial): Integration { + // eslint-disable-next-line deprecation/deprecation + return new BrowserTracing(options); +} + +/** + * @deprecated Use `browserTracingIntegration()` instead. + */ export class BrowserTracing implements Integration { // This class currently doesn't have a static `id` field like the other integration classes, because it prevented // @sentry/tracing from being treeshaken. Tree shakers do not like static fields, because they behave like side effects. diff --git a/packages/tracing-internal/src/browser/index.ts b/packages/tracing-internal/src/browser/index.ts index 5b30bc519404..d9a17641b2b4 100644 --- a/packages/tracing-internal/src/browser/index.ts +++ b/packages/tracing-internal/src/browser/index.ts @@ -2,7 +2,12 @@ export * from '../exports'; export type { RequestInstrumentationOptions } from './request'; -export { BrowserTracing, BROWSER_TRACING_INTEGRATION_ID } from './browsertracing'; +export { + // eslint-disable-next-line deprecation/deprecation + BrowserTracing, + browserTracingIntegration, + BROWSER_TRACING_INTEGRATION_ID, +} from './browsertracing'; export { instrumentOutgoingRequests, defaultRequestInstrumentationOptions } from './request'; export { diff --git a/packages/tracing-internal/src/index.ts b/packages/tracing-internal/src/index.ts index 495d8dbb26b9..5e8791018c6f 100644 --- a/packages/tracing-internal/src/index.ts +++ b/packages/tracing-internal/src/index.ts @@ -13,7 +13,9 @@ export { export type { LazyLoadedIntegration } from './node'; export { + // eslint-disable-next-line deprecation/deprecation BrowserTracing, + browserTracingIntegration, BROWSER_TRACING_INTEGRATION_ID, instrumentOutgoingRequests, defaultRequestInstrumentationOptions, diff --git a/packages/tracing/src/index.ts b/packages/tracing/src/index.ts index 8559188884d7..b0a5b47554ae 100644 --- a/packages/tracing/src/index.ts +++ b/packages/tracing/src/index.ts @@ -38,6 +38,7 @@ import { * import { BrowserTracing } from '@sentry/browser'; * new BrowserTracing() */ +// eslint-disable-next-line deprecation/deprecation export const BrowserTracing = BrowserTracingT; // BrowserTracing is already exported as part of `Integrations` below (and for the moment will remain so for @@ -50,6 +51,7 @@ export const BrowserTracing = BrowserTracingT; * import { BrowserTracing } from '@sentry/browser'; * new BrowserTracing() */ +// eslint-disable-next-line deprecation/deprecation export type BrowserTracing = BrowserTracingT; /** From d471dcf6066dafafd751b7449a507e5071f32008 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 24 Jan 2024 15:09:18 +0100 Subject: [PATCH 05/15] fix(node): LocalVariables integration should use setupOnce (#10307) Closes #10278 This integration incorrectly used `setup` which then caused issues when the core code was changed. It should have been using `setupOnce`. Unfortunately, `setupOnce` causes issues with the unit tests since it then relies on `getClient` to get the client instance for the client options. I've tried using jest to mock `getClient` but this doesn't work which is likely due to `installedIntegrations`. Because we already have [extensive integration tests for LocalVariables](https://github.com/getsentry/sentry-javascript/tree/develop/dev-packages/node-integration-tests/suites/public-api/LocalVariables) I have removed the unit tests because they are becoming a maintenance burden and actually test less than the integration tests. After v8, I plan to move Local variables lookup to the worker thread at which point we can revisit how these might be better tested in unit tests. --- .../local-variables/local-variables-sync.ts | 11 +- .../test/integrations/localvariables.test.ts | 346 +----------------- 2 files changed, 6 insertions(+), 351 deletions(-) diff --git a/packages/node/src/integrations/local-variables/local-variables-sync.ts b/packages/node/src/integrations/local-variables/local-variables-sync.ts index bfe255002975..93f3b61b10f2 100644 --- a/packages/node/src/integrations/local-variables/local-variables-sync.ts +++ b/packages/node/src/integrations/local-variables/local-variables-sync.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines */ -import { convertIntegrationFnToClass } from '@sentry/core'; +import { convertIntegrationFnToClass, getClient } from '@sentry/core'; import type { Event, Exception, Integration, IntegrationClass, IntegrationFn, StackParser } from '@sentry/types'; import { LRUMap, logger } from '@sentry/utils'; import type { Debugger, InspectorNotification, Runtime, Session } from 'inspector'; @@ -326,12 +326,11 @@ const localVariablesSyncIntegration = (( return { name: INTEGRATION_NAME, - // TODO v8: Remove this - setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function - setup(client: NodeClient) { - const clientOptions = client.getOptions(); + setupOnce() { + const client = getClient(); + const clientOptions = client?.getOptions(); - if (session && clientOptions.includeLocalVariables) { + if (session && clientOptions?.includeLocalVariables) { // Only setup this integration if the Node version is >= v18 // https://github.com/getsentry/sentry-javascript/issues/7697 const unsupportedNodeVersion = NODE_VERSION.major < 18; diff --git a/packages/node/test/integrations/localvariables.test.ts b/packages/node/test/integrations/localvariables.test.ts index b0fc6094e6a5..abc1d241f842 100644 --- a/packages/node/test/integrations/localvariables.test.ts +++ b/packages/node/test/integrations/localvariables.test.ts @@ -1,356 +1,12 @@ -import type { Debugger, InspectorNotification } from 'inspector'; - -import { NodeClient, defaultStackParser } from '../../src'; import { createRateLimiter } from '../../src/integrations/local-variables/common'; -import type { FrameVariables } from '../../src/integrations/local-variables/common'; -import type { DebugSession } from '../../src/integrations/local-variables/local-variables-sync'; -import { LocalVariablesSync, createCallbackList } from '../../src/integrations/local-variables/local-variables-sync'; +import { createCallbackList } from '../../src/integrations/local-variables/local-variables-sync'; import { NODE_VERSION } from '../../src/nodeVersion'; -import { getDefaultNodeClientOptions } from '../../test/helper/node-client-options'; jest.setTimeout(20_000); const describeIf = (condition: boolean) => (condition ? describe : describe.skip); -interface ThrowOn { - configureAndConnect?: boolean; - getLocalVariables?: boolean; -} - -class MockDebugSession implements DebugSession { - private _onPause?: (message: InspectorNotification, callback: () => void) => void; - - constructor(private readonly _vars: Record>, private readonly _throwOn?: ThrowOn) {} - - public configureAndConnect( - onPause: (message: InspectorNotification, callback: () => void) => void, - _captureAll: boolean, - ): void { - if (this._throwOn?.configureAndConnect) { - throw new Error('configureAndConnect should not be called'); - } - - this._onPause = onPause; - } - - public setPauseOnExceptions(_: boolean): void {} - - public getLocalVariables(objectId: string, callback: (vars: Record) => void): void { - if (this._throwOn?.getLocalVariables) { - throw new Error('getLocalVariables should not be called'); - } - - callback(this._vars[objectId]); - } - - public runPause(message: InspectorNotification): Promise { - return new Promise(resolve => { - this._onPause?.(message, resolve); - }); - } -} - -interface LocalVariablesPrivate { - _getCachedFramesCount(): number; - _getFirstCachedFrame(): FrameVariables[] | undefined; -} - -const exceptionEvent = { - method: 'Debugger.paused', - params: { - reason: 'exception', - data: { - description: - 'Error: Some error\n' + - ' at two (/dist/javascript/src/main.js:23:9)\n' + - ' at one (/dist/javascript/src/main.js:19:3)\n' + - ' at Timeout._onTimeout (/dist/javascript/src/main.js:40:5)\n' + - ' at listOnTimeout (node:internal/timers:559:17)\n' + - ' at process.processTimers (node:internal/timers:502:7)', - }, - callFrames: [ - { - callFrameId: '-6224981551105448869.1.0', - functionName: 'two', - location: { scriptId: '134', lineNumber: 22 }, - url: '', - scopeChain: [ - { - type: 'local', - object: { - type: 'object', - className: 'Object', - objectId: '-6224981551105448869.1.2', - }, - name: 'two', - }, - ], - this: { - type: 'object', - className: 'global', - }, - }, - { - callFrameId: '-6224981551105448869.1.1', - functionName: 'one', - location: { scriptId: '134', lineNumber: 18 }, - url: '', - scopeChain: [ - { - type: 'local', - object: { - type: 'object', - className: 'Object', - objectId: '-6224981551105448869.1.6', - }, - name: 'one', - }, - ], - this: { - type: 'object', - className: 'global', - }, - }, - ], - }, -}; - -const exceptionEvent100Frames = { - method: 'Debugger.paused', - params: { - reason: 'exception', - data: { - description: - 'Error: Some error\n' + - ' at two (/dist/javascript/src/main.js:23:9)\n' + - ' at one (/dist/javascript/src/main.js:19:3)\n' + - ' at Timeout._onTimeout (/dist/javascript/src/main.js:40:5)\n' + - ' at listOnTimeout (node:internal/timers:559:17)\n' + - ' at process.processTimers (node:internal/timers:502:7)', - }, - callFrames: new Array(100).fill({ - callFrameId: '-6224981551105448869.1.0', - functionName: 'two', - location: { scriptId: '134', lineNumber: 22 }, - url: '', - scopeChain: [ - { - type: 'local', - object: { - type: 'object', - className: 'Object', - objectId: '-6224981551105448869.1.2', - }, - name: 'two', - }, - ], - this: { - type: 'object', - className: 'global', - }, - }), - }, -}; - describeIf(NODE_VERSION.major >= 18)('LocalVariables', () => { - it('Adds local variables to stack frames', async () => { - const session = new MockDebugSession({ - '-6224981551105448869.1.2': { name: 'tim' }, - '-6224981551105448869.1.6': { arr: [1, 2, 3] }, - }); - const localVariables = new LocalVariablesSync({}, session); - const options = getDefaultNodeClientOptions({ - stackParser: defaultStackParser, - includeLocalVariables: true, - integrations: [], - }); - - const client = new NodeClient(options); - client.addIntegration(localVariables); - - const eventProcessors = client['_eventProcessors']; - const eventProcessor = eventProcessors.find(processor => processor.id === 'LocalVariables'); - - expect(eventProcessor).toBeDefined(); - - await session.runPause(exceptionEvent); - - expect((localVariables as unknown as LocalVariablesPrivate)._getCachedFramesCount()).toBe(1); - - const frames = (localVariables as unknown as LocalVariablesPrivate)._getFirstCachedFrame(); - - expect(frames).toBeDefined(); - - const vars = frames as FrameVariables[]; - - expect(vars).toEqual([ - { function: 'two', vars: { name: 'tim' } }, - { function: 'one', vars: { arr: [1, 2, 3] } }, - ]); - - const event = await eventProcessor!( - { - event_id: '9cbf882ade9a415986632ac4e16918eb', - platform: 'node', - timestamp: 1671113680.306, - level: 'fatal', - exception: { - values: [ - { - type: 'Error', - value: 'Some error', - stacktrace: { - frames: [ - { - function: 'process.processTimers', - lineno: 502, - colno: 7, - in_app: false, - }, - { - function: 'listOnTimeout', - lineno: 559, - colno: 17, - in_app: false, - }, - { - function: 'Timeout._onTimeout', - lineno: 40, - colno: 5, - in_app: true, - }, - { - function: 'one', - lineno: 19, - colno: 3, - in_app: true, - }, - { - function: 'two', - lineno: 23, - colno: 9, - in_app: true, - }, - ], - }, - mechanism: { type: 'generic', handled: true }, - }, - ], - }, - }, - {}, - ); - - expect(event?.exception?.values?.[0].stacktrace?.frames?.[3]?.vars).toEqual({ arr: [1, 2, 3] }); - expect(event?.exception?.values?.[0].stacktrace?.frames?.[4]?.vars).toEqual({ name: 'tim' }); - - expect((localVariables as unknown as LocalVariablesPrivate)._getCachedFramesCount()).toBe(0); - }); - - it('Only considers the first 5 frames', async () => { - const session = new MockDebugSession({}); - const localVariables = new LocalVariablesSync({}, session); - const options = getDefaultNodeClientOptions({ - stackParser: defaultStackParser, - includeLocalVariables: true, - integrations: [], - }); - - const client = new NodeClient(options); - client.addIntegration(localVariables); - - await session.runPause(exceptionEvent100Frames); - - expect((localVariables as unknown as LocalVariablesPrivate)._getCachedFramesCount()).toBe(1); - - const frames = (localVariables as unknown as LocalVariablesPrivate)._getFirstCachedFrame(); - - expect(frames).toBeDefined(); - - const vars = frames as FrameVariables[]; - - expect(vars.length).toEqual(5); - }); - - it('Should not lookup variables for non-exception reasons', async () => { - const session = new MockDebugSession({}, { getLocalVariables: true }); - const localVariables = new LocalVariablesSync({}, session); - const options = getDefaultNodeClientOptions({ - stackParser: defaultStackParser, - includeLocalVariables: true, - integrations: [], - }); - - const client = new NodeClient(options); - client.addIntegration(localVariables); - - const nonExceptionEvent = { - method: exceptionEvent.method, - params: { ...exceptionEvent.params, reason: 'non-exception-reason' }, - }; - - await session.runPause(nonExceptionEvent); - - expect((localVariables as unknown as LocalVariablesPrivate)._getCachedFramesCount()).toBe(0); - }); - - it('Should not initialize when disabled', async () => { - const session = new MockDebugSession({}, { configureAndConnect: true }); - const localVariables = new LocalVariablesSync({}, session); - const options = getDefaultNodeClientOptions({ - stackParser: defaultStackParser, - integrations: [], - }); - - const client = new NodeClient(options); - client.addIntegration(localVariables); - - const eventProcessors = client['_eventProcessors']; - const eventProcessor = eventProcessors.find(processor => processor.id === 'LocalVariables'); - - expect(eventProcessor).toBeDefined(); - }); - - it('Should not initialize when inspector not loaded', async () => { - const localVariables = new LocalVariablesSync({}, undefined); - const options = getDefaultNodeClientOptions({ - stackParser: defaultStackParser, - integrations: [], - }); - - const client = new NodeClient(options); - client.addIntegration(localVariables); - - const eventProcessors = client['_eventProcessors']; - const eventProcessor = eventProcessors.find(processor => processor.id === 'LocalVariables'); - - expect(eventProcessor).toBeDefined(); - }); - - it('Should cache identical uncaught exception events', async () => { - const session = new MockDebugSession({ - '-6224981551105448869.1.2': { name: 'tim' }, - '-6224981551105448869.1.6': { arr: [1, 2, 3] }, - }); - const localVariables = new LocalVariablesSync({}, session); - const options = getDefaultNodeClientOptions({ - stackParser: defaultStackParser, - includeLocalVariables: true, - integrations: [], - }); - - const client = new NodeClient(options); - client.addIntegration(localVariables); - - await session.runPause(exceptionEvent); - await session.runPause(exceptionEvent); - await session.runPause(exceptionEvent); - await session.runPause(exceptionEvent); - await session.runPause(exceptionEvent); - - expect((localVariables as unknown as LocalVariablesPrivate)._getCachedFramesCount()).toBe(1); - }); - describe('createCallbackList', () => { it('Should call callbacks in reverse order', done => { const log: number[] = []; From b1adeb682573b9d97fa80282cd24d6327e4ae510 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 24 Jan 2024 17:23:08 +0100 Subject: [PATCH 06/15] fix(node): Fix downleveled types entry point (#10321) Just noticed that this entry point was broken. `build/npm` doesn't exist because the node package is not a package with CDN bundles. Hence all NPM tarball contents are directly in `build`. --- packages/node/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index b096befa6b39..eb8b3056b149 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -20,8 +20,8 @@ "types": "build/types/index.d.ts", "typesVersions": { "<4.9": { - "build/npm/types/index.d.ts": [ - "build/npm/types-ts3.8/index.d.ts" + "build/types/index.d.ts": [ + "build/types-ts3.8/index.d.ts" ] } }, From 656b73762c452fd0db180483c1af55960ad23de6 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 24 Jan 2024 17:34:43 +0100 Subject: [PATCH 07/15] fix(node): Fix `node-cron` types and add test (#10315) --- .../node-integration-tests/package.json | 1 + .../suites/cron/node-cron/scenario.ts | 9 +++++++ .../suites/cron/node-cron/test.ts | 9 +++++++ .../node-integration-tests/utils/runner.ts | 16 +++++++++++++ packages/node/src/cron/cron.ts | 24 ++++++++++++++++--- yarn.lock | 18 ++++++++++++++ 6 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/cron/node-cron/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/cron/node-cron/test.ts diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index f36333c4daa8..68012d36269b 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -36,6 +36,7 @@ "apollo-server": "^3.11.1", "axios": "^0.27.2", "cors": "^2.8.5", + "cron": "^3.1.6", "express": "^4.17.3", "graphql": "^16.3.0", "http-terminator": "^3.2.0", diff --git a/dev-packages/node-integration-tests/suites/cron/node-cron/scenario.ts b/dev-packages/node-integration-tests/suites/cron/node-cron/scenario.ts new file mode 100644 index 000000000000..71b005d4dfd6 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/cron/node-cron/scenario.ts @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { CronJob } from 'cron'; + +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const CronJobWithCheckIn = Sentry.cron.instrumentCron(CronJob, 'my-cron-job'); + +setTimeout(() => { + process.exit(0); +}, 1_000); diff --git a/dev-packages/node-integration-tests/suites/cron/node-cron/test.ts b/dev-packages/node-integration-tests/suites/cron/node-cron/test.ts new file mode 100644 index 000000000000..b768599cd215 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/cron/node-cron/test.ts @@ -0,0 +1,9 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('node-cron types should match', done => { + createRunner(__dirname, 'scenario.ts').ensureNoErrorOutput().start(done); +}); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 1831761c3181..31969452ba74 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -70,6 +70,7 @@ export function createRunner(...paths: string[]) { const flags: string[] = []; const ignored: EnvelopeItemType[] = []; let withSentryServer = false; + let ensureNoErrorOutput = false; if (testPath.endsWith('.ts')) { flags.push('-r', 'ts-node/register'); @@ -92,6 +93,10 @@ export function createRunner(...paths: string[]) { ignored.push(...types); return this; }, + ensureNoErrorOutput: function () { + ensureNoErrorOutput = true; + return this; + }, start: function (done?: (e?: unknown) => void) { const expectedEnvelopeCount = expectedEnvelopes.length; @@ -190,8 +195,19 @@ export function createRunner(...paths: string[]) { CHILD_PROCESSES.add(child); + if (ensureNoErrorOutput) { + child.stderr.on('data', (data: Buffer) => { + const output = data.toString(); + complete(new Error(`Expected no error output but got: '${output}'`)); + }); + } + child.on('close', () => { hasExited = true; + + if (ensureNoErrorOutput) { + complete(); + } }); // Pass error to done to end the test quickly diff --git a/packages/node/src/cron/cron.ts b/packages/node/src/cron/cron.ts index 88a3e9e58eb5..2540d82e736b 100644 --- a/packages/node/src/cron/cron.ts +++ b/packages/node/src/cron/cron.ts @@ -8,10 +8,17 @@ export type CronJobParams = { start?: boolean | null; context?: unknown; runOnInit?: boolean | null; - utcOffset?: number; - timeZone?: string; unrefTimeout?: boolean | null; -}; +} & ( + | { + timeZone?: string | null; + utcOffset?: never; + } + | { + timeZone?: never; + utcOffset?: number | null; + } +); export type CronJob = { // @@ -28,6 +35,17 @@ export type CronJobConstructor = { timeZone?: CronJobParams['timeZone'], context?: CronJobParams['context'], runOnInit?: CronJobParams['runOnInit'], + utcOffset?: null, + unrefTimeout?: CronJobParams['unrefTimeout'], + ): CronJob; + new ( + cronTime: CronJobParams['cronTime'], + onTick: CronJobParams['onTick'], + onComplete?: CronJobParams['onComplete'], + start?: CronJobParams['start'], + timeZone?: null, + context?: CronJobParams['context'], + runOnInit?: CronJobParams['runOnInit'], utcOffset?: CronJobParams['utcOffset'], unrefTimeout?: CronJobParams['unrefTimeout'], ): CronJob; diff --git a/yarn.lock b/yarn.lock index 2a082acd3aa5..daeb04398140 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6268,6 +6268,11 @@ resolved "https://registry.yarnpkg.com/@types/lru-cache/-/lru-cache-5.1.0.tgz#57f228f2b80c046b4a1bd5cac031f81f207f4f03" integrity sha512-RaE0B+14ToE4l6UqdarKPnXwVDuigfFv+5j9Dze/Nqr23yyuqdNvzcZi3xB+3Agvi5R4EOgAksfv3lXX4vBt9w== +"@types/luxon@~3.3.0": + version "3.3.8" + resolved "https://registry.yarnpkg.com/@types/luxon/-/luxon-3.3.8.tgz#84dbf2d020a9209a272058725e168f21d331a67e" + integrity sha512-jYvz8UMLDgy3a5SkGJne8H7VA7zPV2Lwohjx0V8V31+SqAjNmurWMkk9cQhfvlcnXWudBpK9xPM1n4rljOcHYQ== + "@types/md5@2.1.33": version "2.1.33" resolved "https://registry.yarnpkg.com/@types/md5/-/md5-2.1.33.tgz#8c8dba30df4ad0e92296424f08c4898dd808e8df" @@ -11947,6 +11952,14 @@ critters@0.0.12: postcss "^8.3.7" pretty-bytes "^5.3.0" +cron@^3.1.6: + version "3.1.6" + resolved "https://registry.yarnpkg.com/cron/-/cron-3.1.6.tgz#e7e1798a468e017c8d31459ecd7c2d088f97346c" + integrity sha512-cvFiQCeVzsA+QPM6fhjBtlKGij7tLLISnTSvFxVdnFGLdz+ZdXN37kNe0i2gefmdD17XuZA6n2uPVwzl4FxW/w== + dependencies: + "@types/luxon" "~3.3.0" + luxon "~3.4.0" + cross-spawn@^6.0.0, cross-spawn@^6.0.5: version "6.0.5" resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-6.0.5.tgz#4a5ec7c64dfae22c3a14124dbacdee846d80cbc4" @@ -20696,6 +20709,11 @@ lunr@^2.3.8: resolved "https://registry.yarnpkg.com/lunr/-/lunr-2.3.9.tgz#18b123142832337dd6e964df1a5a7707b25d35e1" integrity sha512-zTU3DaZaF3Rt9rhN3uBMGQD3dD2/vFQqnvZCDv4dl5iOzq2IZQqTxu90r4E5J+nP70J3ilqVCrbho2eWaeW8Ow== +luxon@~3.4.0: + version "3.4.4" + resolved "https://registry.yarnpkg.com/luxon/-/luxon-3.4.4.tgz#cf20dc27dc532ba41a169c43fdcc0063601577af" + integrity sha512-zobTr7akeGHnv7eBOXcRgMeCP6+uyYsczwmeRCauvpvaAltgNyTbLH/+VaEAPUeWBT+1GuNmz4wC/6jtQzbbVA== + lz-string@^1.4.4: version "1.4.4" resolved "https://registry.yarnpkg.com/lz-string/-/lz-string-1.4.4.tgz#c0d8eaf36059f705796e1e344811cf4c498d3a26" From b67d9b44412dcd47f52d9b959b76fcd82343edca Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 24 Jan 2024 16:35:55 +0000 Subject: [PATCH 08/15] feat(react): Add `stripBasename` option for React Router 6. (#10314) This PR adds a new option for React Router 6 integration, `stripBasename` for leaving out the `basename` from transaction names. --- packages/react/src/reactrouterv6.tsx | 40 +++++++-- packages/react/test/reactrouterv6.4.test.tsx | 90 ++++++++++++++++++++ 2 files changed, 125 insertions(+), 5 deletions(-) diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index de87e5bb6881..c2dc56687571 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -35,6 +35,7 @@ let _createRoutesFromChildren: CreateRoutesFromChildren; let _matchRoutes: MatchRoutes; let _customStartTransaction: (context: TransactionContext) => Transaction | undefined; let _startTransactionOnLocationChange: boolean; +let _stripBasename: boolean = false; const SENTRY_TAGS = { 'routing.instrumentation': 'react-router-v6', @@ -46,6 +47,7 @@ export function reactRouterV6Instrumentation( useNavigationType: UseNavigationType, createRoutesFromChildren: CreateRoutesFromChildren, matchRoutes: MatchRoutes, + stripBasename?: boolean, ) { return ( customStartTransaction: (context: TransactionContext) => Transaction | undefined, @@ -70,12 +72,40 @@ export function reactRouterV6Instrumentation( _useNavigationType = useNavigationType; _matchRoutes = matchRoutes; _createRoutesFromChildren = createRoutesFromChildren; + _stripBasename = stripBasename || false; _customStartTransaction = customStartTransaction; _startTransactionOnLocationChange = startTransactionOnLocationChange; }; } +/** + * Strip the basename from a pathname if exists. + * + * Vendored and modified from `react-router` + * https://github.com/remix-run/react-router/blob/462bb712156a3f739d6139a0f14810b76b002df6/packages/router/utils.ts#L1038 + */ +function stripBasenameFromPathname(pathname: string, basename: string): string { + if (!basename || basename === '/') { + return pathname; + } + + if (!pathname.toLowerCase().startsWith(basename.toLowerCase())) { + return pathname; + } + + // We want to leave trailing slash behavior in the user's control, so if they + // specify a basename with a trailing slash, we should support it + const startIndex = basename.endsWith('/') ? basename.length - 1 : basename.length; + const nextChar = pathname.charAt(startIndex); + if (nextChar && nextChar !== '/') { + // pathname does not start with basename/ + return pathname; + } + + return pathname.slice(startIndex) || '/'; +} + function getNormalizedName( routes: RouteObject[], location: Location, @@ -83,7 +113,7 @@ function getNormalizedName( basename: string = '', ): [string, TransactionSource] { if (!routes || routes.length === 0) { - return [location.pathname, 'url']; + return [_stripBasename ? stripBasenameFromPathname(location.pathname, basename) : location.pathname, 'url']; } let pathBuilder = ''; @@ -95,7 +125,7 @@ function getNormalizedName( if (route) { // Early return if index route if (route.index) { - return [branch.pathname, 'route']; + return [_stripBasename ? stripBasenameFromPathname(branch.pathname, basename) : branch.pathname, 'route']; } const path = route.path; @@ -112,16 +142,16 @@ function getNormalizedName( // We should not count wildcard operators in the url segments calculation pathBuilder.slice(-2) !== '/*' ) { - return [basename + newPath, 'route']; + return [(_stripBasename ? '' : basename) + newPath, 'route']; } - return [basename + pathBuilder, 'route']; + return [(_stripBasename ? '' : basename) + pathBuilder, 'route']; } } } } } - return [location.pathname, 'url']; + return [_stripBasename ? stripBasenameFromPathname(location.pathname, basename) : location.pathname, 'url']; } function updatePageloadTransaction( diff --git a/packages/react/test/reactrouterv6.4.test.tsx b/packages/react/test/reactrouterv6.4.test.tsx index d6b9c0c45b49..29fe612f7e97 100644 --- a/packages/react/test/reactrouterv6.4.test.tsx +++ b/packages/react/test/reactrouterv6.4.test.tsx @@ -26,6 +26,7 @@ describe('React Router v6.4', () => { function createInstrumentation(_opts?: { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; + stripBasename?: boolean; }): [jest.Mock, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetAttribute: jest.Mock }] { const options = { matchPath: _opts ? matchPath : undefined, @@ -46,6 +47,7 @@ describe('React Router v6.4', () => { useNavigationType, createRoutesFromChildren, matchRoutes, + options.stripBasename, )(mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange); return [mockStartTransaction, { mockUpdateName, mockFinish, mockSetAttribute }]; } @@ -359,5 +361,93 @@ describe('React Router v6.4', () => { metadata: { source: 'route' }, }); }); + + it('strips `basename` from transaction names of parameterized paths', () => { + const [mockStartTransaction] = createInstrumentation({ + stripBasename: true, + }); + const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); + + const router = sentryCreateBrowserRouter( + [ + { + path: '/', + element: , + }, + { + path: ':orgId', + children: [ + { + path: 'users', + children: [ + { + path: ':userId', + element:
User
, + }, + ], + }, + ], + }, + ], + { + initialEntries: ['/admin'], + basename: '/admin', + }, + ); + + // @ts-expect-error router is fine + render(); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/:orgId/users/:userId', + op: 'navigation', + origin: 'auto.navigation.react.reactrouterv6', + tags: { 'routing.instrumentation': 'react-router-v6' }, + metadata: { source: 'route' }, + }); + }); + + it('strips `basename` from transaction names of non-parameterized paths', () => { + const [mockStartTransaction] = createInstrumentation({ + stripBasename: true, + }); + const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); + + const router = sentryCreateBrowserRouter( + [ + { + path: '/', + element: , + }, + { + path: 'about', + element:
About
, + children: [ + { + path: 'us', + element:
Us
, + }, + ], + }, + ], + { + initialEntries: ['/app'], + basename: '/app', + }, + ); + + // @ts-expect-error router is fine + render(); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/about/us', + op: 'navigation', + origin: 'auto.navigation.react.reactrouterv6', + tags: { 'routing.instrumentation': 'react-router-v6' }, + metadata: { source: 'route' }, + }); + }); }); }); From b146d4c2bf38620b41967b89831cc49273c2f386 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 24 Jan 2024 19:34:34 +0100 Subject: [PATCH 09/15] test(node): Add `mongoose` auto instrumentation test for `@sentry/node-experimental` (#10322) This tests v5.13.22 of mongoose because it's the most commonly use version currently supported by the OpenTelemetry plugin: https://github.com/open-telemetry/opentelemetry-js-contrib/blob/fce7d3b5e478ff7525c9ffe99e59bf35f8c06207/plugins/node/instrumentation-mongoose/src/mongoose.ts#L80 https://www.npmjs.com/package/mongoose?activeTab=versions It's worth noting that at least versions 5 and 7 of `mongoose` also generate `mongodb` spans because they use the library internally and we auto instrument everything. For some unknown reason, `mongoose@v8` does not appear to generate `mongodb` spans and I haven't tested v6 because very few people are using it. --- .../node-integration-tests/package.json | 1 + .../tracing-experimental/mongoose/scenario.js | 50 ++++++++++ .../tracing-experimental/mongoose/test.ts | 54 +++++++++++ yarn.lock | 93 ++++++++++++++++++- 4 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing-experimental/mongoose/scenario.js create mode 100644 dev-packages/node-integration-tests/suites/tracing-experimental/mongoose/test.ts diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 68012d36269b..10596903e2b2 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -41,6 +41,7 @@ "graphql": "^16.3.0", "http-terminator": "^3.2.0", "mongodb": "^3.7.3", + "mongoose": "^5.13.22", "mongodb-memory-server-global": "^7.6.3", "mysql": "^2.18.1", "nock": "^13.1.0", diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/mongoose/scenario.js b/dev-packages/node-integration-tests/suites/tracing-experimental/mongoose/scenario.js new file mode 100644 index 000000000000..47a67e4aaf78 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/mongoose/scenario.js @@ -0,0 +1,50 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node-experimental'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + debug: true, + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +// Must be required after Sentry is initialized +const mongoose = require('mongoose'); + +async function run() { + await mongoose.connect(process.env.MONGO_URL || ''); + + const Schema = mongoose.Schema; + + const BlogPostSchema = new Schema({ + title: String, + body: String, + date: Date, + }); + + const BlogPost = mongoose.model('BlogPost', BlogPostSchema); + + await Sentry.startSpan( + { + name: 'Test Transaction', + op: 'transaction', + }, + async () => { + const post = new BlogPost(); + post.title = 'Test'; + post.body = 'Test body'; + post.date = new Date(); + + await post.save(); + + await BlogPost.findOne({}); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/mongoose/test.ts b/dev-packages/node-integration-tests/suites/tracing-experimental/mongoose/test.ts new file mode 100644 index 000000000000..1a246d8ec5b9 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/mongoose/test.ts @@ -0,0 +1,54 @@ +import { MongoMemoryServer } from 'mongodb-memory-server-global'; + +import { conditionalTest } from '../../../utils'; +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +jest.setTimeout(20000); + +conditionalTest({ min: 14 })('Mongoose experimental Test', () => { + let mongoServer: MongoMemoryServer; + + beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + process.env.MONGO_URL = mongoServer.getUri(); + }, 10000); + + afterAll(async () => { + if (mongoServer) { + await mongoServer.stop(); + } + cleanupChildProcesses(); + }); + + const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: expect.objectContaining({ + 'db.mongodb.collection': 'blogposts', + 'db.name': 'test', + 'db.operation': 'save', + 'db.system': 'mongoose', + }), + description: 'mongoose.BlogPost.save', + op: 'db', + origin: 'auto.db.otel.mongoose', + }), + expect.objectContaining({ + data: expect.objectContaining({ + 'db.mongodb.collection': 'blogposts', + 'db.name': 'test', + 'db.operation': 'findOne', + 'db.system': 'mongoose', + }), + description: 'mongoose.BlogPost.findOne', + op: 'db', + origin: 'auto.db.otel.mongoose', + }), + ]), + }; + + test('CJS - should auto-instrument `mongoose` package.', done => { + createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + }); +}); diff --git a/yarn.lock b/yarn.lock index daeb04398140..3825958296ed 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5739,6 +5739,13 @@ dependencies: bson "*" +"@types/bson@1.x || 4.0.x": + version "4.0.5" + resolved "https://registry.yarnpkg.com/@types/bson/-/bson-4.0.5.tgz#9e0e1d1a6f8866483f96868a9b33bc804926b1fc" + integrity sha512-vVLwMUqhYJSQ/WKcE60eFqcyuWse5fGH+NMAXHuKrUAPoryq3ATxk5o4bgYNtg5aOM4APVg7Hnb3ASqUYG0PKg== + dependencies: + "@types/node" "*" + "@types/chai-as-promised@^7.1.2": version "7.1.3" resolved "https://registry.yarnpkg.com/@types/chai-as-promised/-/chai-as-promised-7.1.3.tgz#779166b90fda611963a3adbfd00b339d03b747bd" @@ -6314,7 +6321,7 @@ resolved "https://registry.yarnpkg.com/@types/minimist/-/minimist-1.2.1.tgz#283f669ff76d7b8260df8ab7a4262cc83d988256" integrity sha512-fZQQafSREFyuZcdWFAExYjBiCL7AUCdgsk80iO0q4yihYYdcIiH28CcuPTGFgLOCC8RlW49GSQxdHwZP+I7CNg== -"@types/mongodb@^3.6.20": +"@types/mongodb@^3.5.27", "@types/mongodb@^3.6.20": version "3.6.20" resolved "https://registry.yarnpkg.com/@types/mongodb/-/mongodb-3.6.20.tgz#b7c5c580644f6364002b649af1c06c3c0454e1d2" integrity sha512-WcdpPJCakFzcWWD9juKoZbRtQxKIMYF/JIAM4JrNHrMcnJL6/a2NWjXxW7fo9hxboxxkg+icff8d7+WIEvKgYQ== @@ -9540,6 +9547,11 @@ blank-object@^1.0.1: resolved "https://registry.yarnpkg.com/blank-object/-/blank-object-1.0.2.tgz#f990793fbe9a8c8dd013fb3219420bec81d5f4b9" integrity sha1-+ZB5P76ajI3QE/syGUIL7IHV9Lk= +bluebird@3.5.1: + version "3.5.1" + resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.5.1.tgz#d9551f9de98f1fcda1e683d17ee91a0602ee2eb9" + integrity sha512-MKiLiV+I1AA596t9w1sQJ8jkiSr5+ZKi0WKrYGUn6d1Fx+Ij4tIj+m2WMQSGczs5jZVxV339chE8iwk6F64wjA== + bluebird@^3.4.6, bluebird@^3.5.1, bluebird@^3.5.3, bluebird@^3.5.5, bluebird@^3.7.2: version "3.7.2" resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.7.2.tgz#9f229c15be272454ffa973ace0dbee79a1b0c36f" @@ -19629,6 +19641,11 @@ jws@^4.0.0: jwa "^2.0.0" safe-buffer "^5.0.1" +kareem@2.3.2: + version "2.3.2" + resolved "https://registry.yarnpkg.com/kareem/-/kareem-2.3.2.tgz#78c4508894985b8d38a0dc15e1a8e11078f2ca93" + integrity sha512-STHz9P7X2L4Kwn72fA4rGyqyXdmrMSdxqHx9IXon/FXluXieaFA6KJ2upcHAHxQPQ0LeM/OjLrhFxifHewOALQ== + karma-browserstack-launcher@^1.5.1: version "1.6.0" resolved "https://registry.yarnpkg.com/karma-browserstack-launcher/-/karma-browserstack-launcher-1.6.0.tgz#2f6000647073e77ae296653b8830b279669766ef" @@ -22138,6 +22155,19 @@ mongodb-memory-server-global@^7.6.3: mongodb-memory-server-core "7.6.3" tslib "^2.3.0" +mongodb@3.7.4: + version "3.7.4" + resolved "https://registry.yarnpkg.com/mongodb/-/mongodb-3.7.4.tgz#119530d826361c3e12ac409b769796d6977037a4" + integrity sha512-K5q8aBqEXMwWdVNh94UQTwZ6BejVbFhh1uB6c5FKtPE9eUMZPUO3sRZdgIEcHSrAWmxzpG/FeODDKL388sqRmw== + dependencies: + bl "^2.2.1" + bson "^1.1.4" + denque "^1.4.1" + optional-require "^1.1.8" + safe-buffer "^5.1.2" + optionalDependencies: + saslprep "^1.0.0" + mongodb@^3.7.3: version "3.7.3" resolved "https://registry.yarnpkg.com/mongodb/-/mongodb-3.7.3.tgz#b7949cfd0adc4cc7d32d3f2034214d4475f175a5" @@ -22151,6 +22181,31 @@ mongodb@^3.7.3: optionalDependencies: saslprep "^1.0.0" +mongoose-legacy-pluralize@1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/mongoose-legacy-pluralize/-/mongoose-legacy-pluralize-1.0.2.tgz#3ba9f91fa507b5186d399fb40854bff18fb563e4" + integrity sha512-Yo/7qQU4/EyIS8YDFSeenIvXxZN+ld7YdV9LqFVQJzTLye8unujAWPZ4NWKfFA+RNjh+wvTWKY9Z3E5XM6ZZiQ== + +mongoose@^5.13.22: + version "5.13.22" + resolved "https://registry.yarnpkg.com/mongoose/-/mongoose-5.13.22.tgz#f9a6493ba5f45b7a3d5f9fce58ca9c71aedb8157" + integrity sha512-p51k/c4X/MfqeQ3I1ranlDiggLzNumZrTDD9CeezHwZxt2/btf+YZD7MCe07RAY2NgFYVMayq6jMamw02Jmf9w== + dependencies: + "@types/bson" "1.x || 4.0.x" + "@types/mongodb" "^3.5.27" + bson "^1.1.4" + kareem "2.3.2" + mongodb "3.7.4" + mongoose-legacy-pluralize "1.0.2" + mpath "0.8.4" + mquery "3.2.5" + ms "2.1.2" + optional-require "1.0.x" + regexp-clone "1.0.0" + safe-buffer "5.2.1" + sift "13.5.2" + sliced "1.0.1" + morgan@^1.10.0: version "1.10.0" resolved "https://registry.yarnpkg.com/morgan/-/morgan-1.10.0.tgz#091778abc1fc47cd3509824653dae1faab6b17d7" @@ -22179,6 +22234,22 @@ move-concurrently@^1.0.1: rimraf "^2.5.4" run-queue "^1.0.3" +mpath@0.8.4: + version "0.8.4" + resolved "https://registry.yarnpkg.com/mpath/-/mpath-0.8.4.tgz#6b566d9581621d9e931dd3b142ed3618e7599313" + integrity sha512-DTxNZomBcTWlrMW76jy1wvV37X/cNNxPW1y2Jzd4DZkAaC5ZGsm8bfGfNOthcDuRJujXLqiuS6o3Tpy0JEoh7g== + +mquery@3.2.5: + version "3.2.5" + resolved "https://registry.yarnpkg.com/mquery/-/mquery-3.2.5.tgz#8f2305632e4bb197f68f60c0cffa21aaf4060c51" + integrity sha512-VjOKHHgU84wij7IUoZzFRU07IAxd5kWJaDmyUzQlbjHjyoeK5TNeeo8ZsFDtTYnSgpW6n/nMNIHvE3u8Lbrf4A== + dependencies: + bluebird "3.5.1" + debug "3.1.0" + regexp-clone "^1.0.0" + safe-buffer "5.1.2" + sliced "1.0.1" + mri@1.1.4: version "1.1.4" resolved "https://registry.yarnpkg.com/mri/-/mri-1.1.4.tgz#7cb1dd1b9b40905f1fac053abe25b6720f44744a" @@ -23506,6 +23577,11 @@ opn@^5.5.0: dependencies: is-wsl "^1.1.0" +optional-require@1.0.x: + version "1.0.3" + resolved "https://registry.yarnpkg.com/optional-require/-/optional-require-1.0.3.tgz#275b8e9df1dc6a17ad155369c2422a440f89cb07" + integrity sha512-RV2Zp2MY2aeYK5G+B/Sps8lW5NHAzE5QClbFP15j+PWmP+T9PxlJXBOOLoSAdgwFvS4t0aMR4vpedMkbHfh0nA== + optional-require@^1.1.8: version "1.1.8" resolved "https://registry.yarnpkg.com/optional-require/-/optional-require-1.1.8.tgz#16364d76261b75d964c482b2406cb824d8ec44b7" @@ -26668,6 +26744,11 @@ regex-parser@^2.2.11: resolved "https://registry.yarnpkg.com/regex-parser/-/regex-parser-2.2.11.tgz#3b37ec9049e19479806e878cabe7c1ca83ccfe58" integrity sha512-jbD/FT0+9MBU2XAZluI7w2OBs1RBi6p9M83nkoZayQXXU9e8Robt69FcZc7wU4eJD/YFTjn1JdCk3rbMJajz8Q== +regexp-clone@1.0.0, regexp-clone@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/regexp-clone/-/regexp-clone-1.0.0.tgz#222db967623277056260b992626354a04ce9bf63" + integrity sha512-TuAasHQNamyyJ2hb97IuBEif4qBHGjPHBS64sZwytpLEqtBQ1gPJTnOaQ6qmpET16cK14kkjbazl6+p0RRv0yw== + regexp.prototype.flags@^1.2.0, regexp.prototype.flags@^1.4.3: version "1.4.3" resolved "https://registry.yarnpkg.com/regexp.prototype.flags/-/regexp.prototype.flags-1.4.3.tgz#87cab30f80f66660181a3bb7bf5981a872b367ac" @@ -28060,6 +28141,11 @@ side-channel@^1.0.4: get-intrinsic "^1.0.2" object-inspect "^1.9.0" +sift@13.5.2: + version "13.5.2" + resolved "https://registry.yarnpkg.com/sift/-/sift-13.5.2.tgz#24a715e13c617b086166cd04917d204a591c9da6" + integrity sha512-+gxdEOMA2J+AI+fVsCqeNn7Tgx3M9ZN9jdi95939l1IJ8cZsqS8sqpJyOkic2SJk+1+98Uwryt/gL6XDaV+UZA== + siginfo@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/siginfo/-/siginfo-2.0.0.tgz#32e76c70b79724e3bb567cb9d543eb858ccfaf30" @@ -28230,6 +28316,11 @@ slice-ansi@^5.0.0: ansi-styles "^6.0.0" is-fullwidth-code-point "^4.0.0" +sliced@1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/sliced/-/sliced-1.0.1.tgz#0b3a662b5d04c3177b1926bea82b03f837a2ef41" + integrity sha512-VZBmZP8WU3sMOZm1bdgTadsQbcscK0UM8oKxKVBs4XAhUo2Xxzm/OFMGBkPusxw9xL3Uy8LrzEqGqJhclsr0yA== + smart-buffer@^4.1.0, smart-buffer@^4.2.0: version "4.2.0" resolved "https://registry.yarnpkg.com/smart-buffer/-/smart-buffer-4.2.0.tgz#6e1d71fa4f18c05f7d0ff216dd16a481d0e8d9ae" From 10345d4c94aa2fd20b376f6b080c3681c7014c22 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 24 Jan 2024 18:35:15 +0000 Subject: [PATCH 10/15] test(node): Add tests for Apollo / GraphQL OpenTelemetry auto-instrumentation. (#10320) --- .../apollo-graphql/scenario.js | 52 +++++++++++++++++++ .../apollo-graphql/test.ts | 38 ++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/tracing-experimental/apollo-graphql/scenario.js create mode 100644 dev-packages/node-integration-tests/suites/tracing-experimental/apollo-graphql/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/apollo-graphql/scenario.js b/dev-packages/node-integration-tests/suites/tracing-experimental/apollo-graphql/scenario.js new file mode 100644 index 000000000000..42bb8edacb8f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/apollo-graphql/scenario.js @@ -0,0 +1,52 @@ +const Sentry = require('@sentry/node-experimental'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const { ApolloServer, gql } = require('apollo-server'); + + await Sentry.startSpan( + { + name: 'Test Transaction', + op: 'transaction', + }, + async span => { + const typeDefs = gql`type Query { hello: String }`; + + const resolvers = { + Query: { + hello: () => { + return 'Hello world!'; + }, + }, + }; + + const server = new ApolloServer({ + typeDefs, + resolvers, + }); + + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: '{hello}', + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/apollo-graphql/test.ts b/dev-packages/node-integration-tests/suites/tracing-experimental/apollo-graphql/test.ts new file mode 100644 index 000000000000..dc7c304484f9 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/apollo-graphql/test.ts @@ -0,0 +1,38 @@ +import { conditionalTest } from '../../../utils'; +import { createRunner } from '../../../utils/runner'; + +conditionalTest({ min: 14 })('GraphQL/Apollo Tests', () => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'graphql.operation.type': 'query', + 'graphql.source': '{hello}', + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'query', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + expect.objectContaining({ + data: { + 'graphql.field.name': 'hello', + 'graphql.field.path': 'hello', + 'graphql.field.type': 'String', + 'graphql.source': 'hello', + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, + description: 'graphql.resolve', + status: 'ok', + origin: 'manual', + }), + ]), + }; + + test('CJS - should instrument GraphQL queries used from Apollo Server.', done => { + createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + }); +}); From f613940ae40421d246c78d6c5118d73709931f65 Mon Sep 17 00:00:00 2001 From: Shubhdeep Chhabra Date: Thu, 25 Jan 2024 01:36:05 +0530 Subject: [PATCH 11/15] fix(replay): Fix type for options of replayIntegration (#10325) Before submitting a pull request, please take a look at our [Contributing](https://github.com/getsentry/sentry-javascript/blob/master/CONTRIBUTING.md) guidelines and verify: - [ ] If you've added code that should be tested, please add tests. - [ ] Ensure your code lints and the test suite passes (`yarn lint`) & (`yarn test`). fixes: #10305 --- packages/replay/src/integration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 9f4d03e7f5fa..3434888f6574 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -30,7 +30,7 @@ let _initialized = false; type InitialReplayPluginOptions = Omit & Partial>; -export const replayIntegration = ((options?: InitialReplayPluginOptions) => { +export const replayIntegration = ((options?: ReplayConfiguration) => { // eslint-disable-next-line deprecation/deprecation return new Replay(options); }) satisfies IntegrationFn; From 0d0998380d237da3f1788c4318281055d3688b98 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 24 Jan 2024 15:21:24 -0500 Subject: [PATCH 12/15] feat(tracing): Revert browser tracing integration changes (#10328) Due to https://github.com/getsentry/sentry-javascript/pull/10327, this reverts changes to the browser tracing integration as we're actually going to replace this differently. --- MIGRATION.md | 11 -- packages/astro/src/client/sdk.ts | 4 +- packages/astro/test/client/sdk.test.ts | 7 +- packages/browser/src/index.bundle.feedback.ts | 11 +- packages/browser/src/index.bundle.replay.ts | 4 - .../index.bundle.tracing.replay.feedback.ts | 5 +- .../src/index.bundle.tracing.replay.ts | 5 +- packages/browser/src/index.bundle.tracing.ts | 5 +- packages/browser/src/index.bundle.ts | 4 - packages/browser/src/index.ts | 2 - .../test/unit/profiling/integration.test.ts | 2 +- packages/core/src/tracing/trace.ts | 113 +++++++++++- .../sentry-performance.ts | 4 +- packages/ember/addon/types.ts | 4 +- packages/gatsby/src/utils/integrations.ts | 6 +- packages/gatsby/test/gatsby-browser.test.ts | 4 +- packages/gatsby/test/sdk.test.ts | 10 +- .../integration-shims/src/BrowserTracing.ts | 19 +- packages/integration-shims/src/index.ts | 9 +- .../src/client/browserTracingIntegration.ts | 13 -- packages/nextjs/src/client/index.ts | 15 +- packages/nextjs/test/clientSdk.test.ts | 27 +-- .../src/client/browserTracingIntegration.ts | 13 -- packages/sveltekit/src/client/sdk.ts | 8 +- packages/sveltekit/test/client/sdk.test.ts | 6 +- .../src/browser/browsertracing.ts | 128 +++---------- .../tracing-internal/src/browser/index.ts | 7 +- packages/tracing-internal/src/index.ts | 2 - .../test/browser/browsertracing.test.ts | 174 ++++++------------ packages/tracing/src/index.ts | 2 - packages/types/src/index.ts | 1 - packages/types/src/startSpanOptions.ts | 104 ----------- 32 files changed, 241 insertions(+), 488 deletions(-) delete mode 100644 packages/types/src/startSpanOptions.ts diff --git a/MIGRATION.md b/MIGRATION.md index 063788881242..4c0ea3eddc91 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -10,16 +10,6 @@ npx @sentry/migr8@latest This will let you select which updates to run, and automatically update your code. Make sure to still review all code changes! -## Deprecate transaction-related options to `BrowserTracing` - -When configuring the `BrowserTracing` integration, some options have been renamed: - -- `startTransactionOnPageLoad` --> `spanOnPageLoad` -- `startTransactionOnLocationChange` --> `spanOnLocationChange` -- `markBackgroundTransactions` --> `markBackgroundSpan` - -Also, `beforeNavigate` is replaced with a `beforeStartSpan` callback, which receives `StartSpanOptions`. - ## Deprecate using `getClient()` to check if the SDK was initialized In v8, `getClient()` will stop returning `undefined` if `Sentry.init()` was not called. For cases where this may be used @@ -46,7 +36,6 @@ The following list shows how integrations should be migrated: | Old | New | Packages | | ------------------------- | -------------------------------- | ------------------------------------------------------------------------------------------------------- | -| `new BrowserTracing()` | `browserTracingIntegration()` | `@sentry/browser` | | `new InboundFilters()` | `inboundFiltersIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | | `new FunctionToString()` | `functionToStringIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | | `new LinkedErrors()` | `linkedErrorsIntegration()` | `@sentry/core`, `@sentry/browser`, `@sentry/node`, `@sentry/deno`, `@sentry/bun`, `@sentry/vercel-edge` | diff --git a/packages/astro/src/client/sdk.ts b/packages/astro/src/client/sdk.ts index a289296c2ab2..8d2b70ee6751 100644 --- a/packages/astro/src/client/sdk.ts +++ b/packages/astro/src/client/sdk.ts @@ -1,6 +1,6 @@ import type { BrowserOptions } from '@sentry/browser'; import { - browserTracingIntegration, + BrowserTracing, getDefaultIntegrations as getBrowserDefaultIntegrations, init as initBrowserSdk, setTag, @@ -34,7 +34,7 @@ function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefi // in which case everything inside will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - return [...getBrowserDefaultIntegrations(options), browserTracingIntegration()]; + return [...getBrowserDefaultIntegrations(options), new BrowserTracing()]; } } diff --git a/packages/astro/test/client/sdk.test.ts b/packages/astro/test/client/sdk.test.ts index 4996df46ca39..2e10d4210953 100644 --- a/packages/astro/test/client/sdk.test.ts +++ b/packages/astro/test/client/sdk.test.ts @@ -1,7 +1,7 @@ -import type { BrowserClient, BrowserTracing } from '@sentry/browser'; +import type { BrowserClient } from '@sentry/browser'; import { getCurrentScope } from '@sentry/browser'; import * as SentryBrowser from '@sentry/browser'; -import { SDK_VERSION, WINDOW, browserTracingIntegration, getClient } from '@sentry/browser'; +import { BrowserTracing, SDK_VERSION, WINDOW, getClient } from '@sentry/browser'; import { vi } from 'vitest'; import { init } from '../../../astro/src/client/sdk'; @@ -103,13 +103,12 @@ describe('Sentry client SDK', () => { it('Overrides the automatically default BrowserTracing instance with a a user-provided instance', () => { init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [browserTracingIntegration({ finalTimeout: 10, startTransactionOnLocationChange: false })], + integrations: [new BrowserTracing({ finalTimeout: 10, startTransactionOnLocationChange: false })], enableTracing: true, }); const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations; - // eslint-disable-next-line deprecation/deprecation const browserTracing = getClient()?.getIntegrationByName('BrowserTracing') as BrowserTracing; const options = browserTracing.options; diff --git a/packages/browser/src/index.bundle.feedback.ts b/packages/browser/src/index.bundle.feedback.ts index 8e653c2d4757..5d3612106286 100644 --- a/packages/browser/src/index.bundle.feedback.ts +++ b/packages/browser/src/index.bundle.feedback.ts @@ -1,12 +1,6 @@ // This is exported so the loader does not fail when switching off Replay/Tracing import { Feedback, feedbackIntegration } from '@sentry-internal/feedback'; -import { - BrowserTracing, - Replay, - addTracingExtensions, - browserTracingIntegration, - replayIntegration, -} from '@sentry-internal/integration-shims'; +import { BrowserTracing, Replay, addTracingExtensions, replayIntegration } from '@sentry-internal/integration-shims'; import * as Sentry from './index.bundle.base'; @@ -14,14 +8,11 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; -// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; export { - // eslint-disable-next-line deprecation/deprecation BrowserTracing, - browserTracingIntegration, addTracingExtensions, // eslint-disable-next-line deprecation/deprecation Replay, diff --git a/packages/browser/src/index.bundle.replay.ts b/packages/browser/src/index.bundle.replay.ts index 2e4619ab49ea..2609e7d9b48c 100644 --- a/packages/browser/src/index.bundle.replay.ts +++ b/packages/browser/src/index.bundle.replay.ts @@ -3,7 +3,6 @@ import { BrowserTracing, Feedback, addTracingExtensions, - browserTracingIntegration, feedbackIntegration, } from '@sentry-internal/integration-shims'; import { Replay, replayIntegration } from '@sentry/replay'; @@ -14,14 +13,11 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; -// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; export { - // eslint-disable-next-line deprecation/deprecation BrowserTracing, - browserTracingIntegration, addTracingExtensions, // eslint-disable-next-line deprecation/deprecation Replay, diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.ts index 5822a2379cc4..e17c7de4159a 100644 --- a/packages/browser/src/index.bundle.tracing.replay.feedback.ts +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.ts @@ -1,5 +1,5 @@ import { Feedback, feedbackIntegration } from '@sentry-internal/feedback'; -import { BrowserTracing, Span, addExtensionMethods, browserTracingIntegration } from '@sentry-internal/tracing'; +import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; import { Replay, replayIntegration } from '@sentry/replay'; import * as Sentry from './index.bundle.base'; @@ -10,7 +10,6 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; -// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods @@ -23,9 +22,7 @@ export { Replay, feedbackIntegration, replayIntegration, - // eslint-disable-next-line deprecation/deprecation BrowserTracing, - browserTracingIntegration, Span, addExtensionMethods, }; diff --git a/packages/browser/src/index.bundle.tracing.replay.ts b/packages/browser/src/index.bundle.tracing.replay.ts index 41ea77b8d982..5dc0537be064 100644 --- a/packages/browser/src/index.bundle.tracing.replay.ts +++ b/packages/browser/src/index.bundle.tracing.replay.ts @@ -1,5 +1,5 @@ import { Feedback, feedbackIntegration } from '@sentry-internal/integration-shims'; -import { BrowserTracing, Span, addExtensionMethods, browserTracingIntegration } from '@sentry-internal/tracing'; +import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; import { Replay, replayIntegration } from '@sentry/replay'; import * as Sentry from './index.bundle.base'; @@ -10,7 +10,6 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; -// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods @@ -23,9 +22,7 @@ export { Replay, replayIntegration, feedbackIntegration, - // eslint-disable-next-line deprecation/deprecation BrowserTracing, - browserTracingIntegration, Span, addExtensionMethods, }; diff --git a/packages/browser/src/index.bundle.tracing.ts b/packages/browser/src/index.bundle.tracing.ts index 6607dacf992c..f810b61b92a7 100644 --- a/packages/browser/src/index.bundle.tracing.ts +++ b/packages/browser/src/index.bundle.tracing.ts @@ -1,6 +1,6 @@ // This is exported so the loader does not fail when switching off Replay import { Feedback, Replay, feedbackIntegration, replayIntegration } from '@sentry-internal/integration-shims'; -import { BrowserTracing, Span, addExtensionMethods, browserTracingIntegration } from '@sentry-internal/tracing'; +import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; import * as Sentry from './index.bundle.base'; @@ -10,7 +10,6 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; -// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods @@ -23,9 +22,7 @@ export { Replay, feedbackIntegration, replayIntegration, - // eslint-disable-next-line deprecation/deprecation BrowserTracing, - browserTracingIntegration, Span, addExtensionMethods, }; diff --git a/packages/browser/src/index.bundle.ts b/packages/browser/src/index.bundle.ts index 3087d7d317ca..a92ff6bf66ec 100644 --- a/packages/browser/src/index.bundle.ts +++ b/packages/browser/src/index.bundle.ts @@ -4,7 +4,6 @@ import { Feedback, Replay, addTracingExtensions, - browserTracingIntegration, feedbackIntegration, replayIntegration, } from '@sentry-internal/integration-shims'; @@ -15,19 +14,16 @@ import * as Sentry from './index.bundle.base'; // eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; -// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; export { - // eslint-disable-next-line deprecation/deprecation BrowserTracing, addTracingExtensions, // eslint-disable-next-line deprecation/deprecation Replay, // eslint-disable-next-line deprecation/deprecation Feedback, - browserTracingIntegration, feedbackIntegration, replayIntegration, }; diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 8fbf60bf1bd7..19c377fc5931 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -54,9 +54,7 @@ export { } from '@sentry-internal/feedback'; export { - // eslint-disable-next-line deprecation/deprecation BrowserTracing, - browserTracingIntegration, defaultRequestInstrumentationOptions, instrumentOutgoingRequests, } from '@sentry-internal/tracing'; diff --git a/packages/browser/test/unit/profiling/integration.test.ts b/packages/browser/test/unit/profiling/integration.test.ts index d7f13a3848f6..b69d3a52d655 100644 --- a/packages/browser/test/unit/profiling/integration.test.ts +++ b/packages/browser/test/unit/profiling/integration.test.ts @@ -44,7 +44,7 @@ describe('BrowserProfilingIntegration', () => { send, }; }, - integrations: [Sentry.browserTracingIntegration(), new BrowserProfilingIntegration()], + integrations: [new Sentry.BrowserTracing(), new BrowserProfilingIntegration()], }); const client = Sentry.getClient(); diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 885cbd7c9d08..eb070510a3b7 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -1,5 +1,15 @@ -import type { Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types'; - +import type { + Instrumenter, + Primitive, + Scope, + Span, + SpanTimeInput, + TransactionContext, + TransactionMetadata, +} from '@sentry/types'; +import type { SpanAttributes } from '@sentry/types'; +import type { SpanOrigin } from '@sentry/types'; +import type { TransactionSource } from '@sentry/types'; import { dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; @@ -10,6 +20,105 @@ import { handleCallbackErrors } from '../utils/handleCallbackErrors'; import { hasTracingEnabled } from '../utils/hasTracingEnabled'; import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; +interface StartSpanOptions extends TransactionContext { + /** A manually specified start time for the created `Span` object. */ + startTime?: SpanTimeInput; + + /** If defined, start this span off this scope instead off the current scope. */ + scope?: Scope; + + /** The name of the span. */ + name: string; + + /** An op for the span. This is a categorization for spans. */ + op?: string; + + /** The origin of the span - if it comes from auto instrumenation or manual instrumentation. */ + origin?: SpanOrigin; + + /** Attributes for the span. */ + attributes?: SpanAttributes; + + // All remaining fields are deprecated + + /** + * @deprecated Manually set the end timestamp instead. + */ + trimEnd?: boolean; + + /** + * @deprecated This cannot be set manually anymore. + */ + parentSampled?: boolean; + + /** + * @deprecated Use attributes or set data on scopes instead. + */ + metadata?: Partial; + + /** + * The name thingy. + * @deprecated Use `name` instead. + */ + description?: string; + + /** + * @deprecated Use `span.setStatus()` instead. + */ + status?: string; + + /** + * @deprecated Use `scope` instead. + */ + parentSpanId?: string; + + /** + * @deprecated You cannot manually set the span to sampled anymore. + */ + sampled?: boolean; + + /** + * @deprecated You cannot manually set the spanId anymore. + */ + spanId?: string; + + /** + * @deprecated You cannot manually set the traceId anymore. + */ + traceId?: string; + + /** + * @deprecated Use an attribute instead. + */ + source?: TransactionSource; + + /** + * @deprecated Use attributes or set tags on the scope instead. + */ + tags?: { [key: string]: Primitive }; + + /** + * @deprecated Use attributes instead. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + data?: { [key: string]: any }; + + /** + * @deprecated Use `startTime` instead. + */ + startTimestamp?: number; + + /** + * @deprecated Use `span.end()` instead. + */ + endTimestamp?: number; + + /** + * @deprecated You cannot set the instrumenter manually anymore. + */ + instrumenter?: Instrumenter; +} + /** * Wraps a function with a transaction/span and finishes the span after the function is done. * diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index d6f5bf61a053..b25125b28da6 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -404,11 +404,11 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) // Maintaining backwards compatibility with config.browserTracingOptions, but passing it with Sentry options is preferred. const browserTracingOptions = config.browserTracingOptions || config.sentry.browserTracingOptions || {}; - const { browserTracingIntegration } = await import('@sentry/browser'); + const { BrowserTracing } = await import('@sentry/browser'); const idleTimeout = config.transitionTimeout || 5000; - const browserTracing = browserTracingIntegration({ + const browserTracing = new BrowserTracing({ routingInstrumentation: (customStartTransaction, startTransactionOnPageLoad) => { // eslint-disable-next-line ember/no-private-routing-service const routerMain = appInstance.lookup('router:main') as EmberRouterMain; diff --git a/packages/ember/addon/types.ts b/packages/ember/addon/types.ts index dbd14ff96f70..787eecc7e4cf 100644 --- a/packages/ember/addon/types.ts +++ b/packages/ember/addon/types.ts @@ -1,7 +1,7 @@ -import type { BrowserOptions, browserTracingIntegration } from '@sentry/browser'; +import type { BrowserOptions, BrowserTracing } from '@sentry/browser'; import type { Transaction, TransactionContext } from '@sentry/types'; -type BrowserTracingOptions = Parameters[0]; +type BrowserTracingOptions = ConstructorParameters[0]; export type EmberSentryConfig = { sentry: BrowserOptions & { browserTracingOptions?: BrowserTracingOptions }; diff --git a/packages/gatsby/src/utils/integrations.ts b/packages/gatsby/src/utils/integrations.ts index 7c61adee1d50..94ef28f21272 100644 --- a/packages/gatsby/src/utils/integrations.ts +++ b/packages/gatsby/src/utils/integrations.ts @@ -1,5 +1,5 @@ import { hasTracingEnabled } from '@sentry/core'; -import { browserTracingIntegration } from '@sentry/react'; +import { BrowserTracing } from '@sentry/react'; import type { Integration } from '@sentry/types'; import type { GatsbyOptions } from './types'; @@ -31,8 +31,8 @@ export function getIntegrationsFromOptions(options: GatsbyOptions): UserIntegrat * @param isTracingEnabled Whether the user has enabled tracing. */ function getIntegrationsFromArray(userIntegrations: Integration[], isTracingEnabled: boolean): Integration[] { - if (isTracingEnabled && !userIntegrations.some(integration => integration.name === 'BrowserTracing')) { - userIntegrations.push(browserTracingIntegration()); + if (isTracingEnabled && !userIntegrations.some(integration => integration.name === BrowserTracing.name)) { + userIntegrations.push(new BrowserTracing()); } return userIntegrations; } diff --git a/packages/gatsby/test/gatsby-browser.test.ts b/packages/gatsby/test/gatsby-browser.test.ts index cf456a9d3e9d..b67305042c71 100644 --- a/packages/gatsby/test/gatsby-browser.test.ts +++ b/packages/gatsby/test/gatsby-browser.test.ts @@ -2,7 +2,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { onClientEntry } from '../gatsby-browser'; -import { browserTracingIntegration } from '../src/index'; +import { BrowserTracing } from '../src/index'; (global as any).__SENTRY_RELEASE__ = '683f3a6ab819d47d23abfca9a914c81f0524d35b'; (global as any).__SENTRY_DSN__ = 'https://examplePublicKey@o0.ingest.sentry.io/0'; @@ -141,7 +141,7 @@ describe('onClientEntry', () => { }); it('only defines a single `BrowserTracing` integration', () => { - const integrations = [browserTracingIntegration()]; + const integrations = [new BrowserTracing()]; onClientEntry(undefined, { tracesSampleRate: 0.5, integrations }); expect(sentryInit).toHaveBeenLastCalledWith( diff --git a/packages/gatsby/test/sdk.test.ts b/packages/gatsby/test/sdk.test.ts index 28206d1ef6c5..c3c95cdabc1f 100644 --- a/packages/gatsby/test/sdk.test.ts +++ b/packages/gatsby/test/sdk.test.ts @@ -1,4 +1,4 @@ -import { SDK_VERSION, browserTracingIntegration, init } from '@sentry/react'; +import { BrowserTracing, SDK_VERSION, init } from '@sentry/react'; import type { Integration } from '@sentry/types'; import { init as gatsbyInit } from '../src/sdk'; @@ -68,27 +68,27 @@ describe('Integrations from options', () => { [ 'tracing disabled, with BrowserTracing as an array', [], - { integrations: [browserTracingIntegration()] }, + { integrations: [new BrowserTracing()] }, ['BrowserTracing'], ], [ 'tracing disabled, with BrowserTracing as a function', [], { - integrations: () => [browserTracingIntegration()], + integrations: () => [new BrowserTracing()], }, ['BrowserTracing'], ], [ 'tracing enabled, with BrowserTracing as an array', [], - { tracesSampleRate: 1, integrations: [browserTracingIntegration()] }, + { tracesSampleRate: 1, integrations: [new BrowserTracing()] }, ['BrowserTracing'], ], [ 'tracing enabled, with BrowserTracing as a function', [], - { tracesSampleRate: 1, integrations: () => [browserTracingIntegration()] }, + { tracesSampleRate: 1, integrations: () => [new BrowserTracing()] }, ['BrowserTracing'], ], ] as TestArgs[])( diff --git a/packages/integration-shims/src/BrowserTracing.ts b/packages/integration-shims/src/BrowserTracing.ts index 94936b004903..310dc589afe9 100644 --- a/packages/integration-shims/src/BrowserTracing.ts +++ b/packages/integration-shims/src/BrowserTracing.ts @@ -5,8 +5,6 @@ import { consoleSandbox } from '@sentry/utils'; * This is a shim for the BrowserTracing integration. * It is needed in order for the CDN bundles to continue working when users add/remove tracing * from it, without changing their config. This is necessary for the loader mechanism. - * - * @deprecated Use `browserTracingIntegration()` instead. */ class BrowserTracingShim implements Integration { /** @@ -21,7 +19,6 @@ class BrowserTracingShim implements Integration { // eslint-disable-next-line @typescript-eslint/no-explicit-any public constructor(_options: any) { - // eslint-disable-next-line deprecation/deprecation this.name = BrowserTracingShim.id; consoleSandbox(() => { @@ -36,21 +33,7 @@ class BrowserTracingShim implements Integration { } } -/** - * This is a shim for the BrowserTracing integration. - * It is needed in order for the CDN bundles to continue working when users add/remove tracing - * from it, without changing their config. This is necessary for the loader mechanism. - */ -function browserTracingIntegrationShim(): Integration { - // eslint-disable-next-line deprecation/deprecation - return new BrowserTracingShim({}); -} - -export { - // eslint-disable-next-line deprecation/deprecation - BrowserTracingShim as BrowserTracing, - browserTracingIntegrationShim as browserTracingIntegration, -}; +export { BrowserTracingShim as BrowserTracing }; /** Shim function */ export function addTracingExtensions(): void { diff --git a/packages/integration-shims/src/index.ts b/packages/integration-shims/src/index.ts index bffdf82c99f7..43243f69a194 100644 --- a/packages/integration-shims/src/index.ts +++ b/packages/integration-shims/src/index.ts @@ -3,16 +3,9 @@ export { Feedback, feedbackIntegration, } from './Feedback'; - export { // eslint-disable-next-line deprecation/deprecation Replay, replayIntegration, } from './Replay'; - -export { - // eslint-disable-next-line deprecation/deprecation - BrowserTracing, - browserTracingIntegration, - addTracingExtensions, -} from './BrowserTracing'; +export { BrowserTracing, addTracingExtensions } from './BrowserTracing'; diff --git a/packages/nextjs/src/client/browserTracingIntegration.ts b/packages/nextjs/src/client/browserTracingIntegration.ts index b7c6f33159eb..c3eb18887301 100644 --- a/packages/nextjs/src/client/browserTracingIntegration.ts +++ b/packages/nextjs/src/client/browserTracingIntegration.ts @@ -1,14 +1,10 @@ import { BrowserTracing as OriginalBrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/react'; -import type { Integration } from '@sentry/types'; import { nextRouterInstrumentation } from '../index.client'; /** * A custom BrowserTracing integration for Next.js. - * @deprecated Use `browserTracingIntegration()` instead. */ -// eslint-disable-next-line deprecation/deprecation export class BrowserTracing extends OriginalBrowserTracing { - // eslint-disable-next-line deprecation/deprecation public constructor(options?: ConstructorParameters[0]) { super({ // eslint-disable-next-line deprecation/deprecation @@ -28,12 +24,3 @@ export class BrowserTracing extends OriginalBrowserTracing { }); } } - -/** - * A custom BrowserTracing integration for Next.js. - */ -// eslint-disable-next-line deprecation/deprecation -export function browserTracingIntegration(options?: ConstructorParameters[0]): Integration { - // eslint-disable-next-line deprecation/deprecation - return new BrowserTracing(options); -} diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index 60c1c5d2a7f8..d1d5e1db7ff5 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -10,7 +10,7 @@ import type { EventProcessor, Integration } from '@sentry/types'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; -import { BrowserTracing, browserTracingIntegration } from './browserTracingIntegration'; +import { BrowserTracing } from './browserTracingIntegration'; import { rewriteFramesIntegration } from './rewriteFramesIntegration'; import { applyTunnelRouteOption } from './tunnelRoute'; @@ -35,12 +35,7 @@ export const Integrations = { // // import { BrowserTracing } from '@sentry/nextjs'; // const instance = new BrowserTracing(); -export { - // eslint-disable-next-line deprecation/deprecation - BrowserTracing, - browserTracingIntegration, - rewriteFramesIntegration, -}; +export { BrowserTracing, rewriteFramesIntegration }; // Treeshakable guard to remove all code related to tracing declare const __SENTRY_TRACING__: boolean; @@ -95,15 +90,13 @@ function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Inte const browserTracing = integrations.find(integration => integration.name === 'BrowserTracing'); // If BrowserTracing was added, but it is not our forked version, // replace it with our forked version with the same options - // eslint-disable-next-line deprecation/deprecation if (browserTracing && !(browserTracing instanceof BrowserTracing)) { - // eslint-disable-next-line deprecation/deprecation const options: ConstructorParameters[0] = (browserTracing as BrowserTracing).options; // These two options are overwritten by the custom integration delete options.routingInstrumentation; // eslint-disable-next-line deprecation/deprecation delete options.tracingOrigins; - integrations[integrations.indexOf(browserTracing)] = browserTracingIntegration(options); + integrations[integrations.indexOf(browserTracing)] = new BrowserTracing(options); } return integrations; @@ -116,7 +109,7 @@ function getDefaultIntegrations(options: BrowserOptions): Integration[] { // will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - customDefaultIntegrations.push(browserTracingIntegration()); + customDefaultIntegrations.push(new BrowserTracing()); } } diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts index 6de340dfc3e1..464b7db14dc7 100644 --- a/packages/nextjs/test/clientSdk.test.ts +++ b/packages/nextjs/test/clientSdk.test.ts @@ -6,8 +6,7 @@ import type { Integration } from '@sentry/types'; import { logger } from '@sentry/utils'; import { JSDOM } from 'jsdom'; -import type { BrowserTracing } from '../src/client'; -import { breadcrumbsIntegration, browserTracingIntegration, init, nextRouterInstrumentation } from '../src/client'; +import { BrowserTracing, breadcrumbsIntegration, init, nextRouterInstrumentation } from '../src/client'; const reactInit = jest.spyOn(SentryReact, 'init'); const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent'); @@ -125,7 +124,6 @@ describe('Client init()', () => { }); const client = getClient()!; - // eslint-disable-next-line deprecation/deprecation const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeDefined(); @@ -143,7 +141,6 @@ describe('Client init()', () => { }); const client = getClient()!; - // eslint-disable-next-line deprecation/deprecation const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeDefined(); @@ -160,7 +157,6 @@ describe('Client init()', () => { }); const client = getClient()!; - // eslint-disable-next-line deprecation/deprecation const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeUndefined(); @@ -170,15 +166,14 @@ describe('Client init()', () => { init({ dsn: TEST_DSN, tracesSampleRate: 1.0, - integrations: [browserTracingIntegration({ startTransactionOnLocationChange: false })], + integrations: [new BrowserTracing({ startTransactionOnLocationChange: false })], }); const client = getClient()!; - // eslint-disable-next-line deprecation/deprecation - const integration = client.getIntegrationByName('BrowserTracing'); + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); - expect(integration).toBeDefined(); - expect(integration?.options).toEqual( + expect(browserTracingIntegration).toBeDefined(); + expect(browserTracingIntegration?.options).toEqual( expect.objectContaining({ routingInstrumentation: nextRouterInstrumentation, // This proves it's still the user's copy @@ -191,19 +186,15 @@ describe('Client init()', () => { init({ dsn: TEST_DSN, tracesSampleRate: 1.0, - integrations: defaults => [ - ...defaults, - browserTracingIntegration({ startTransactionOnLocationChange: false }), - ], + integrations: defaults => [...defaults, new BrowserTracing({ startTransactionOnLocationChange: false })], }); const client = getClient()!; - // eslint-disable-next-line deprecation/deprecation - const integration = client.getIntegrationByName('BrowserTracing'); + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); - expect(integration).toBeDefined(); - expect(integration?.options).toEqual( + expect(browserTracingIntegration).toBeDefined(); + expect(browserTracingIntegration?.options).toEqual( expect.objectContaining({ routingInstrumentation: nextRouterInstrumentation, // This proves it's still the user's copy diff --git a/packages/sveltekit/src/client/browserTracingIntegration.ts b/packages/sveltekit/src/client/browserTracingIntegration.ts index d6b5e0bde95f..9968f8b6de5f 100644 --- a/packages/sveltekit/src/client/browserTracingIntegration.ts +++ b/packages/sveltekit/src/client/browserTracingIntegration.ts @@ -1,14 +1,10 @@ import { BrowserTracing as OriginalBrowserTracing } from '@sentry/svelte'; -import type { Integration } from '@sentry/types'; import { svelteKitRoutingInstrumentation } from './router'; /** * A custom BrowserTracing integration for Sveltekit. - * @deprecated Use `browserTracingIntegration()` instead. */ -// eslint-disable-next-line deprecation/deprecation export class BrowserTracing extends OriginalBrowserTracing { - // eslint-disable-next-line deprecation/deprecation public constructor(options?: ConstructorParameters[0]) { super({ routingInstrumentation: svelteKitRoutingInstrumentation, @@ -16,12 +12,3 @@ export class BrowserTracing extends OriginalBrowserTracing { }); } } - -/** - * A custom BrowserTracing integration for Sveltekit. - */ -// eslint-disable-next-line deprecation/deprecation -export function browserTracingIntegration(options?: ConstructorParameters[0]): Integration { - // eslint-disable-next-line deprecation/deprecation - return new BrowserTracing(options); -} diff --git a/packages/sveltekit/src/client/sdk.ts b/packages/sveltekit/src/client/sdk.ts index c8894605f53d..7b9c608a862d 100644 --- a/packages/sveltekit/src/client/sdk.ts +++ b/packages/sveltekit/src/client/sdk.ts @@ -4,7 +4,7 @@ import { getDefaultIntegrations as getDefaultSvelteIntegrations } from '@sentry/ import { WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte'; import type { Integration } from '@sentry/types'; -import { BrowserTracing, browserTracingIntegration } from './browserTracingIntegration'; +import { BrowserTracing } from './browserTracingIntegration'; type WindowWithSentryFetchProxy = typeof WINDOW & { _sentryFetchProxy?: typeof fetch; @@ -65,13 +65,11 @@ function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Inte const browserTracing = integrations.find(integration => integration.name === 'BrowserTracing'); // If BrowserTracing was added, but it is not our forked version, // replace it with our forked version with the same options - // eslint-disable-next-line deprecation/deprecation if (browserTracing && !(browserTracing instanceof BrowserTracing)) { - // eslint-disable-next-line deprecation/deprecation const options: ConstructorParameters[0] = (browserTracing as BrowserTracing).options; // This option is overwritten by the custom integration delete options.routingInstrumentation; - integrations[integrations.indexOf(browserTracing)] = browserTracingIntegration(options); + integrations[integrations.indexOf(browserTracing)] = new BrowserTracing(options); } return integrations; @@ -82,7 +80,7 @@ function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefi // will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - return [...getDefaultSvelteIntegrations(options), browserTracingIntegration()]; + return [...getDefaultSvelteIntegrations(options), new BrowserTracing()]; } } diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts index 0f643c9e9b14..10292658bc54 100644 --- a/packages/sveltekit/test/client/sdk.test.ts +++ b/packages/sveltekit/test/client/sdk.test.ts @@ -4,8 +4,7 @@ import * as SentrySvelte from '@sentry/svelte'; import { SDK_VERSION, WINDOW } from '@sentry/svelte'; import { vi } from 'vitest'; -import type { BrowserTracing } from '../../src/client'; -import { browserTracingIntegration, init } from '../../src/client'; +import { BrowserTracing, init } from '../../src/client'; import { svelteKitRoutingInstrumentation } from '../../src/client/router'; const svelteInit = vi.spyOn(SentrySvelte, 'init'); @@ -101,11 +100,10 @@ describe('Sentry client SDK', () => { it('Merges a user-provided BrowserTracing integration with the automatically added one', () => { init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [browserTracingIntegration({ finalTimeout: 10, startTransactionOnLocationChange: false })], + integrations: [new BrowserTracing({ finalTimeout: 10, startTransactionOnLocationChange: false })], enableTracing: true, }); - // eslint-disable-next-line deprecation/deprecation const browserTracing = getClient()?.getIntegrationByName('BrowserTracing') as BrowserTracing; const options = browserTracing.options; diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index 28c10215118d..e9f61c73c0f3 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -1,4 +1,4 @@ -/* eslint-disable max-lines, complexity */ +/* eslint-disable max-lines */ import type { Hub, IdleTransaction } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, @@ -9,14 +9,7 @@ import { spanToJSON, startIdleTransaction, } from '@sentry/core'; -import type { - EventProcessor, - Integration, - StartSpanOptions, - Transaction, - TransactionContext, - TransactionSource, -} from '@sentry/types'; +import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; import { DEBUG_BUILD } from '../common/debug-build'; @@ -66,49 +59,28 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { */ heartbeatInterval: number; - /** - * If a span should be created on location (history) changes. - * Default: true - */ - spanOnLocationChange: boolean; - - /** - * If a span should be created on pageload. - * Default: true - */ - spanOnPageLoad: boolean; - - /** - * Flag spans where tabs moved to background with "cancelled". Browser background tab timing is - * not suited towards doing precise measurements of operations. By default, we recommend that this option - * be enabled as background transactions can mess up your statistics in nondeterministic ways. - * - * Default: true - */ - markBackgroundSpan: boolean; - /** * Flag to enable/disable creation of `navigation` transaction on history changes. + * * Default: true - * @deprecated Configure `spanOnLocationChange` instead. */ - startTransactionOnLocationChange?: boolean; + startTransactionOnLocationChange: boolean; /** * Flag to enable/disable creation of `pageload` transaction on first pageload. + * * Default: true - * @deprecated Configure `spanOnPageLoad` instead. */ - startTransactionOnPageLoad?: boolean; + startTransactionOnPageLoad: boolean; /** * Flag Transactions where tabs moved to background with "cancelled". Browser background tab timing is * not suited towards doing precise measurements of operations. By default, we recommend that this option * be enabled as background transactions can mess up your statistics in nondeterministic ways. + * * Default: true - * @deprecated Configure `markBackgroundSpan` instead. */ - markBackgroundTransactions?: boolean; + markBackgroundTransactions: boolean; /** * If true, Sentry will capture long tasks and add them to the corresponding transaction. @@ -145,12 +117,6 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { onStartRouteTransaction: (t: Transaction | undefined, ctx: TransactionContext, getCurrentHub: () => Hub) => void; }>; - /** - * A callback which is called before a span for a pageload or navigation is started. - * It receives the options passed to `startSpan`, and expects to return an updated options object. - */ - beforeStartSpan?: (options: StartSpanOptions) => StartSpanOptions; - /** * beforeNavigate is called before a pageload/navigation transaction is created and allows users to modify transaction * context data, or drop the transaction entirely (by setting `sampled = false` in the context). @@ -160,8 +126,6 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { * @param context: The context data which will be passed to `startTransaction` by default * * @returns A (potentially) modified context object, with `sampled = false` if the transaction should be dropped. - * - * @deprecated Use `beforeStartSpan` instead. */ beforeNavigate?(this: void, context: TransactionContext): TransactionContext | undefined; @@ -179,10 +143,10 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { ...TRACING_DEFAULTS, + markBackgroundTransactions: true, routingInstrumentation: instrumentRoutingWithDefaults, - spanOnLocationChange: true, - spanOnPageLoad: true, - markBackgroundSpan: true, + startTransactionOnLocationChange: true, + startTransactionOnPageLoad: true, enableLongTask: true, _experiments: {}, ...defaultRequestInstrumentationOptions, @@ -195,14 +159,6 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { * The integration can be configured with a variety of options, and can be extended to use * any routing library. This integration uses {@see IdleTransaction} to create transactions. */ -export function browserTracingIntegration(options?: Partial): Integration { - // eslint-disable-next-line deprecation/deprecation - return new BrowserTracing(options); -} - -/** - * @deprecated Use `browserTracingIntegration()` instead. - */ export class BrowserTracing implements Integration { // This class currently doesn't have a static `id` field like the other integration classes, because it prevented // @sentry/tracing from being treeshaken. Tree shakers do not like static fields, because they behave like side effects. @@ -226,7 +182,7 @@ export class BrowserTracing implements Integration { private _hasSetTracePropagationTargets: boolean; - public constructor(_options: Partial = {}) { + public constructor(_options?: Partial) { this.name = BROWSER_TRACING_INTEGRATION_ID; this._hasSetTracePropagationTargets = false; @@ -234,34 +190,12 @@ export class BrowserTracing implements Integration { if (DEBUG_BUILD) { this._hasSetTracePropagationTargets = !!( + _options && // eslint-disable-next-line deprecation/deprecation (_options.tracePropagationTargets || _options.tracingOrigins) ); } - // Migrate legacy options - // TODO v8: Remove this - /* eslint-disable deprecation/deprecation */ - if (typeof _options.startTransactionOnPageLoad === 'boolean') { - _options.spanOnPageLoad = _options.startTransactionOnPageLoad; - } - if (typeof _options.startTransactionOnLocationChange === 'boolean') { - _options.spanOnLocationChange = _options.startTransactionOnLocationChange; - } - if (typeof _options.markBackgroundTransactions === 'boolean') { - _options.markBackgroundSpan = _options.markBackgroundTransactions; - } - /* eslint-enable deprecation/deprecation */ - - // TODO (v8): remove this block after tracingOrigins is removed - // Set tracePropagationTargets to tracingOrigins if specified by the user - // In case both are specified, tracePropagationTargets takes precedence - // eslint-disable-next-line deprecation/deprecation - if (!_options.tracePropagationTargets && _options.tracingOrigins) { - // eslint-disable-next-line deprecation/deprecation - _options.tracePropagationTargets = _options.tracingOrigins; - } - this.options = { ...DEFAULT_BROWSER_TRACING_OPTIONS, ..._options, @@ -273,6 +207,15 @@ export class BrowserTracing implements Integration { this.options.enableLongTask = this.options._experiments.enableLongTask; } + // TODO (v8): remove this block after tracingOrigins is removed + // Set tracePropagationTargets to tracingOrigins if specified by the user + // In case both are specified, tracePropagationTargets takes precedence + // eslint-disable-next-line deprecation/deprecation + if (_options && !_options.tracePropagationTargets && _options.tracingOrigins) { + // eslint-disable-next-line deprecation/deprecation + this.options.tracePropagationTargets = _options.tracingOrigins; + } + this._collectWebVitals = startTrackingWebVitals(); if (this.options.enableLongTask) { startTrackingLongTasks(); @@ -294,9 +237,9 @@ export class BrowserTracing implements Integration { const { routingInstrumentation: instrumentRouting, - spanOnPageLoad, - spanOnLocationChange, - markBackgroundSpan, + startTransactionOnLocationChange, + startTransactionOnPageLoad, + markBackgroundTransactions, traceFetch, traceXHR, shouldCreateSpanForRequest, @@ -332,11 +275,11 @@ export class BrowserTracing implements Integration { return transaction; }, - spanOnPageLoad, - spanOnLocationChange, + startTransactionOnPageLoad, + startTransactionOnLocationChange, ); - if (markBackgroundSpan) { + if (markBackgroundTransactions) { registerBackgroundTabDetection(); } @@ -363,14 +306,7 @@ export class BrowserTracing implements Integration { const hub = this._getCurrentHub(); - const { - // eslint-disable-next-line deprecation/deprecation - beforeNavigate, - beforeStartSpan, - idleTimeout, - finalTimeout, - heartbeatInterval, - } = this.options; + const { beforeNavigate, idleTimeout, finalTimeout, heartbeatInterval } = this.options; const isPageloadTransaction = context.op === 'pageload'; @@ -392,11 +328,7 @@ export class BrowserTracing implements Integration { trimEnd: true, }; - const contextAfterProcessing = beforeStartSpan ? beforeStartSpan(expandedContext) : expandedContext; - - const modifiedContext = - // eslint-disable-next-line deprecation/deprecation - typeof beforeNavigate === 'function' ? beforeNavigate(contextAfterProcessing) : contextAfterProcessing; + const modifiedContext = typeof beforeNavigate === 'function' ? beforeNavigate(expandedContext) : expandedContext; // For backwards compatibility reasons, beforeNavigate can return undefined to "drop" the transaction (prevent it // from being sent to Sentry). diff --git a/packages/tracing-internal/src/browser/index.ts b/packages/tracing-internal/src/browser/index.ts index d9a17641b2b4..5b30bc519404 100644 --- a/packages/tracing-internal/src/browser/index.ts +++ b/packages/tracing-internal/src/browser/index.ts @@ -2,12 +2,7 @@ export * from '../exports'; export type { RequestInstrumentationOptions } from './request'; -export { - // eslint-disable-next-line deprecation/deprecation - BrowserTracing, - browserTracingIntegration, - BROWSER_TRACING_INTEGRATION_ID, -} from './browsertracing'; +export { BrowserTracing, BROWSER_TRACING_INTEGRATION_ID } from './browsertracing'; export { instrumentOutgoingRequests, defaultRequestInstrumentationOptions } from './request'; export { diff --git a/packages/tracing-internal/src/index.ts b/packages/tracing-internal/src/index.ts index 5e8791018c6f..495d8dbb26b9 100644 --- a/packages/tracing-internal/src/index.ts +++ b/packages/tracing-internal/src/index.ts @@ -13,9 +13,7 @@ export { export type { LazyLoadedIntegration } from './node'; export { - // eslint-disable-next-line deprecation/deprecation BrowserTracing, - browserTracingIntegration, BROWSER_TRACING_INTEGRATION_ID, instrumentOutgoingRequests, defaultRequestInstrumentationOptions, diff --git a/packages/tracing-internal/test/browser/browsertracing.test.ts b/packages/tracing-internal/test/browser/browsertracing.test.ts index 0892169c28a2..b9830b8d754c 100644 --- a/packages/tracing-internal/test/browser/browsertracing.test.ts +++ b/packages/tracing-internal/test/browser/browsertracing.test.ts @@ -85,6 +85,61 @@ conditionalTest({ min: 10 })('BrowserTracing', () => { return instance; } + // These are important enough to check with a test as incorrect defaults could + // break a lot of users' configurations. + it('is created with default settings', () => { + const browserTracing = createBrowserTracing(); + + expect(browserTracing.options).toEqual({ + enableLongTask: true, + _experiments: {}, + ...TRACING_DEFAULTS, + markBackgroundTransactions: true, + routingInstrumentation: instrumentRoutingWithDefaults, + startTransactionOnLocationChange: true, + startTransactionOnPageLoad: true, + ...defaultRequestInstrumentationOptions, + }); + }); + + it('is allows to disable enableLongTask via _experiments', () => { + const browserTracing = createBrowserTracing(false, { + _experiments: { + enableLongTask: false, + }, + }); + + expect(browserTracing.options).toEqual({ + enableLongTask: false, + ...TRACING_DEFAULTS, + markBackgroundTransactions: true, + routingInstrumentation: instrumentRoutingWithDefaults, + startTransactionOnLocationChange: true, + startTransactionOnPageLoad: true, + ...defaultRequestInstrumentationOptions, + _experiments: { + enableLongTask: false, + }, + }); + }); + + it('is allows to disable enableLongTask', () => { + const browserTracing = createBrowserTracing(false, { + enableLongTask: false, + }); + + expect(browserTracing.options).toEqual({ + enableLongTask: false, + _experiments: {}, + ...TRACING_DEFAULTS, + markBackgroundTransactions: true, + routingInstrumentation: instrumentRoutingWithDefaults, + startTransactionOnLocationChange: true, + startTransactionOnPageLoad: true, + ...defaultRequestInstrumentationOptions, + }); + }); + /** * All of these tests under `describe('route transaction')` are tested with * `browserTracing.options = { routingInstrumentation: customInstrumentRouting }`, @@ -332,71 +387,6 @@ conditionalTest({ min: 10 })('BrowserTracing', () => { }); }); - describe('beforeStartSpan', () => { - it('is called on transaction creation', () => { - const beforeStartSpan = jest.fn().mockReturnValue({ name: 'here/is/my/path' }); - createBrowserTracing(true, { - beforeStartSpan, - routingInstrumentation: customInstrumentRouting, - }); - const transaction = getActiveTransaction(hub) as IdleTransaction; - expect(transaction).toBeDefined(); - - expect(beforeStartSpan).toHaveBeenCalledTimes(1); - }); - - it('can override default context values', () => { - const beforeStartSpan = jest.fn(ctx => ({ - ...ctx, - op: 'not-pageload', - })); - createBrowserTracing(true, { - beforeStartSpan, - routingInstrumentation: customInstrumentRouting, - }); - const transaction = getActiveTransaction(hub) as IdleTransaction; - expect(transaction).toBeDefined(); - expect(transaction.op).toBe('not-pageload'); - - expect(beforeStartSpan).toHaveBeenCalledTimes(1); - }); - - it("sets transaction name source to `'custom'` if name is changed", () => { - const beforeStartSpan = jest.fn(ctx => ({ - ...ctx, - name: 'newName', - })); - createBrowserTracing(true, { - beforeStartSpan, - routingInstrumentation: customInstrumentRouting, - }); - const transaction = getActiveTransaction(hub) as IdleTransaction; - expect(transaction).toBeDefined(); - expect(transaction.name).toBe('newName'); - expect(transaction.metadata.source).toBe('custom'); - - expect(beforeStartSpan).toHaveBeenCalledTimes(1); - }); - - it('sets transaction name source to default `url` if name is not changed', () => { - const beforeStartSpan = jest.fn(ctx => ({ - ...ctx, - })); - createBrowserTracing(true, { - beforeStartSpan, - routingInstrumentation: (customStartTransaction: (obj: any) => void) => { - customStartTransaction({ name: 'a/path', op: 'pageload', metadata: { source: 'url' } }); - }, - }); - const transaction = getActiveTransaction(hub) as IdleTransaction; - expect(transaction).toBeDefined(); - expect(transaction.name).toBe('a/path'); - expect(transaction.metadata.source).toBe('url'); - - expect(beforeStartSpan).toHaveBeenCalledTimes(1); - }); - }); - it('sets transaction context from sentry-trace header', () => { const name = 'sentry-trace'; const content = '126de09502ae4e0fb26c6967190756a4-b6e54397b12a2a0f-1'; @@ -709,58 +699,4 @@ conditionalTest({ min: 10 })('BrowserTracing', () => { ); }); }); - - describe('options', () => { - // These are important enough to check with a test as incorrect defaults could - // break a lot of users' configurations. - it('is created with default settings', () => { - const browserTracing = createBrowserTracing(); - - expect(browserTracing.options).toEqual({ - enableLongTask: true, - _experiments: {}, - ...TRACING_DEFAULTS, - routingInstrumentation: instrumentRoutingWithDefaults, - spanOnLocationChange: true, - spanOnPageLoad: true, - markBackgroundSpan: true, - ...defaultRequestInstrumentationOptions, - }); - }); - - it('handles legacy `startTransactionOnLocationChange` option', () => { - const integration = new BrowserTracing({ startTransactionOnLocationChange: false }); - expect(integration.options.spanOnLocationChange).toBe(false); - }); - - it('handles legacy `startTransactionOnPageLoad` option', () => { - const integration = new BrowserTracing({ startTransactionOnPageLoad: false }); - expect(integration.options.spanOnPageLoad).toBe(false); - }); - - it('handles legacy `markBackgroundTransactions` option', () => { - const integration = new BrowserTracing({ markBackgroundTransactions: false }); - expect(integration.options.markBackgroundSpan).toBe(false); - }); - - it('allows to disable enableLongTask via _experiments', () => { - const browserTracing = createBrowserTracing(false, { - _experiments: { - enableLongTask: false, - }, - }); - - expect(browserTracing.options.enableLongTask).toBe(false); - expect(browserTracing.options._experiments.enableLongTask).toBe(false); - }); - - it('allows to disable enableLongTask', () => { - const browserTracing = createBrowserTracing(false, { - enableLongTask: false, - }); - - expect(browserTracing.options.enableLongTask).toBe(false); - expect(browserTracing.options._experiments.enableLongTask).toBe(undefined); - }); - }); }); diff --git a/packages/tracing/src/index.ts b/packages/tracing/src/index.ts index b0a5b47554ae..8559188884d7 100644 --- a/packages/tracing/src/index.ts +++ b/packages/tracing/src/index.ts @@ -38,7 +38,6 @@ import { * import { BrowserTracing } from '@sentry/browser'; * new BrowserTracing() */ -// eslint-disable-next-line deprecation/deprecation export const BrowserTracing = BrowserTracingT; // BrowserTracing is already exported as part of `Integrations` below (and for the moment will remain so for @@ -51,7 +50,6 @@ export const BrowserTracing = BrowserTracingT; * import { BrowserTracing } from '@sentry/browser'; * new BrowserTracing() */ -// eslint-disable-next-line deprecation/deprecation export type BrowserTracing = BrowserTracingT; /** diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index ca4c976db1c8..7f9d66c904fa 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -100,7 +100,6 @@ export type { SpanContextData, TraceFlag, } from './span'; -export type { StartSpanOptions } from './startSpanOptions'; export type { StackFrame } from './stackframe'; export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace'; export type { TextEncoderInternal } from './textencoder'; diff --git a/packages/types/src/startSpanOptions.ts b/packages/types/src/startSpanOptions.ts deleted file mode 100644 index 49f81250008c..000000000000 --- a/packages/types/src/startSpanOptions.ts +++ /dev/null @@ -1,104 +0,0 @@ -import type { Instrumenter } from './instrumenter'; -import type { Primitive } from './misc'; -import type { Scope } from './scope'; -import type { SpanAttributes, SpanOrigin, SpanTimeInput } from './span'; -import type { TransactionContext, TransactionMetadata, TransactionSource } from './transaction'; - -export interface StartSpanOptions extends TransactionContext { - /** A manually specified start time for the created `Span` object. */ - startTime?: SpanTimeInput; - - /** If defined, start this span off this scope instead off the current scope. */ - scope?: Scope; - - /** The name of the span. */ - name: string; - - /** An op for the span. This is a categorization for spans. */ - op?: string; - - /** The origin of the span - if it comes from auto instrumenation or manual instrumentation. */ - origin?: SpanOrigin; - - /** Attributes for the span. */ - attributes?: SpanAttributes; - - // All remaining fields are deprecated - - /** - * @deprecated Manually set the end timestamp instead. - */ - trimEnd?: boolean; - - /** - * @deprecated This cannot be set manually anymore. - */ - parentSampled?: boolean; - - /** - * @deprecated Use attributes or set data on scopes instead. - */ - metadata?: Partial; - - /** - * The name thingy. - * @deprecated Use `name` instead. - */ - description?: string; - - /** - * @deprecated Use `span.setStatus()` instead. - */ - status?: string; - - /** - * @deprecated Use `scope` instead. - */ - parentSpanId?: string; - - /** - * @deprecated You cannot manually set the span to sampled anymore. - */ - sampled?: boolean; - - /** - * @deprecated You cannot manually set the spanId anymore. - */ - spanId?: string; - - /** - * @deprecated You cannot manually set the traceId anymore. - */ - traceId?: string; - - /** - * @deprecated Use an attribute instead. - */ - source?: TransactionSource; - - /** - * @deprecated Use attributes or set tags on the scope instead. - */ - tags?: { [key: string]: Primitive }; - - /** - * @deprecated Use attributes instead. - */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - data?: { [key: string]: any }; - - /** - * @deprecated Use `startTime` instead. - */ - startTimestamp?: number; - - /** - * @deprecated Use `span.end()` instead. - */ - endTimestamp?: number; - - /** - * @deprecated You cannot set the instrumenter manually anymore. - */ - instrumenter?: Instrumenter; -} From 52495c00ba700dd1df58025ef49195048279b692 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 24 Jan 2024 15:39:11 -0500 Subject: [PATCH 13/15] ref(tracing): Remove startChild calls from fetch/xhr instrumentation (#10236) --- .../tracing-internal/src/browser/request.ts | 31 +- packages/tracing-internal/src/common/fetch.ts | 29 +- .../test/browser/request.test.ts | 377 +----------------- 3 files changed, 28 insertions(+), 409 deletions(-) diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 461171ec1e25..c84d0545054b 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -1,6 +1,5 @@ /* eslint-disable max-lines */ import { - getActiveSpan, getClient, getCurrentScope, getDynamicSamplingContextFromClient, @@ -9,6 +8,7 @@ import { hasTracingEnabled, spanToJSON, spanToTraceHeader, + startInactiveSpan, } from '@sentry/core'; import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, Span } from '@sentry/types'; import { @@ -275,22 +275,19 @@ export function xhrCallback( } const scope = getCurrentScope(); - const parentSpan = getActiveSpan(); - - const span = - shouldCreateSpanResult && parentSpan - ? // eslint-disable-next-line deprecation/deprecation - parentSpan.startChild({ - data: { - type: 'xhr', - 'http.method': sentryXhrData.method, - url: sentryXhrData.url, - }, - description: `${sentryXhrData.method} ${sentryXhrData.url}`, - op: 'http.client', - origin: 'auto.http.browser', - }) - : undefined; + + const span = shouldCreateSpanResult + ? startInactiveSpan({ + attributes: { + type: 'xhr', + 'http.method': sentryXhrData.method, + url: sentryXhrData.url, + }, + name: `${sentryXhrData.method} ${sentryXhrData.url}`, + op: 'http.client', + origin: 'auto.http.browser', + }) + : undefined; if (span) { xhr.__sentry_xhr_span_id__ = span.spanContext().spanId; diff --git a/packages/tracing-internal/src/common/fetch.ts b/packages/tracing-internal/src/common/fetch.ts index e0a8e2ed9fa3..c96778f8cd35 100644 --- a/packages/tracing-internal/src/common/fetch.ts +++ b/packages/tracing-internal/src/common/fetch.ts @@ -1,5 +1,4 @@ import { - getActiveSpan, getClient, getCurrentScope, getDynamicSamplingContextFromClient, @@ -7,6 +6,7 @@ import { getRootSpan, hasTracingEnabled, spanToTraceHeader, + startInactiveSpan, } from '@sentry/core'; import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types'; import { @@ -76,24 +76,21 @@ export function instrumentFetchRequest( const scope = getCurrentScope(); const client = getClient(); - const parentSpan = getActiveSpan(); const { method, url } = handlerData.fetchData; - const span = - shouldCreateSpanResult && parentSpan - ? // eslint-disable-next-line deprecation/deprecation - parentSpan.startChild({ - data: { - url, - type: 'fetch', - 'http.method': method, - }, - description: `${method} ${url}`, - op: 'http.client', - origin: spanOrigin, - }) - : undefined; + const span = shouldCreateSpanResult + ? startInactiveSpan({ + attributes: { + url, + type: 'fetch', + 'http.method': method, + }, + name: `${method} ${url}`, + op: 'http.client', + origin: spanOrigin, + }) + : undefined; if (span) { handlerData.fetchData.__span = span.spanContext().spanId; diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index 782c0890e156..426325072984 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -1,32 +1,14 @@ /* eslint-disable deprecation/deprecation */ -import * as sentryCore from '@sentry/core'; -import { Hub, makeMain, spanToJSON } from '@sentry/core'; -import type { HandlerDataFetch, HandlerDataXhr, SentryWrappedXMLHttpRequest } from '@sentry/types'; import * as utils from '@sentry/utils'; -import { SENTRY_XHR_DATA_KEY } from '@sentry/utils'; -import type { Transaction } from '../../../tracing/src'; -import { Span, addExtensionMethods, spanStatusfromHttpCode } from '../../../tracing/src'; -import { getDefaultBrowserClientOptions } from '../../../tracing/test/testutils'; -import { - extractNetworkProtocol, - instrumentOutgoingRequests, - shouldAttachHeaders, - xhrCallback, -} from '../../src/browser/request'; -import { instrumentFetchRequest } from '../../src/common/fetch'; -import { TestClient } from '../utils/TestClient'; +import { extractNetworkProtocol, instrumentOutgoingRequests, shouldAttachHeaders } from '../../src/browser/request'; beforeAll(() => { - addExtensionMethods(); // @ts-expect-error need to override global Request because it's not in the jest environment (even with an // `@jest-environment jsdom` directive, for some reason) global.Request = {}; }); -const hasTracingEnabled = jest.spyOn(sentryCore, 'hasTracingEnabled'); -const setRequestHeader = jest.fn(); - describe('instrumentOutgoingRequests', () => { beforeEach(() => { jest.clearAllMocks(); @@ -59,363 +41,6 @@ describe('instrumentOutgoingRequests', () => { }); }); -describe('callbacks', () => { - let hub: Hub; - let transaction: Transaction; - const alwaysCreateSpan = () => true; - const alwaysAttachHeaders = () => true; - const startTimestamp = 1356996072000; - const endTimestamp = 1356996072000; - - beforeAll(() => { - const options = getDefaultBrowserClientOptions({ tracesSampleRate: 1 }); - hub = new Hub(new TestClient(options)); - makeMain(hub); - }); - - beforeEach(() => { - transaction = hub.startTransaction({ name: 'organizations/users/:userid', op: 'pageload' }) as Transaction; - hub.getScope().setSpan(transaction); - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - describe('fetchCallback()', () => { - let fetchHandlerData: HandlerDataFetch; - - const fetchSpan = { - data: { - 'http.method': 'GET', - url: 'http://dogs.are.great/', - type: 'fetch', - }, - description: 'GET http://dogs.are.great/', - op: 'http.client', - parentSpanId: expect.any(String), - spanId: expect.any(String), - startTimestamp: expect.any(Number), - traceId: expect.any(String), - }; - - beforeEach(() => { - fetchHandlerData = { - args: ['http://dogs.are.great/', {}], - fetchData: { url: 'http://dogs.are.great/', method: 'GET' }, - startTimestamp, - }; - }); - - it.each([ - // each case is [shouldCreateSpanReturnValue, shouldAttachHeadersReturnValue, expectedSpan, expectedHeaderKeys] - [true, true, expect.objectContaining(fetchSpan), ['sentry-trace', 'baggage']], - [true, false, expect.objectContaining(fetchSpan), []], - [false, true, undefined, ['sentry-trace', 'baggage']], - [false, false, undefined, []], - ])( - 'span creation/header attachment interaction - shouldCreateSpan: %s, shouldAttachHeaders: %s', - (shouldCreateSpanReturnValue, shouldAttachHeadersReturnValue, expectedSpan, expectedHeaderKeys) => { - instrumentFetchRequest( - fetchHandlerData, - () => shouldCreateSpanReturnValue, - () => shouldAttachHeadersReturnValue, - {}, - ); - - // spans[0] is the transaction itself - const newSpan = transaction.spanRecorder?.spans[1] as Span; - expect(newSpan).toEqual(expectedSpan); - - const headers = (fetchHandlerData.args[1].headers as Record) || {}; - expect(Object.keys(headers)).toEqual(expectedHeaderKeys); - }, - ); - - it('adds neither fetch request spans nor fetch request headers if tracing is disabled', () => { - hasTracingEnabled.mockReturnValueOnce(false); - const spans = {}; - - instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - expect(spans).toEqual({}); - - const headers = (fetchHandlerData.args[1].headers as Record) || {}; - expect(Object.keys(headers)).toEqual([]); - }); - - it('creates and finishes fetch span on active transaction', () => { - const spans = {}; - - // triggered by request being sent - instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - const newSpan = transaction.spanRecorder?.spans[1] as Span; - - expect(newSpan).toBeDefined(); - expect(newSpan).toBeInstanceOf(Span); - expect(newSpan.data).toEqual({ - 'http.method': 'GET', - type: 'fetch', - url: 'http://dogs.are.great/', - }); - expect(newSpan.description).toBe('GET http://dogs.are.great/'); - expect(newSpan.op).toBe('http.client'); - const spanId = fetchHandlerData.fetchData?.__span; - expect(spanId).toBeDefined(); - - const postRequestFetchHandlerData = { - ...fetchHandlerData, - endTimestamp, - }; - - // triggered by response coming back - instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - expect(spanToJSON(newSpan).timestamp).toBeDefined(); - }); - - it('sets response status on finish', () => { - const spans: Record = {}; - - // triggered by request being sent - instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - const newSpan = transaction.spanRecorder?.spans[1] as Span; - - expect(newSpan).toBeDefined(); - - const postRequestFetchHandlerData = { - ...fetchHandlerData, - endTimestamp, - response: { status: 404 } as Response, - }; - - // triggered by response coming back - instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - expect(newSpan.status).toBe(spanStatusfromHttpCode(404)); - }); - - it('ignores response with no associated span', () => { - // the request might be missed somehow. E.g. if it was sent before tracing gets enabled. - - const postRequestFetchHandlerData = { - ...fetchHandlerData, - endTimestamp, - response: { status: 404 } as Response, - }; - - // in that case, the response coming back will be ignored - instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, {}); - - const newSpan = transaction.spanRecorder?.spans[1]; - - expect(newSpan).toBeUndefined(); - }); - - it('uses active span to generate sentry-trace header', () => { - const spans: Record = {}; - // triggered by request being sent - instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - const activeSpan = transaction.spanRecorder?.spans[1] as Span; - - const postRequestFetchHandlerData = { - ...fetchHandlerData, - endTimestamp, - response: { status: 200 } as Response, - }; - - // triggered by response coming back - instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - const headers = (fetchHandlerData.args[1].headers as Record) || {}; - expect(headers['sentry-trace']).toEqual(`${activeSpan.traceId}-${activeSpan.spanId}-1`); - }); - - it('adds content-length to span data on finish', () => { - const spans: Record = {}; - - // triggered by request being sent - instrumentFetchRequest(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - const newSpan = transaction.spanRecorder?.spans[1] as Span; - - expect(newSpan).toBeDefined(); - - const postRequestFetchHandlerData = { - ...fetchHandlerData, - endTimestamp, - response: { status: 404, headers: { get: () => 123 } }, - } as unknown as HandlerDataFetch; - - // triggered by response coming back - instrumentFetchRequest(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - const finishedSpan = transaction.spanRecorder?.spans[1] as Span; - - expect(finishedSpan).toBeDefined(); - expect(finishedSpan).toBeInstanceOf(Span); - expect(spanToJSON(finishedSpan).data).toEqual({ - 'http.response_content_length': 123, - 'http.method': 'GET', - 'http.response.status_code': 404, - type: 'fetch', - url: 'http://dogs.are.great/', - 'sentry.op': 'http.client', - 'sentry.origin': 'auto.http.browser', - }); - expect(finishedSpan.op).toBe('http.client'); - }); - }); - - describe('xhrCallback()', () => { - let xhrHandlerData: HandlerDataXhr; - - const xhrSpan = { - data: { - 'http.method': 'GET', - url: 'http://dogs.are.great/', - type: 'xhr', - }, - description: 'GET http://dogs.are.great/', - op: 'http.client', - parentSpanId: expect.any(String), - spanId: expect.any(String), - startTimestamp: expect.any(Number), - traceId: expect.any(String), - }; - - beforeEach(() => { - xhrHandlerData = { - args: ['GET', 'http://dogs.are.great/'], - xhr: { - [SENTRY_XHR_DATA_KEY]: { - method: 'GET', - url: 'http://dogs.are.great/', - status_code: 200, - request_headers: {}, - }, - __sentry_xhr_span_id__: '1231201211212012', - setRequestHeader, - } as SentryWrappedXMLHttpRequest, - startTimestamp, - }; - }); - - it.each([ - // each case is [shouldCreateSpanReturnValue, shouldAttachHeadersReturnValue, expectedSpan, expectedHeaderKeys] - [true, true, expect.objectContaining(xhrSpan), ['sentry-trace', 'baggage']], - [true, false, expect.objectContaining(xhrSpan), []], - [false, true, undefined, ['sentry-trace', 'baggage']], - [false, false, undefined, []], - ])( - 'span creation/header attachment interaction - shouldCreateSpan: %s, shouldAttachHeaders: %s', - (shouldCreateSpanReturnValue, shouldAttachHeadersReturnValue, expectedSpan, expectedHeaderKeys) => { - xhrCallback( - xhrHandlerData, - () => shouldCreateSpanReturnValue, - () => shouldAttachHeadersReturnValue, - {}, - ); - - // spans[0] is the transaction itself - const newSpan = transaction.spanRecorder?.spans[1] as Span; - expect(newSpan).toEqual(expectedSpan); - - const headerKeys = setRequestHeader.mock.calls.map(header => header[0]); - expect(headerKeys).toEqual(expectedHeaderKeys); - }, - ); - - it('adds neither xhr request spans nor xhr request headers if tracing is disabled', () => { - hasTracingEnabled.mockReturnValueOnce(false); - const spans = {}; - - xhrCallback(xhrHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - expect(spans).toEqual({}); - expect(setRequestHeader).not.toHaveBeenCalled(); - }); - - it('creates and finishes XHR span on active transaction', () => { - const spans = {}; - - // triggered by request being sent - xhrCallback(xhrHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - const newSpan = transaction.spanRecorder?.spans[1] as Span; - - expect(newSpan).toBeInstanceOf(Span); - expect(newSpan.data).toEqual({ - 'http.method': 'GET', - type: 'xhr', - url: 'http://dogs.are.great/', - }); - expect(newSpan.description).toBe('GET http://dogs.are.great/'); - expect(newSpan.op).toBe('http.client'); - const spanId = xhrHandlerData.xhr?.__sentry_xhr_span_id__; - expect(spanId).toBeDefined(); - expect(spanId).toEqual(newSpan?.spanId); - - const postRequestXHRHandlerData = { - ...xhrHandlerData, - endTimestamp, - }; - - // triggered by response coming back - xhrCallback(postRequestXHRHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - expect(spanToJSON(newSpan).timestamp).toBeDefined(); - }); - - it('sets response status on finish', () => { - const spans = {}; - - // triggered by request being sent - xhrCallback(xhrHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - const newSpan = transaction.spanRecorder?.spans[1] as Span; - - expect(newSpan).toBeDefined(); - - const postRequestXHRHandlerData = { - ...xhrHandlerData, - endTimestamp, - }; - postRequestXHRHandlerData.xhr![SENTRY_XHR_DATA_KEY]!.status_code = 404; - - // triggered by response coming back - xhrCallback(postRequestXHRHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); - - expect(newSpan.status).toBe(spanStatusfromHttpCode(404)); - }); - - it('ignores response with no associated span', () => { - // the request might be missed somehow. E.g. if it was sent before tracing gets enabled. - - const postRequestXHRHandlerData: HandlerDataXhr = { - ...{ - xhr: { - [SENTRY_XHR_DATA_KEY]: xhrHandlerData.xhr?.[SENTRY_XHR_DATA_KEY], - }, - }, - args: ['GET', 'http://dogs.are.great/'], - startTimestamp, - endTimestamp, - }; - - // in that case, the response coming back will be ignored - xhrCallback(postRequestXHRHandlerData, alwaysCreateSpan, alwaysAttachHeaders, {}); - - const newSpan = transaction.spanRecorder?.spans[1]; - - expect(newSpan).toBeUndefined(); - }); - }); -}); - interface ProtocolInfo { name: string; version: string; From b72a681e3e14c61e39d6cad8ca62b741c8235e6f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 24 Jan 2024 16:48:44 -0500 Subject: [PATCH 14/15] fix(tracing): Ensure web vitals are correctly stopped/captured (#10323) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was identified as a problem by @edwardgou-sentry , and was incorrectly changed in 7.75.0. I think this should work now as expected 🤔 I added some tests for LCP to hopefully very this works kind of as expected (=even after stopping it you can still get a LCP value). --- .../assets/sentry-logo-600x179.png | Bin 0 -> 16118 bytes .../tracing/metrics/handlers-lcp/subject.js | 18 +++++++ .../metrics/handlers-lcp/template.html | 11 ++++ .../tracing/metrics/handlers-lcp/test.ts | 51 ++++++++++++++++++ .../tests/fixtures/ReplayRecordingData.ts | 6 ++- .../tests/fixtures/ReplayRecordingData.ts | 6 ++- .../tests/fixtures/ReplayRecordingData.ts | 6 ++- .../src/browser/instrument.ts | 51 +++++++++++++----- .../src/browser/metrics/index.ts | 13 ++--- 9 files changed, 140 insertions(+), 22 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/assets/sentry-logo-600x179.png create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/assets/sentry-logo-600x179.png b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/assets/sentry-logo-600x179.png new file mode 100644 index 0000000000000000000000000000000000000000..353b7233d6bfa4f026f9998cacfa4add4bba9274 GIT binary patch literal 16118 zcmeHu^+S})7xyl#gwjYzhlrF&cOzZW-Q5k+0s<-_QZ6msEZvPD-L-UgN%#AB@AcmA z-|)V#KOoD_GtbPKGc)IWKA-bZNkJ0*G0|fX2!t*zC9VttA!q`>SD+vPKh?a9D}n#O zF3OUkpprq7Z4ih8BrPtY>Z!k<;+?FcR@FVl+at$>|46kYBS2WzfXAwjotEiQCvxwN z*ZVTy(;HRsciU<{RKJwo3YUIbdh#fEBMa>kME){Q_YpkV)ix;xAF}p61LdWZjgI-( ziUSKpHf}qMuT?`l(C1@rf@${?i~8P&hde#E&gTm@ls{=8!2wuU2%^F$C}2t~I&e69 zA_VuJj}*VZ1F>lS!UDeg>jTh`!Wh9AMukMb{6Ei8Ajziw*FqRYTuvBGy;GPb`QM%? zAf1LiY=8OyUiLl=HLUFuVP5e6zKoFZ9n$}Fi^T$F(ZoiKbNqXBe?J{WGxXnpDIioY zD3BP8({U7jkN=5Eg~Km*M`? z2k>23+gFRf%<@mS*tl3hcq~e&w2=SC2p9+LzZd^sV;uD-sqa-;Pt2e5Kxz=48N4hp z(6`Nsdmupg(;EI6u(eYy`IF7l=E>5o7DbeJa}PlZ-L3fKAu!0Q8n5nOw;84aoGlSe z(9AqaI70`#97kDSZ&G8OeQ|*9n=>=o%tcy%=UB3PaBaM54l$KLL4 zAtzmPFy6nj7R3R)2WKgAz}wr~BuWVD0%U03`P~35RM>)`@pfY16SLEYKaTz05isY6 z)fyEW2BS1Y6oUxvy~yR%eI))ZGUf~0n49RMf9EX>0xX0eOk60f74dRD@yd68J zMy{IQCq*xb@e;2@FC~i3@O++)+WK#D$JD^C5uLOeyT=DNhLSpW<_j~_?SayJsrRZu zScuehqdFz=)ay9^jvgNNdwUNEeq-f^&l7kr66aujn7~_*?J;rr%nnWyu_^}yZ z#iu0I-F^CR8(Rwg-u$ntzs{(<-$Jr~38^hy;*pHVQKVrFrCMp4(*zw&SzqMj-`&+X zOj=Sa|GRP1KmImeaD(Obb_1rn!xbW^@zXPFn9P0u{jbLp=Hm=@a4wuyWpjSvblE5R z_o5~GuK!un1vr%+_i)Aa>%SK#-v|*2X2zFM(7Vfc2lYi_=n?}}60;pVQEPE_e7J5F zVm+h!@AOKP0?s^?+A8BgOv~WQS^J6J@dmC_JL~ZmW^i(`fIgz_kcTuBd#3mlm0GWn z|Bj-8-%A0Rt+ZN{200jo3Ud!I@f0;_3kX=a{y^fSKk0gfAAv-ouemwbK^BGbpNstV zPdu#MK+_HJv?EEPfhQ%KjKjlXUhd}+ro*M|@FEWZ+V9^9^8a0iVaQ*elEQoTP1;+2 z;Y0)kkD5ttz1XbV)hIIAF^JX5NkIf9%|?a|GQ^(@yqf4Bjr`YT_Pzo}eN?+o$L0b? z(C@YXbV^zi>(8YlB&x~KlF8b5IB45aM(pUHuktgChV{={$Ws6}1Fki1-I|;DeHweB z-6xaV^cxv2!R!%pJ}Y{w7IM^*jcVF-2fU_R@4ElB$_TJqvU|o@?cz9^5@yk`1vOvt z7;Nlx4}Ujr5{`$KzM(zm?>?VG^mnuhXkojn&qOL z*I``8T-|v?5Gkyvc-XfZ&Tw<=%Dy%LoPI=kJavzc|TLsFZn5#L0k66DeHIc3h%zMRyc^H^AQgafUfn1 zpazOu?^EB8x5rU%E&H*gScpK@VP z>Z&vHq%G~}d>_-5z8(WSlu_ZnCvTGq>9h1WYlQ{ku$o-m*ZCDM3~#zyB;@y__l!$`&eP&Iq{mxV8rBuI3w z9(MRh!1f$r&Ey;1^=5Elkl_oDS5mKYmE2`??>u*urEeh5Ya?c9!DMQu&K^!@Z+AKk zSu6*Wrfv#KfEM-BL6qcQu}1uUIO|)m@iVx>`}Jjn31FSGx|0_3mLqRDs)J$g4Zb!nFTm*#)#S1bNqj7H;?>@B|&q)U|%$1*Dhd9qYH-=5;(SBz`)EFd_=QFS*MPNQuDb1+LNlo9l5GwP$?n%7m$$yo-8G=&+QfV%aA-J3^7cH!YF8eV-(>8m(F zKb}jve$^R#2*|gumjkSG+{v!VtU&%Sb>?i_k#!!f&h=x$%GLaeV@_^u$g)cY?FJ(f zYcQ?JG}`ELPuVfpQLb!l4m+zf4<6QLEj-G~#sL1Jw-SP)_$uCIF=_`X0z&C{TODT2m1J|4Kab*JIP zcN?0DTm>rHeA_IB->wQy)i zy1J5Ix>52V-nj28SCz#|mJmr}iS@9!-Rsn)@DWzKs?g6VVToa^Kb-v@(YjZ#C(=@; z6}W3)92_3rSX6X%L@*%Ppea`kxbCb?`ooN_yY||z*LNzrYIW7yo55HL>+bk(M^)@S zNQ=h?QVlH16%pEFgpUwxXSr$|R9dk}593dSMm{n*z+5fStr8kiD^wfQI?>nA@rk#B z{ew6HJ41>IYq+iSto>&XkOQX5q;UAzu+rG=_Tjr}Cfn5bXW!1l2?L0jcd3GLKhtcw z$Jh2yv1pors&UTO100(%{2`YZeIt9^!L|7NizDeStN{-AhAb|w8`hkNP0VCzTWyz}lRDv0eQaWd)Z z=eAK=1r!TB*+GtLrc4{kcd%(jJ|;|jZ{tkVj}!)se4MmDeO!)NkO+m=EkoHMQ}t${ z{==V1Ys-`@k}weFRF9I)A~GYRMA0T+CU4RvC~)N!OnM{gRu=4N5L}cV-k1b*!R1GL z_TGtiGXEG`F)4)O+KL2$WP^qf+e7ERy4VdbYv(k5!BP;;&<7>=Mg>KwH*mcOWYjA| z#~;PEr(0A&eRkz3@zM<^Vsf`tSQ}Ol%M2kBheTxf1XWQ)^RNa|?mumpTonghQ_xn+ z{Oljuw!S62`=M7OnmzUGY*cz|$x9*2pFwRWP19{upCw(Ot#lB99fLOg!<>MG$p9+$ zz9?P7CanDH%pn+iHXLh-_>;c9?n#Ii>J5qTCB20p=*X38>D0S%u0qat%q&`vU7mcO z+szO)^i@D3oDvJ$ESU9^GyqFWS=$A0ay6SCytZ zYK#1p6=4Q^u(MD14>aa9d*}5So zcy`QrkGSI#NxrOS-JKKhR)qrD-OgP(75u#)3F0IGFM3WOFXVgiURsA)JOfh_Zj8{nqAWW^kw~72g99i`4>oq7(1tPd+d4b8Yo-ezId2EK}ZH{vMbv4Y}Sw_ffS@_L_UTbKwCJU zgz>oCBUj7C(hZ1Q{E(?&k<`>z4h*px>_x}~>&ePFcPx-vxgVW`WF(x-F@!}&9?0M* z22Y-Q^W`Hw1U~s7!lL?0zBa>rR1$OZ3lh*Z2dIBpP*5CJtLbjPu{DNzBZMNdujc(s zG74hjjAzCg#mH~+q0m{id~)o>6+p66IbURgrmZ-49}aN9GaG`%OBh@ z_<~r23}l~2x3s}IKW~N;4(L>IK1_Sc)Ayf!R{6&vG$|c7G?t^LFVGss6I?mYiX&$o}5#;~rg{PVeS1;_mC+XUyf8zj_hJ1CFSb(J-Ov;VOkh(1egEn*Ttfl@H!+L4P7ddg>nAa zQ#@f(lTH07^v)LpHs?Tr55sYL)Hf`FybR|Je3v3xC3zj|X)03q;8ve2P(%Jat7?IxEkVJC-?@ zSp$OgP0>}#B~|srfEqJ4pCis~R$H;U?Xbk0CpLDg4+glm(X@l7Twb78j68rpv^X{; z!x`mBn(K+qi5Y{d?0cZO#6pjox|GM=UP3DYHl$6^~EQIR??D9 zhzv`t%rv%fF3HSf}9wTO2Z$C~xyxL44aHo9|)b^XUWNbDM{ zr(1$O+T;)T?nSEPbS4b>l6{$zYQq^FVACGoOJF98hR2KNPj!d1uwvuQ%ymI>PH^g0 z#79HWo{`A$)93i)PsM_61g>>8lFb(zUOW)Y`)Ow{nxV(X{q(_K?K(4x;fQo!bxqim z=t3!HDO>0;fzEV;ErkvcC7O-!rkFPm7gvxDKzi^>X6k?_me6aRDVIn?>sjR&NH%-3 z<(X{-G$y*aDx#l3QS@Fs2hlF)Y(RzxZ19{*J&^pS`lZ~ZfBP!L^ zcoMAqu&49DR&11yW8F8P29+*xw_Dlw)QQmg6;?H5wGq{o27;RRJTzXdJnbH@)7m!W z6oV|Her5X{u%LDNYsXGzOO}@JU2(-QjixAO{TndggWSTmWfB+f&$%r-9ST&@@D~l~ zvi$}WFOjhaBFeF9i9!jBECumNZ;<*b)!v&cKwovACEF5e)3M#qLF$8vJ1G1o3w}M0 zr<0{+&BdiU9A|190y43IksqY?V{Q!vZOYy;5*u(&{P%)*ZszduJsLW8pR*_G49iLN zyMmY9CFkmS0wUj;`zj~k+%1(N$oI-9=+b6{deeM$^>ocz%=q`l6q8N0(4@q(t_}?# zTx8RfDQ?)q07X9-Vh8>3@7kr~n>MstTPvVyE&Qy1um`JD&e0PE#o&g1Ibe$ag|0|p z9ltq4$VShWsbJj;Rn#1KWdS3y&GQ}>=Ji*j^ViXYb?g&7XR}IEA}ur{KP##772N3d z%9!3{^H(rWU0yw8-#D!)dEYa3aChf1f&4C^zQ}aLonm}m{ECZ;J3;5BP%q$9+bEZ2 z!5ey`#XuBVmqQa(|E-W4+@K#c1_o0fkqYC{PNNtRP{^HR2k{u~NZ(qkyT9s^qCNWw zE!p&`D5>MHQ==_QVB>u5rSU!a;g#g9eWG5k;aR`#b)x2+f9*(ligv>}cyRwB89AfY zh*ECyu0f4KeVT+?tfleF?K~t$E=&Vow(cbJn?LQ3;o=?+XZtLL=h!|?D&g^jMTST= zBd22?<|lQNwu*%_QVShyBOE{DJ+yS6i-zlRD0t4PA!aZlKL z=adShn4|q^Hzy7-R&U<#^3XbU^T*pC%#EFIl{BX-*fa)`_8sLm?5gL{?>3B^L95|w zLPDKc4@i)yYnJ+|8EIOv@aQx}Nd@eKU>=U{)Cj@)2*08T%i0fUQg~B5^eswxvS{ir zeV929l!AWT+r#ZvvwA#)+yxl$o`mMb!N?)ORwVnHZ@$?GDU6d9PqD7pQe|kpJv1^s zC5Dh#e9sL`HgRwxHfeHcQ;D2Vw7l$eOyLLPJWnJ(>Q;6Dqj)$Xr8CoYIh>~A_L1{N!1!9i;fkgDe>-x^;<;*=iO;@ zAV8r5>tP*3IKizBVM`h;9S>&(qmNU3ov<-Kq+ut94Soj4QjrU{1{C(bQ+NJJ+L0S0 zt1r1$=q!cK9NDHU!pP==2imcHe*b-!=V8JTMd5Y-^1#cUDlXhe`^tUn|lB2lTh%s0j;-Fahi~7gGbuT;zM!U>pOSjS&%=! zTkaJ*Xzq=0OMvxR`}YsH3x;O^dZS}&_zCWPniyu^g;_WsH`cx} z@OJ=tFwC!4YsAo1ti0Xj{{i|r4(5OujC;v)WKxoFgJ2-!6q}gD*P-BpNOfEE$@65# zjpuINc3@#fHaO8Ki3#(%TcN}7y#l$IldnHIMdMe23#=PBkfbL$>lx)Xww252tbgK7 z;j=-wRvqc{4=J3Q$nM|M!IIji-Ovw3SeXsj8cRkBzPvZ4yKsxFp639D3v4|A)a=Lz}{fpcGF4EkNa+_NCC2izjp* z3f7@Z8b-4D8*MDe${47v##ur&k~PZ0IZwb|=1&3Rw03_^J$aYH$!Ed1uDA$Dbz*PFi7SBB7+*eIR{8)%-FVuouoTf-Gv7w>AatacNDSIR9HzjMlA`daug+Y4-R05F z(L;+zn&%yL_X>(jZ8F6-Iztgw)!}kYP4geZ`W>y8mYq?PbgPaEf>Ehtd(k1A^=6v| zj+xe_lem+H+qQ;i-m)mm-;8_S-sB_bRdLzbfS1;Z!0Pp>E_oq-wD|`tE(o`JBYjrS z+Fq~PY*wGde8$}(CfGJTjkFeoI7*MWXx=>AIio{j@Cs79F)JnL1dLFtbfxi+c36<7H?#JN>lhpa4o30RZR@*Zmzw>K*$BPE0r@Ir-i;xa zNh62Su@^0L5}JDQ`nsdb3BI%~7mv8-m8iGWH&2tni<{w%1TBo7zQ^TX?LMkar^Ns= zpKe2R-+0^^5AbN!oCOF>mgnlQvuvwio@)itT=g2HoMREsufTz_>6Nd7Kp?WL-&z20 zv>T4xg|LX~svLBN8Y}OeLrw;ffjav4tHO3QG_Q0~1D;Ipe{4R8KRLA6mBike zq=SI=eW{JQrasz*js`Uzm2-hrqaLwGBBdiHvPW5<{~{=+FI8Hnt$c+Zlzo2vp~N(u zqn39iJvTftF2|_o`wm$tazK*akX3Ds;)3Culjm;N#EgGu$&n!vwJ+6KT^b}(OQKUg zEbQReuMOANK?a zQbfJa81x|tduikGFai85wD_Q$JGjUaYd~dq?j`PD5f(;DmMz3nDKnY4o_1eiu0+Tc zT`rZtwN$yc-w*x$;L3s@0&nKlddt8>R8B?z-YTe)oC^t|xoK&A+_^xP@^7>Xui^d) z8ZR-5zHZ4Rx^#fUORu^`$62RELvVZ2{MQo(`#JZChzsH(BMczAU<7ZEpZFNR)`!-Z z%cs$2SB_;zOE)Z@C0`ZSlBd9s3)SUUYl@dT4q}$v5wyt`o4j-wILj)!wQ#j_iq9)a zM`}q0cf9HliFbK*Sd7r(4|K>=e8mBfn1yP?&B5(k(XN=kYea1U-_N`C8%4rB-k-ip z-u_dho3I7eiYAb2g~Uy_?vT~w0#$^O9Enx2@m(l*Y5%$+L#-r1iCe2|BeM~yG`4E~QwCG4>_k8#z(tJ50;p#`56+*@_b@Sle zu(V=neZg50kMIyHaACt6#i>8#3mxc>^s z)^UnTZBA&~BN?B?D44g1if?k(i#5n7{6IB=DWl$qg*lpKCh^`r1Ussit?715VD4I5 zk*IG~KdC^S2Z&Ko%C`M6uGgfe)6F~7!tf9#mP}T=_4;PoT9c=wX~MkFx;PZYm6|1j z4g4Su&WXH5!~9fho}h8RqA91tO-p3+PD7{dHx{sQ!A`=fxYq(VNI3yZ&i3I7x5xAV z>(wxOjI=wvR^G&*`qrT|=Qa7zkKwE3CM_jjqeK{z%viux%hk8P`Rp62G0Ct-OZXF?!pwDy}aas5|@~Mr; zAc*4>(OhAY)ra2kKvDjLA3R80r=bd5hE2T6DO{RwpB=ceCQZ>*y!k76W|m-E-H^l% z;%GyQkAtObS=y20pm;Bbf7^OS62XQ2a4^Gar`8zLMB6R7sH>;_ z7K69Dqkk_IQMh2~g~tN{)<}jiPibUUA0Eql_~;}e=txW`X8o z)J+5|w@uIasjkZxf(UJe;^TDPK}sJCjFOkMYxWu?9c*U{i-UbJD16kYyWCNAIN%>+ zMOy#i?XrX?dTqFATx*f~D7;n+obT-w7Wa#+&d7?QZC!0@DO=?q4QX+j4&$7^4IVy{ zG-kM{b8V+4XbXDCv6h9gq|0G&K8m0Z|T+~ZzhaB zTkWat@5clm32~6!Mg$%AikOhLln`{Y?lXg0CBOMLVVyAC6zMKrVCMMIYB@MK>>VQ; zV}h1cBM2?(Q%~g4o1~zSBA%I=_lk{yE8=uD$p(7Nr{nqEa&VhpWI8rKnNK6MSYaj3o=kO`j(4ml0~#eB!f7#X9e) z9z1SZFgst!qpJ|J^n21e&wjTk_I|Sav-unm-K)q8350uNW?#EVxdwZ5j2mKmcG`5({q*;|`{Y=SYQBNU~?CGNy{6&+n5^gS&e|qbNi> z#`?>}H6LO|t%Ep(>RjtZ)XZRYG`(;^HvpDLHHKNAod_a<9>%sX6@nffaB)ZsUh);( z8>BOsSwAD6kX*WxiHG_I>u`Vj8v_uIDd3QS&LCn-rHq;ymKLy-VRSSms*qS+aZ&D6f5H6n}?2T>Re`>RK00LcD%*+KPHlG<5 z*r#!ehlpl?NPuSKzU8kmh2)kwDjeD{`0a^8&|(Z-RM5i30LEEuFke^$;P54dbIXWv zo0xwJz8*+?A8pzvPqRNn+16d>GA51J8o{%`YsTp756jRIuG3mA!oyf-|2V{y-pir+ zmK1F{ya}4<4qt0tr**N?Y|d_wpn7P&BVn*6Ssbnh&At0<=kzx~46)HreUrsA*D z9P|~EYbmx)O08crW1rDLet8aNjjvx{7qXww47r);N$b2bY30xia1GY!t??M(>_WG; z2%2DX*|sr0-q8k4C$Zqgi!T^XbdT@Qv@efL;0zaxqd&COmkDD!{3xm0|5h)uU})!$ zOdH-tlp2398^7-~-0^z4NqFiMtve6e=(i9mJ^@9o9$8@B-vtLWyuHVta`BomAriBs zu_H!tzU97@oa);jnyQH!WvviGPOrK57C(IiaB%Be@p?opXf=IMRfkpp70>D5$bTz{ z#p{2>1BexN(1Nlhcoubi!ogjNJQr0tS0%eVhoi$$pHbFi~i};gj<`iG+weL@HZGD;8{bij=eKYzl>3g91UGM!{Mz`&pAHo5O1pJ#g z=bdn2&F-JK*01$@Akt;n?%MfUO{HC$x}-avTA7-`o@QmSd@a!mcYt%9vucVh>V51F zMLm8zcxRJ^9W=p%WHYCrde-i_9M#F-CjDZ{PiB#!6DIvtI8Nob-G7D$k2IMSzG`w%r&8A2OV{AKuxR zB6g!PhVk|p=KIF$xceKSgZmBr94hs8kXSSu!rGL_=HeOI?wLTtLxIGOxYxr3rtQrn zSeg#lG6@r>l?LlAUM}5`tIe)t^e_c#*@O~1*!|GRr7FAT*y$U+ZW1sk~U;>;e!HRX93_cQr(n1PY|xvs5@ z8Tq`Qa?c)~)Txm=`QKb|StYtS7y}0~c!T%DllTV==R8Pz=F$V>@+sCF5{l5wr(?dL zgGCi1f5t;D1*@MW($xZx6s+N2yDH5y!_7!N{}%`KX} zzA1&|iYaFBC*aqZ!Z&Y5m=Wf(`|+`#8vE^SGwB^$bkUP%je4UAF8ZPvR{;z!dbx#C z1oSr_QP<$wHRKzVja#7xx2CC9e7NJftI~f+IYqMZB;$S4?2+6+SOgG=RTgSZZF4~v z(2UT+`G>@@MgJS~)Mg%fz^`F#$|^uWEsJub!W*SAESmJ2j1Z^iBG}YE^upi@o&0<} zYJQ#zaYf4wdOw{dkXeA6SBETzepvU-faj$g*m{s8$lA-x%YckHj3J1aH~UQr3%@i- zk-AL?9Quni-@l)ExGWQAIQNtOJzKpC0TBF_9s8NQJzUVr(Nm?payu;jYlzls*JiO> zeD-t?_Vb(1WdOkg2CHT~)Cqej^1bI-!ON)b+?W}D>}}gdKQD$ZOqB9d6X0K z)qFQ@yUn9kzl6B}GxmT$2&}Jn>bz(2^KFC*twKaDykUjEA3rMKi??R^5a(T!=NgGmZ|N0G)=}ayKS`u*j~K8PpchjNRbuw*4Xw z^X0f2pGY>RUWGp%)sn?yeTChD1fr}sm=GdL+8P?SlvkK`WLCW_HT89K+Zl-;Pa0BC zNs6ad?s=KZMy`4}+AoVz`qJ98e20%k)u#oIj|Q!JK$j^CNo2u%*l$HcjR2kRnCr1Vrkb#LI(uiG2Tzo29ATe|sO#8rn28-6NDdcN*qd%qdh(gqan%~DXI@OY@F_ez@I@+`dKKy(XX1Q0G#MR5nhRG7; zDj8p5UWF&D*`F~_<_hDQ~TW3 zT8_4|k_q7P`u#A&;wPZpj- z&{a^h+Uswi14ggGFka7yhO&NJX3+ecX1+qwqggCH=IXeg%}>6Rte+Z;-STi|JHza? zA4gouM^9of=|VtbNuwe`=~9m9YW1(sv>9A-neE2Rv#l(D=(^p^si938nY#~x({VtJ zCD_y~nz-GD`OvL+`XKx)dq6tBfz-#kKLHvZ+v+p9eCTM79?3sZ-(^!eHtp*d9F#{) z_YSmLXk5^6Q(IAFfzEGw4N~RAGXcn|UHkf>mT2QN(|WAdb}d6nf3j$>b$&=3hrk+y z3p^4rVSPI)VTIy)6a70nAG<@_DaXuI9OxbVLC8^L0Hm&g&x+Q{5d-My-8y8ULqpVn zEN{az=l7VpkQiD=9vlEn^u>u0u}V&ZDxqbX>gfG*ELh|yb(_u;Gs!qRBLg7R9@vmp z2zoijfp(EwVdPqij=i6&Llwr>ay*)cc7!y%ES2}`GgG)dDmEAHh+PP{=BZ31+;%Y* z;GZmr7&)vNtXhasb~XlZgE-27@FHR%-^uN{v5yKDpnX&cQk(6|{A%uOvmt;Sq^FXp z{c40RH5`=o%5#v0*Z=JFYv4-U59LUmK1WN!?JQp!Y@aZ;915gJTrM-XkZ1rd4?OY3 z@T_pePk8Az1gHq@t*eqR|+G z=XJPCV&Vt?t^zPrMX;+t?qjVmKuE{@b%>j~>Qz^Ie0COBWk#+Z`BH7KW{bxfGkB5= zKfMwT%E;mo=lVOe!de~LGpY<_}( zY=Ryv=2)WRUNX8bd-_L54v!P=L!bOdRIe*6{)IW7G^ucE1fYIK@Q9}`YvB4&33+5> z8QGQpTT}f#E@jwml4J@@y8Gj70XbL zJ@%&%%sR78!<6a#;5##M?x?YxneJgAv+0p@>va`(rix|r82M9*WXbtHo~TH*{T_=- zwv@w? zdvjj?gu+dg0`+Oc%!jkS2|@0_FE*KC2eJqQQpB>853~1^{RGxU?#)pxd^y zQ{-o%mR|-kr$djs_@8s#-9_`A|?Bjk};*9+EyY_=*BVSCRGle7G>H!2= z7_^snEY@*f=$QPh^iLg^a1xFs6u37l4#hHTrgeXEw=pqW#3dPZlZaVDs|%J-u@y+> z$NZ{%HH(@g{*enP3l=f_#S6#stWa=Z@;FIaTYo4aM+lJauV!AtH#uzY=u5NMdGiAD z_hizUP%8%wq35O8OHwa@)X$5!*gwUrJ>Uiw+Xl~Ep5suQV$%IvZct}5XBX%+b7E>` z3eyM@%~}wp^MB%k@o)Xda6ClY{P$Y&B;`2fsrID74fMZrc0~1Mo2X*_h767DK9Q60 zdmwUC`Bf?PmT?wib}K;7aZzB3VoE42?y`ePL_1ViKq7SRRrdHfZY#~3p^-cy@-`4n z@IMtCK*d~Try@VbJck zfBp_E4myZrFzn7^%H`Z_OH|g{?eTOh^(W}C4RyjT8zDmM1z(rExuKx4mP z5S~vUVh9R8LEqp2iCt`BtknZEf=>Xh%0*>b9E=$mB2g^jHGTGLjN628QuA+?e@wz- zF8P~@l``tANH7|;gi84-+cn z{MMv@IK86HUvkwn{VDGorbl36(X=k# zYZ4W+8alQRRKmut`os|WUItPso?gMt5@z|fEV(5rlQhwLRFDW&GZ*pWOI$>ynRTs% zC`WdXUo7<`hbA4Hw_*709d}tMhhS$8)@3eBg1B0R;FQb4_pwmRKQ%X%#P8#u{YG+7 zL969G^)tiR%dw(8b#b$b0V|Vp!`i%dfB;M=6hc0=Bd)?v)?>;M`rP@CVc`&MOISHj zV-fuF=yMqPOMnDl9u@lalX^Aj;XV;;E-z}YX*Un391lDkwq`r+zOn9q#$FOF5e$ng z;4TG(gq%`3R@8qSR}ZDK-;ne#iKVivf!;m>btm^L4?yn4$Tb&g2G`-haSS1ZPs{&4 zr^e~jQ&`yuD3-G2pM^Uo{iVB1!ZTQT4E}p;gv3EihmZ>ZCnpN*!b@+nht7A#`p?5phF7hls{Ex3-myP3MUi{?9cK~yB< z0ni<8hleeNp@em>~gtpe3^usaUL64Drc|Tj5QlX!u|<)l=sv z$8J05OccWKW}R2g0ne|98vH3{=Krbd8b1Gh-_XfC*)Z2X)Od?`wQuEZ&4=-2sltAy zE`aYKUnL4!IkhG*{^BBDUo8N7mzuc)DSwsnJSRB-vUKIQ?wL|!nOvKJ{m=b1Y>i-N z0t*qz`g(p7Ewz6JgzP2F zWxMN%)kcDWEHWs6-0D4QWeR}tNb?$zKSCnI6t6%rd;x0YJYr0m$NC%VoR$AONPo=t z@RX~hM1gDbX-mk;H)=X;C4;#<{TgRI@qO<2z=3LKOmcL!+9A|T(3aDKj}9L;w7Snf&T~F Cl9AH@ literal 0 HcmV?d00001 diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/subject.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/subject.js new file mode 100644 index 000000000000..d0f8df871ee3 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/subject.js @@ -0,0 +1,18 @@ +import { addLcpInstrumentationHandler } from '@sentry-internal/tracing'; + +addLcpInstrumentationHandler(({ metric }) => { + const entry = metric.entries[metric.entries.length - 1]; + window._LCP = entry.size; +}); + +addLcpInstrumentationHandler(({ metric }) => { + const entry = metric.entries[metric.entries.length - 1]; + window._LCP2 = entry.size; +}); + +window.ADD_HANDLER = () => { + addLcpInstrumentationHandler(({ metric }) => { + const entry = metric.entries[metric.entries.length - 1]; + window._LCP3 = entry.size; + }); +}; diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/template.html new file mode 100644 index 000000000000..caf4b8f2deab --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/template.html @@ -0,0 +1,11 @@ + + + + + + +
+ + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts new file mode 100644 index 000000000000..23cd29099a0f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts @@ -0,0 +1,51 @@ +import type { Route } from '@playwright/test'; +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; + +const bundle = process.env.PW_BUNDLE || ''; + +sentryTest( + 'should capture metrics for LCP instrumentation handlers', + async ({ browserName, getLocalTestPath, page }) => { + // This uses a utility that is not exported in CDN bundles + if (shouldSkipTracingTest() || browserName !== 'chromium' || bundle.startsWith('bundle')) { + sentryTest.skip(); + } + + await page.route('**/path/to/image.png', (route: Route) => + route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png` }), + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + const [eventData] = await Promise.all([ + getFirstSentryEnvelopeRequest(page), + page.goto(url), + page.click('button'), + ]); + + expect(eventData.measurements).toBeDefined(); + expect(eventData.measurements?.lcp?.value).toBeDefined(); + + expect(eventData.tags?.['lcp.element']).toBe('body > img'); + expect(eventData.tags?.['lcp.size']).toBe(107400); + expect(eventData.tags?.['lcp.url']).toBe('https://example.com/path/to/image.png'); + + const lcp = await (await page.waitForFunction('window._LCP')).jsonValue(); + const lcp2 = await (await page.waitForFunction('window._LCP2')).jsonValue(); + const lcp3 = await page.evaluate('window._LCP3'); + + expect(lcp).toEqual(107400); + expect(lcp2).toEqual(107400); + // this has not been triggered yet + expect(lcp3).toEqual(undefined); + + // Adding a handler after LCP is completed still triggers the handler + await page.evaluate('window.ADD_HANDLER()'); + const lcp3_2 = await (await page.waitForFunction('window._LCP3')).jsonValue(); + + expect(lcp3_2).toEqual(107400); + }, +); diff --git a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/fixtures/ReplayRecordingData.ts b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/fixtures/ReplayRecordingData.ts index 698cb83b5fb4..0b454ba12214 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/fixtures/ReplayRecordingData.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/fixtures/ReplayRecordingData.ts @@ -215,7 +215,11 @@ export const ReplayRecordingData = [ description: 'largest-contentful-paint', startTimestamp: expect.any(Number), endTimestamp: expect.any(Number), - data: { value: expect.any(Number), size: expect.any(Number) }, + data: { + value: expect.any(Number), + size: expect.any(Number), + nodeId: 16, + }, }, }, }, diff --git a/dev-packages/e2e-tests/test-applications/react-router-6-use-routes/tests/fixtures/ReplayRecordingData.ts b/dev-packages/e2e-tests/test-applications/react-router-6-use-routes/tests/fixtures/ReplayRecordingData.ts index 698cb83b5fb4..0b454ba12214 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6-use-routes/tests/fixtures/ReplayRecordingData.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-6-use-routes/tests/fixtures/ReplayRecordingData.ts @@ -215,7 +215,11 @@ export const ReplayRecordingData = [ description: 'largest-contentful-paint', startTimestamp: expect.any(Number), endTimestamp: expect.any(Number), - data: { value: expect.any(Number), size: expect.any(Number) }, + data: { + value: expect.any(Number), + size: expect.any(Number), + nodeId: 16, + }, }, }, }, diff --git a/dev-packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts b/dev-packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts index 698cb83b5fb4..0b454ba12214 100644 --- a/dev-packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts +++ b/dev-packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts @@ -215,7 +215,11 @@ export const ReplayRecordingData = [ description: 'largest-contentful-paint', startTimestamp: expect.any(Number), endTimestamp: expect.any(Number), - data: { value: expect.any(Number), size: expect.any(Number) }, + data: { + value: expect.any(Number), + size: expect.any(Number), + nodeId: 16, + }, }, }, }, diff --git a/packages/tracing-internal/src/browser/instrument.ts b/packages/tracing-internal/src/browser/instrument.ts index ae4532259f94..2a4e7acaf3b1 100644 --- a/packages/tracing-internal/src/browser/instrument.ts +++ b/packages/tracing-internal/src/browser/instrument.ts @@ -73,6 +73,8 @@ interface Metric { type InstrumentHandlerType = InstrumentHandlerTypeMetric | InstrumentHandlerTypePerformanceObserver; +type StopListening = undefined | void | (() => void); + // eslint-disable-next-line @typescript-eslint/no-explicit-any type InstrumentHandlerCallback = (data: any) => void; @@ -88,17 +90,29 @@ let _previousLcp: Metric | undefined; /** * Add a callback that will be triggered when a CLS metric is available. * Returns a cleanup callback which can be called to remove the instrumentation handler. + * + * Pass `stopOnCallback = true` to stop listening for CLS when the cleanup callback is called. + * This will lead to the CLS being finalized and frozen. */ -export function addClsInstrumentationHandler(callback: (data: { metric: Metric }) => void): CleanupHandlerCallback { - return addMetricObserver('cls', callback, instrumentCls, _previousCls); +export function addClsInstrumentationHandler( + callback: (data: { metric: Metric }) => void, + stopOnCallback = false, +): CleanupHandlerCallback { + return addMetricObserver('cls', callback, instrumentCls, _previousCls, stopOnCallback); } /** * Add a callback that will be triggered when a LCP metric is available. * Returns a cleanup callback which can be called to remove the instrumentation handler. + * + * Pass `stopOnCallback = true` to stop listening for LCP when the cleanup callback is called. + * This will lead to the LCP being finalized and frozen. */ -export function addLcpInstrumentationHandler(callback: (data: { metric: Metric }) => void): CleanupHandlerCallback { - return addMetricObserver('lcp', callback, instrumentLcp, _previousLcp); +export function addLcpInstrumentationHandler( + callback: (data: { metric: Metric }) => void, + stopOnCallback = false, +): CleanupHandlerCallback { + return addMetricObserver('lcp', callback, instrumentLcp, _previousLcp, stopOnCallback); } /** @@ -158,8 +172,8 @@ function triggerHandlers(type: InstrumentHandlerType, data: unknown): void { } } -function instrumentCls(): void { - onCLS(metric => { +function instrumentCls(): StopListening { + return onCLS(metric => { triggerHandlers('cls', { metric, }); @@ -168,7 +182,7 @@ function instrumentCls(): void { } function instrumentFid(): void { - onFID(metric => { + return onFID(metric => { triggerHandlers('fid', { metric, }); @@ -176,8 +190,8 @@ function instrumentFid(): void { }); } -function instrumentLcp(): void { - onLCP(metric => { +function instrumentLcp(): StopListening { + return onLCP(metric => { triggerHandlers('lcp', { metric, }); @@ -188,13 +202,16 @@ function instrumentLcp(): void { function addMetricObserver( type: InstrumentHandlerTypeMetric, callback: InstrumentHandlerCallback, - instrumentFn: () => void, + instrumentFn: () => StopListening, previousValue: Metric | undefined, + stopOnCallback = false, ): CleanupHandlerCallback { addHandler(type, callback); + let stopListening: StopListening | undefined; + if (!instrumented[type]) { - instrumentFn(); + stopListening = instrumentFn(); instrumented[type] = true; } @@ -202,7 +219,7 @@ function addMetricObserver( callback({ metric: previousValue }); } - return getCleanupCallback(type, callback); + return getCleanupCallback(type, callback, stopOnCallback ? stopListening : undefined); } function instrumentPerformanceObserver(type: InstrumentHandlerTypePerformanceObserver): void { @@ -228,8 +245,16 @@ function addHandler(type: InstrumentHandlerType, handler: InstrumentHandlerCallb } // Get a callback which can be called to remove the instrumentation handler -function getCleanupCallback(type: InstrumentHandlerType, callback: InstrumentHandlerCallback): CleanupHandlerCallback { +function getCleanupCallback( + type: InstrumentHandlerType, + callback: InstrumentHandlerCallback, + stopListening: StopListening, +): CleanupHandlerCallback { return () => { + if (stopListening) { + stopListening(); + } + const typeHandlers = handlers[type]; if (!typeHandlers) { diff --git a/packages/tracing-internal/src/browser/metrics/index.ts b/packages/tracing-internal/src/browser/metrics/index.ts index d0eb73b945d2..4c9c25111e11 100644 --- a/packages/tracing-internal/src/browser/metrics/index.ts +++ b/packages/tracing-internal/src/browser/metrics/index.ts @@ -39,7 +39,8 @@ let _lcpEntry: LargestContentfulPaint | undefined; let _clsEntry: LayoutShift | undefined; /** - * Start tracking web vitals + * Start tracking web vitals. + * The callback returned by this function can be used to stop tracking & ensure all measurements are final & captured. * * @returns A function that forces web vitals collection */ @@ -129,7 +130,7 @@ export function startTrackingInteractions(): void { /** Starts tracking the Cumulative Layout Shift on the current page. */ function _trackCLS(): () => void { return addClsInstrumentationHandler(({ metric }) => { - const entry = metric.entries.pop(); + const entry = metric.entries[metric.entries.length - 1]; if (!entry) { return; } @@ -137,13 +138,13 @@ function _trackCLS(): () => void { DEBUG_BUILD && logger.log('[Measurements] Adding CLS'); _measurements['cls'] = { value: metric.value, unit: '' }; _clsEntry = entry as LayoutShift; - }); + }, true); } /** Starts tracking the Largest Contentful Paint on the current page. */ function _trackLCP(): () => void { return addLcpInstrumentationHandler(({ metric }) => { - const entry = metric.entries.pop(); + const entry = metric.entries[metric.entries.length - 1]; if (!entry) { return; } @@ -151,13 +152,13 @@ function _trackLCP(): () => void { DEBUG_BUILD && logger.log('[Measurements] Adding LCP'); _measurements['lcp'] = { value: metric.value, unit: 'millisecond' }; _lcpEntry = entry as LargestContentfulPaint; - }); + }, true); } /** Starts tracking the First Input Delay on the current page. */ function _trackFID(): () => void { return addFidInstrumentationHandler(({ metric }) => { - const entry = metric.entries.pop(); + const entry = metric.entries[metric.entries.length - 1]; if (!entry) { return; } From bf84267a8ba2cf4cadea34587f1c6643a1a4a0ee Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 24 Jan 2024 16:51:11 -0500 Subject: [PATCH 15/15] meta(changelog): Update changelog for 7.96.0 --- CHANGELOG.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28161bfb0ecc..ab411b85a506 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,35 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.96.0 + +### Important Changes + +#### Deprecations + +This release includes some deprecations for integrations in `@sentry/browser` and frontend framework SDKs +(`@sentry/react`, `@sentry/vue`, etc.). Please take a look at our +[migration guide](https://github.com/getsentry/sentry-javascript/blob/develop/MIGRATION.md) for more details. + +- feat(browser): Export functional integrations & deprecate classes (#10267) + +#### Web Vitals Fix for LCP and CLS + +This release fixes an issue with the Web Vitals integration where LCP and CLS were not being captured correctly, +increasing capture rate by 10-30% for some apps. LCP and CLS capturing issues were introduced with version `7.75.0`. + +- fix(tracing): Ensure web vitals are correctly stopped/captured (#10323) + +### Other Changes + +- feat(react): Add `stripBasename` option for React Router 6. (#10314) +- fix(node): Fix `node-cron` types and add test (#10315) +- fix(node): Fix downleveled types entry point (#10321) +- fix(node): LocalVariables integration should use setupOnce (#10307) +- fix(replay): Fix type for options of replayIntegration (#10325) + +Work in this release contributed by @Shubhdeep12. Thank you for your contribution! + ## 7.95.0 ### Important Changes