From 33d1cb0944a06ea4b17fb633a7ddab6c232dfa55 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 19 Jan 2024 09:39:53 +0100 Subject: [PATCH 01/16] feat: Refactor exposed `defaultIntegrations` to `getDefaultIntegrations()` (#10243) The current implementation has two problems: 1. It is weird that you can accidentally mutate the default integrations of another package 2. We sometimes have logic-based default integrations - e.g. adding an integration only if tracing is enabled, or similar. This means that either we have to add some logic in the _upstream_ SDK to ensure this is still added even if downstream SDKs overwrite default integrations, or we need to duplicate the logic in the _downstream_ SDKs. With this new method, we can instead centralize this, and downstream SDKs simply need to call upstream `getDefaultIntegrations(options)`. --- packages/angular-ivy/src/sdk.ts | 5 +- packages/angular/src/sdk.ts | 5 +- packages/angular/test/sdk.test.ts | 27 +++++---- packages/astro/src/index.server.ts | 2 + packages/astro/src/index.types.ts | 1 + packages/browser/src/exports.ts | 2 + packages/browser/src/sdk.ts | 18 ++++-- packages/bun/src/index.ts | 7 ++- packages/bun/src/sdk.ts | 23 ++++++-- packages/deno/src/index.ts | 7 ++- packages/deno/src/sdk.ts | 23 +++++--- packages/deno/test/mod.test.ts | 4 +- packages/nextjs/src/index.types.ts | 1 + packages/nextjs/test/clientSdk.test.ts | 2 +- .../src/integrations/node-fetch.ts | 8 +++ packages/node-experimental/src/sdk/init.ts | 36 ++++++------ .../src/sdk/spanProcessor.ts | 1 - .../node-experimental/test/sdk/init.test.ts | 58 ++++++++----------- packages/node/src/index.ts | 9 ++- packages/node/src/sdk.ts | 36 +++++++----- packages/node/test/index.test.ts | 20 ++----- packages/node/test/sdk.test.ts | 36 ++++-------- packages/remix/src/index.server.ts | 2 + packages/remix/src/index.types.ts | 3 +- packages/serverless/src/awslambda.ts | 17 +++++- packages/serverless/src/gcpfunction/index.ts | 33 ++++++++--- packages/serverless/src/index.ts | 2 + .../serverless/test/__mocks__/@sentry/node.ts | 1 + packages/sveltekit/src/index.types.ts | 1 + packages/sveltekit/src/server/index.ts | 2 + packages/vercel-edge/src/index.ts | 7 ++- packages/vercel-edge/src/sdk.ts | 24 ++++---- packages/vue/src/sdk.ts | 4 +- 33 files changed, 256 insertions(+), 171 deletions(-) diff --git a/packages/angular-ivy/src/sdk.ts b/packages/angular-ivy/src/sdk.ts index 0b222339f45d..3cbf570bcb5d 100644 --- a/packages/angular-ivy/src/sdk.ts +++ b/packages/angular-ivy/src/sdk.ts @@ -1,6 +1,7 @@ import { VERSION } from '@angular/core'; import type { BrowserOptions } from '@sentry/browser'; -import { SDK_VERSION, defaultIntegrations, init as browserInit, setContext } from '@sentry/browser'; +import { getDefaultIntegrations } from '@sentry/browser'; +import { SDK_VERSION, init as browserInit, setContext } from '@sentry/browser'; import type { SdkMetadata } from '@sentry/types'; import { logger } from '@sentry/utils'; @@ -18,7 +19,7 @@ export function init(options: BrowserOptions): void { // see: // - https://github.com/getsentry/sentry-javascript/issues/5417#issuecomment-1453407097 // - https://github.com/getsentry/sentry-javascript/issues/2744 - defaultIntegrations: defaultIntegrations.filter(integration => { + defaultIntegrations: getDefaultIntegrations(options).filter(integration => { return integration.name !== 'TryCatch'; }), ...options, diff --git a/packages/angular/src/sdk.ts b/packages/angular/src/sdk.ts index c448daa037d7..78d3d4ba632b 100755 --- a/packages/angular/src/sdk.ts +++ b/packages/angular/src/sdk.ts @@ -1,6 +1,7 @@ import { VERSION } from '@angular/core'; import type { BrowserOptions } from '@sentry/browser'; -import { SDK_VERSION, defaultIntegrations, init as browserInit, setContext } from '@sentry/browser'; +import { getDefaultIntegrations } from '@sentry/browser'; +import { SDK_VERSION, init as browserInit, setContext } from '@sentry/browser'; import type { SdkMetadata } from '@sentry/types'; import { logger } from '@sentry/utils'; @@ -18,7 +19,7 @@ export function init(options: BrowserOptions): void { // see: // - https://github.com/getsentry/sentry-javascript/issues/5417#issuecomment-1453407097 // - https://github.com/getsentry/sentry-javascript/issues/2744 - defaultIntegrations: defaultIntegrations.filter(integration => { + defaultIntegrations: getDefaultIntegrations(options).filter(integration => { return integration.name !== 'TryCatch'; }), ...options, diff --git a/packages/angular/test/sdk.test.ts b/packages/angular/test/sdk.test.ts index 0a7244d424f7..781d02c61005 100644 --- a/packages/angular/test/sdk.test.ts +++ b/packages/angular/test/sdk.test.ts @@ -1,6 +1,6 @@ import * as SentryBrowser from '@sentry/browser'; -import { defaultIntegrations, init } from '../src/index'; +import { getDefaultIntegrations, init } from '../src/index'; describe('init', () => { it('sets the Angular version (if available) in the global scope', () => { @@ -30,16 +30,23 @@ describe('init', () => { expect(options.defaultIntegrations).not.toContainEqual(expect.objectContaining({ name: 'TryCatch' })); }); - it.each([false as const, defaultIntegrations])( - "doesn't filter if `defaultIntegrations` is set to %s", - defaultIntegrations => { - init({ defaultIntegrations }); + it("doesn't filter if `defaultIntegrations` is set to `false`", () => { + init({ defaultIntegrations: false }); - expect(browserInitSpy).toHaveBeenCalledTimes(1); + expect(browserInitSpy).toHaveBeenCalledTimes(1); + + const options = browserInitSpy.mock.calls[0][0] || {}; + expect(options.defaultIntegrations).toEqual(false); + }); + + it("doesn't filter if `defaultIntegrations` is overwritten", () => { + const defaultIntegrations = getDefaultIntegrations({}); + init({ defaultIntegrations }); - const options = browserInitSpy.mock.calls[0][0] || {}; - expect(options.defaultIntegrations).toEqual(defaultIntegrations); - }, - ); + expect(browserInitSpy).toHaveBeenCalledTimes(1); + + const options = browserInitSpy.mock.calls[0][0] || {}; + expect(options.defaultIntegrations).toEqual(defaultIntegrations); + }); }); }); diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index f2136c859a28..61e2bf2f6c98 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -52,7 +52,9 @@ export { withIsolationScope, autoDiscoverNodePerformanceMonitoringIntegrations, makeNodeTransport, + // eslint-disable-next-line deprecation/deprecation defaultIntegrations, + getDefaultIntegrations, defaultStackParser, // eslint-disable-next-line deprecation/deprecation lastEventId, diff --git a/packages/astro/src/index.types.ts b/packages/astro/src/index.types.ts index 5a693fbc5c63..146d6a6e23b0 100644 --- a/packages/astro/src/index.types.ts +++ b/packages/astro/src/index.types.ts @@ -17,6 +17,7 @@ export declare function init(options: Options | clientSdk.BrowserOptions | serve export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations; export declare const defaultIntegrations: Integration[]; +export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; export declare function close(timeout?: number | undefined): PromiseLike; diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 0ff189977701..41338b4047df 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -89,7 +89,9 @@ export { export { eventFromException, eventFromMessage, exceptionFromError } from './eventbuilder'; export { createUserFeedbackEnvelope } from './userfeedback'; export { + // eslint-disable-next-line deprecation/deprecation defaultIntegrations, + getDefaultIntegrations, forceLoad, init, onLoad, diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 0b64dd0c2735..6332cdac9d4b 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -10,7 +10,7 @@ import { initAndBind, startSession, } from '@sentry/core'; -import type { UserFeedback } from '@sentry/types'; +import type { Integration, Options, UserFeedback } from '@sentry/types'; import { addHistoryInstrumentationHandler, logger, @@ -27,10 +27,12 @@ import { Breadcrumbs, Dedupe, GlobalHandlers, HttpContext, LinkedErrors, TryCatc import { defaultStackParser } from './stack-parsers'; import { makeFetchTransport, makeXHRTransport } from './transports'; -/* eslint-disable deprecation/deprecation */ +/** @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(), @@ -38,7 +40,15 @@ export const defaultIntegrations = [ new Dedupe(), new HttpContext(), ]; -/* eslint-enable deprecation/deprecation */ + +/** Get the default integrations for the browser SDK. */ +export function getDefaultIntegrations(_options: Options): Integration[] { + // We return a copy of the defaultIntegrations here to avoid mutating this + return [ + // eslint-disable-next-line deprecation/deprecation + ...defaultIntegrations, + ]; +} /** * A magic string that build tooling can leverage in order to inject a release value into the SDK. @@ -104,7 +114,7 @@ declare const __SENTRY_RELEASE__: string | undefined; */ export function init(options: BrowserOptions = {}): void { if (options.defaultIntegrations === undefined) { - options.defaultIntegrations = defaultIntegrations; + options.defaultIntegrations = getDefaultIntegrations(options); } if (options.release === undefined) { // This allows build tooling to find-and-replace __SENTRY_RELEASE__ to inject a release value diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index 2f6794b4dae9..725eecb24753 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -88,7 +88,12 @@ export type { SpanStatusType } from '@sentry/core'; export { autoDiscoverNodePerformanceMonitoringIntegrations, cron } from '@sentry/node'; export { BunClient } from './client'; -export { defaultIntegrations, init } from './sdk'; +export { + // eslint-disable-next-line deprecation/deprecation + defaultIntegrations, + getDefaultIntegrations, + init, +} from './sdk'; import { Integrations as CoreIntegrations } from '@sentry/core'; import { Integrations as NodeIntegrations } from '@sentry/node'; diff --git a/packages/bun/src/sdk.ts b/packages/bun/src/sdk.ts index 1288820e2401..ab637bc7a59e 100644 --- a/packages/bun/src/sdk.ts +++ b/packages/bun/src/sdk.ts @@ -1,18 +1,21 @@ /* eslint-disable max-lines */ import { FunctionToString, InboundFilters, LinkedErrors } from '@sentry/core'; import { Integrations as NodeIntegrations, init as initNode } from '@sentry/node'; +import type { Integration, Options } from '@sentry/types'; import { BunClient } from './client'; import { BunServer } from './integrations'; import { makeFetchTransport } from './transports'; import type { BunOptions } from './types'; -/* eslint-disable deprecation/deprecation */ +/** @deprecated Use `getDefaultIntegrations(options)` instead. */ export const defaultIntegrations = [ + /* eslint-disable deprecation/deprecation */ // Common new InboundFilters(), new FunctionToString(), new LinkedErrors(), + /* eslint-enable deprecation/deprecation */ // Native Wrappers new NodeIntegrations.Console(), new NodeIntegrations.Http(), @@ -29,7 +32,15 @@ export const defaultIntegrations = [ // Bun Specific new BunServer(), ]; -/* eslint-enable deprecation/deprecation */ + +/** Get the default integrations for the Bun SDK. */ +export function getDefaultIntegrations(_options: Options): Integration[] { + // We return a copy of the defaultIntegrations here to avoid mutating this + return [ + // eslint-disable-next-line deprecation/deprecation + ...defaultIntegrations, + ]; +} /** * The Sentry Bun SDK Client. @@ -90,9 +101,9 @@ export function init(options: BunOptions = {}): void { options.clientClass = BunClient; options.transport = options.transport || makeFetchTransport; - options.defaultIntegrations = - options.defaultIntegrations === false - ? [] - : [...(Array.isArray(options.defaultIntegrations) ? options.defaultIntegrations : defaultIntegrations)]; + if (options.defaultIntegrations === undefined) { + options.defaultIntegrations = getDefaultIntegrations(options); + } + initNode(options); } diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index 4a300046cc68..d4759e71ce78 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -86,7 +86,12 @@ export type { SpanStatusType } from '@sentry/core'; export { DenoClient } from './client'; -export { defaultIntegrations, init } from './sdk'; +export { + // eslint-disable-next-line deprecation/deprecation + defaultIntegrations, + getDefaultIntegrations, + init, +} from './sdk'; import { Integrations as CoreIntegrations } from '@sentry/core'; diff --git a/packages/deno/src/sdk.ts b/packages/deno/src/sdk.ts index 09416b453394..81834a8b0081 100644 --- a/packages/deno/src/sdk.ts +++ b/packages/deno/src/sdk.ts @@ -2,7 +2,7 @@ import { Breadcrumbs, Dedupe } from '@sentry/browser'; import type { ServerRuntimeClientOptions } from '@sentry/core'; import { FunctionToString, InboundFilters, LinkedErrors } from '@sentry/core'; import { getIntegrationsToSetup, initAndBind } from '@sentry/core'; -import type { StackParser } from '@sentry/types'; +import type { Integration, Options, StackParser } from '@sentry/types'; import { createStackParser, nodeStackLineParser, stackParserFromStackParserOptions } from '@sentry/utils'; import { DenoClient } from './client'; @@ -10,12 +10,14 @@ import { ContextLines, DenoContext, GlobalHandlers, NormalizePaths } from './int import { makeFetchTransport } from './transports'; import type { DenoOptions } from './types'; -/* eslint-disable deprecation/deprecation */ +/** @deprecated Use `getDefaultIntegrations(options)` instead. */ export const defaultIntegrations = [ + /* eslint-disable deprecation/deprecation */ // Common new InboundFilters(), new FunctionToString(), new LinkedErrors(), + /* eslint-enable deprecation/deprecation */ // From Browser new Dedupe(), new Breadcrumbs({ @@ -29,7 +31,15 @@ export const defaultIntegrations = [ new NormalizePaths(), new GlobalHandlers(), ]; -/* eslint-enable deprecation/deprecation */ + +/** Get the default integrations for the Deno SDK. */ +export function getDefaultIntegrations(_options: Options): Integration[] { + // We return a copy of the defaultIntegrations here to avoid mutating this + return [ + // eslint-disable-next-line deprecation/deprecation + ...defaultIntegrations, + ]; +} const defaultStackParser: StackParser = createStackParser(nodeStackLineParser()); @@ -89,10 +99,9 @@ const defaultStackParser: StackParser = createStackParser(nodeStackLineParser()) * @see {@link DenoOptions} for documentation on configuration options. */ export function init(options: DenoOptions = {}): void { - options.defaultIntegrations = - options.defaultIntegrations === false - ? [] - : [...(Array.isArray(options.defaultIntegrations) ? options.defaultIntegrations : defaultIntegrations)]; + if (options.defaultIntegrations === undefined) { + options.defaultIntegrations = getDefaultIntegrations(options); + } const clientOptions: ServerRuntimeClientOptions = { ...options, diff --git a/packages/deno/test/mod.test.ts b/packages/deno/test/mod.test.ts index 1568584d8281..657ce0a1f233 100644 --- a/packages/deno/test/mod.test.ts +++ b/packages/deno/test/mod.test.ts @@ -3,7 +3,7 @@ import { assertSnapshot } from 'https://deno.land/std@0.202.0/testing/snapshot.t import type { sentryTypes } from '../build-test/index.js'; import { sentryUtils } from '../build-test/index.js'; -import { DenoClient, Hub, Scope, defaultIntegrations } from '../build/index.mjs'; +import { DenoClient, Hub, Scope, getDefaultIntegrations } from '../build/index.mjs'; import { getNormalizedEvent } from './normalize.ts'; import { makeTestTransport } from './transport.ts'; @@ -14,7 +14,7 @@ function getTestClient( const client = new DenoClient({ dsn: 'https://233a45e5efe34c47a3536797ce15dafa@nothing.here/5650507', debug: true, - integrations: [...defaultIntegrations, ...integrations], + integrations: [...getDefaultIntegrations({}), ...integrations], stackParser: sentryUtils.createStackParser(sentryUtils.nodeStackLineParser()), transport: makeTestTransport(envelope => { callback(getNormalizedEvent(envelope)); diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index 50c1a747b599..8914296cd3a1 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -25,6 +25,7 @@ export declare const Integrations: typeof clientSdk.Integrations & typeof edgeSdk.Integrations; export declare const defaultIntegrations: Integration[]; +export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; export declare function getSentryRelease(fallback?: string): string | undefined; diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts index bc6732db9c5a..16ee177d4294 100644 --- a/packages/nextjs/test/clientSdk.test.ts +++ b/packages/nextjs/test/clientSdk.test.ts @@ -184,7 +184,7 @@ describe('Client init()', () => { }); const reactInitOptions = reactInit.mock.calls[0][0] as ModifiedInitOptionsIntegrationFunction; - const materializedIntegrations = reactInitOptions.integrations(SentryReact.defaultIntegrations); + const materializedIntegrations = reactInitOptions.integrations(SentryReact.getDefaultIntegrations({})); const browserTracingIntegration = findIntegrationByName(materializedIntegrations, 'BrowserTracing'); expect(browserTracingIntegration).toEqual( diff --git a/packages/node-experimental/src/integrations/node-fetch.ts b/packages/node-experimental/src/integrations/node-fetch.ts index 2f9db367d511..a2b7b61bdfc0 100644 --- a/packages/node-experimental/src/integrations/node-fetch.ts +++ b/packages/node-experimental/src/integrations/node-fetch.ts @@ -4,11 +4,14 @@ import type { Instrumentation } from '@opentelemetry/instrumentation'; import { addBreadcrumb, hasTracingEnabled } from '@sentry/core'; import { _INTERNAL, getClient, getSpanKind } from '@sentry/opentelemetry'; import type { Integration } from '@sentry/types'; +import { parseSemver } from '@sentry/utils'; import type { NodeExperimentalClient } from '../types'; import { addOriginToSpan } from '../utils/addOriginToSpan'; import { NodePerformanceIntegration } from './NodePerformanceIntegration'; +const NODE_VERSION: ReturnType = parseSemver(process.versions.node); + interface NodeFetchOptions { /** * Whether breadcrumbs should be recorded for requests @@ -65,6 +68,11 @@ export class NodeFetch extends NodePerformanceIntegration impl /** @inheritDoc */ public setupInstrumentation(): void | Instrumentation[] { + // Only add NodeFetch if Node >= 16, as previous versions do not support it + if (!NODE_VERSION.major || NODE_VERSION.major < 16) { + return; + } + try { // eslint-disable-next-line @typescript-eslint/no-var-requires const { FetchInstrumentation } = require('opentelemetry-instrumentation-fetch-node'); diff --git a/packages/node-experimental/src/sdk/init.ts b/packages/node-experimental/src/sdk/init.ts index ce22aa109339..353d38be90f2 100644 --- a/packages/node-experimental/src/sdk/init.ts +++ b/packages/node-experimental/src/sdk/init.ts @@ -3,15 +3,15 @@ import { Integrations, defaultIntegrations as defaultNodeIntegrations, defaultStackParser, + getDefaultIntegrations as getDefaultNodeIntegrations, getSentryRelease, makeNodeTransport, } from '@sentry/node'; -import type { Client, Integration } from '@sentry/types'; +import type { Client, Integration, Options } from '@sentry/types'; import { consoleSandbox, dropUndefinedKeys, logger, - parseSemver, stackParserFromStackParserOptions, tracingContextFromHeaders, } from '@sentry/utils'; @@ -28,17 +28,24 @@ import { getGlobalCarrier } from './globals'; import { setLegacyHubOnCarrier } from './hub'; import { initOtel } from './initOtel'; -const NODE_VERSION: ReturnType = parseSemver(process.versions.node); const ignoredDefaultIntegrations = ['Http', 'Undici']; +/** @deprecated Use `getDefaultIntegrations(options)` instead. */ export const defaultIntegrations: Integration[] = [ + // eslint-disable-next-line deprecation/deprecation ...defaultNodeIntegrations.filter(i => !ignoredDefaultIntegrations.includes(i.name)), new Http(), + new NodeFetch(), ]; -// Only add NodeFetch if Node >= 16, as previous versions do not support it -if (NODE_VERSION.major && NODE_VERSION.major >= 16) { - defaultIntegrations.push(new NodeFetch()); +/** Get the default integrations for the Node Experimental SDK. */ +export function getDefaultIntegrations(options: Options): Integration[] { + return [ + ...getDefaultNodeIntegrations(options).filter(i => !ignoredDefaultIntegrations.includes(i.name)), + new Http(), + new NodeFetch(), + ...(hasTracingEnabled(options) ? getAutoPerformanceIntegrations() : []), + ]; } /** @@ -103,18 +110,9 @@ function getClientOptions(options: NodeExperimentalOptions): NodeExperimentalCli const carrier = getGlobalCarrier(); setLegacyHubOnCarrier(carrier); - const isTracingEnabled = hasTracingEnabled(options); - - const autoloadedIntegrations = carrier.integrations || []; - - const fullDefaultIntegrations = - options.defaultIntegrations === false - ? [] - : [ - ...(Array.isArray(options.defaultIntegrations) ? options.defaultIntegrations : defaultIntegrations), - ...(isTracingEnabled ? getAutoPerformanceIntegrations() : []), - ...autoloadedIntegrations, - ]; + if (options.defaultIntegrations === undefined) { + options.defaultIntegrations = getDefaultIntegrations(options); + } const release = getRelease(options.release); @@ -146,7 +144,7 @@ function getClientOptions(options: NodeExperimentalOptions): NodeExperimentalCli instrumenter: 'otel', stackParser: stackParserFromStackParserOptions(options.stackParser || defaultStackParser), integrations: getIntegrationsToSetup({ - defaultIntegrations: fullDefaultIntegrations, + defaultIntegrations: options.defaultIntegrations, integrations: options.integrations, }), }; diff --git a/packages/node-experimental/src/sdk/spanProcessor.ts b/packages/node-experimental/src/sdk/spanProcessor.ts index 73f741505444..226add7753cf 100644 --- a/packages/node-experimental/src/sdk/spanProcessor.ts +++ b/packages/node-experimental/src/sdk/spanProcessor.ts @@ -40,7 +40,6 @@ export class NodeExperimentalSentrySpanProcessor extends SentrySpanProcessor { // In this case, if `shouldCreateSpansForRequests` is false, we want to _record_ the span but not _sample_ it, // So we can generate a breadcrumb for it but no span will be sent if ( - httpIntegration && (span.kind === SpanKind.CLIENT || span.kind === SpanKind.SERVER) && span.attributes[SemanticAttributes.HTTP_URL] && span.attributes[SemanticAttributes.HTTP_METHOD] diff --git a/packages/node-experimental/test/sdk/init.test.ts b/packages/node-experimental/test/sdk/init.test.ts index e220bf7e6ecd..d65734a6ad5c 100644 --- a/packages/node-experimental/test/sdk/init.test.ts +++ b/packages/node-experimental/test/sdk/init.test.ts @@ -1,8 +1,8 @@ import type { Integration } from '@sentry/types'; import * as auto from '../../src/integrations/getAutoPerformanceIntegrations'; -import * as sdk from '../../src/sdk/init'; import { init } from '../../src/sdk/init'; +import { getClient } from '../../src/sdk/scope'; import { cleanupOtel } from '../helpers/mockSdkInit'; // eslint-disable-next-line no-var @@ -18,8 +18,6 @@ class MockIntegration implements Integration { } } -const defaultIntegrationsBackup = sdk.defaultIntegrations; - describe('init()', () => { let mockAutoPerformanceIntegrations: jest.SpyInstance = jest.fn(() => []); @@ -30,25 +28,21 @@ describe('init()', () => { }); afterEach(() => { - // @ts-expect-error - Reset the default integrations of node sdk to original - sdk.defaultIntegrations = defaultIntegrationsBackup; - cleanupOtel(); + + jest.clearAllMocks(); }); it("doesn't install default integrations if told not to", () => { - const mockDefaultIntegrations = [ - new MockIntegration('Mock integration 1.1'), - new MockIntegration('Mock integration 1.2'), - ]; - - // @ts-expect-error - Replace default integrations with mock integrations, needs ts-ignore because imports are readonly - sdk.defaultIntegrations = mockDefaultIntegrations; - init({ dsn: PUBLIC_DSN, defaultIntegrations: false }); - expect(mockDefaultIntegrations[0].setupOnce as jest.Mock).toHaveBeenCalledTimes(0); - expect(mockDefaultIntegrations[1].setupOnce as jest.Mock).toHaveBeenCalledTimes(0); + const client = getClient(); + expect(client.getOptions()).toEqual( + expect.objectContaining({ + integrations: [], + }), + ); + expect(mockAutoPerformanceIntegrations).toHaveBeenCalledTimes(0); }); @@ -58,15 +52,12 @@ describe('init()', () => { new MockIntegration('Some mock integration 2.2'), ]; - // @ts-expect-error - Replace default integrations with mock integrations, needs ts-ignore because imports are readonly - sdk.defaultIntegrations = mockDefaultIntegrations; - const mockIntegrations = [ new MockIntegration('Some mock integration 2.1'), new MockIntegration('Some mock integration 2.3'), ]; - init({ dsn: PUBLIC_DSN, integrations: mockIntegrations }); + init({ dsn: PUBLIC_DSN, integrations: mockIntegrations, defaultIntegrations: mockDefaultIntegrations }); expect(mockDefaultIntegrations[0].setupOnce as jest.Mock).toHaveBeenCalledTimes(0); expect(mockDefaultIntegrations[1].setupOnce as jest.Mock).toHaveBeenCalledTimes(1); @@ -81,13 +72,11 @@ describe('init()', () => { new MockIntegration('Some mock integration 3.2'), ]; - // @ts-expect-error - Replace default integrations with mock integrations, needs ts-ignore because imports are readonly - sdk.defaultIntegrations = mockDefaultIntegrations; - const newIntegration = new MockIntegration('Some mock integration 3.3'); init({ dsn: PUBLIC_DSN, + defaultIntegrations: mockDefaultIntegrations, integrations: integrations => { const newIntegrations = [...integrations]; newIntegrations[1] = newIntegration; @@ -102,14 +91,6 @@ describe('init()', () => { }); it('installs performance default instrumentations if tracing is enabled', () => { - const mockDefaultIntegrations = [ - new MockIntegration('Some mock integration 4.1'), - new MockIntegration('Some mock integration 4.2'), - ]; - - // @ts-expect-error - Replace default integrations with mock integrations, needs ts-ignore because imports are readonly - sdk.defaultIntegrations = mockDefaultIntegrations; - const autoPerformanceIntegration = new MockIntegration('Some mock integration 4.4'); mockAutoPerformanceIntegrations.mockReset().mockImplementation(() => [autoPerformanceIntegration]); @@ -119,13 +100,22 @@ describe('init()', () => { new MockIntegration('Some mock integration 4.3'), ]; - init({ dsn: PUBLIC_DSN, integrations: mockIntegrations, enableTracing: true }); + init({ + dsn: PUBLIC_DSN, + integrations: mockIntegrations, + enableTracing: true, + }); - expect(mockDefaultIntegrations[0].setupOnce as jest.Mock).toHaveBeenCalledTimes(0); - expect(mockDefaultIntegrations[1].setupOnce as jest.Mock).toHaveBeenCalledTimes(1); expect(mockIntegrations[0].setupOnce as jest.Mock).toHaveBeenCalledTimes(1); expect(mockIntegrations[1].setupOnce as jest.Mock).toHaveBeenCalledTimes(1); expect(autoPerformanceIntegration.setupOnce as jest.Mock).toHaveBeenCalledTimes(1); expect(mockAutoPerformanceIntegrations).toHaveBeenCalledTimes(1); + + const client = getClient(); + expect(client.getOptions()).toEqual( + expect.objectContaining({ + integrations: expect.arrayContaining([mockIntegrations[0], mockIntegrations[1], autoPerformanceIntegration]), + }), + ); }); }); diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 72c7528b0606..a48bc13a6d4b 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -90,7 +90,14 @@ export { autoDiscoverNodePerformanceMonitoringIntegrations } from './tracing'; export { NodeClient } from './client'; export { makeNodeTransport } from './transports'; -export { defaultIntegrations, init, defaultStackParser, getSentryRelease } from './sdk'; +export { + // eslint-disable-next-line deprecation/deprecation + defaultIntegrations, + getDefaultIntegrations, + init, + defaultStackParser, + getSentryRelease, +} from './sdk'; export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from '@sentry/utils'; // eslint-disable-next-line deprecation/deprecation export { deepReadDirSync } from './utils'; diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 8a255b5e217a..9ef08c88aeb9 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -12,7 +12,7 @@ import { initAndBind, startSession, } from '@sentry/core'; -import type { SessionStatus, StackParser } from '@sentry/types'; +import type { Integration, Options, SessionStatus, StackParser } from '@sentry/types'; import { GLOBAL_OBJ, createStackParser, @@ -40,12 +40,15 @@ import { createGetModuleFromFilename } from './module'; import { makeNodeTransport } from './transports'; import type { NodeClientOptions, NodeOptions } from './types'; -/* eslint-disable deprecation/deprecation */ +/** @deprecated Use `getDefaultIntegrations(options)` instead. */ + export const defaultIntegrations = [ + /* eslint-disable deprecation/deprecation */ // Common new InboundFilters(), new FunctionToString(), new LinkedErrors(), + /* eslint-enable deprecation/deprecation */ // Native Wrappers new Console(), new Http(), @@ -58,9 +61,22 @@ export const defaultIntegrations = [ new LocalVariables(), new Context(), new Modules(), + // eslint-disable-next-line deprecation/deprecation new RequestData(), ]; -/* eslint-enable deprecation/deprecation */ + +/** Get the default integrations for the Node SDK. */ +export function getDefaultIntegrations(_options: Options): Integration[] { + const carrier = getMainCarrier(); + + const autoloadedIntegrations = carrier.__SENTRY__?.integrations || []; + + return [ + // eslint-disable-next-line deprecation/deprecation + ...defaultIntegrations, + ...autoloadedIntegrations, + ]; +} /** * The Sentry Node SDK Client. @@ -119,19 +135,11 @@ export const defaultIntegrations = [ */ // eslint-disable-next-line complexity export function init(options: NodeOptions = {}): void { - const carrier = getMainCarrier(); - setNodeAsyncContextStrategy(); - const autoloadedIntegrations = carrier.__SENTRY__?.integrations || []; - - options.defaultIntegrations = - options.defaultIntegrations === false - ? [] - : [ - ...(Array.isArray(options.defaultIntegrations) ? options.defaultIntegrations : defaultIntegrations), - ...autoloadedIntegrations, - ]; + if (options.defaultIntegrations === undefined) { + options.defaultIntegrations = getDefaultIntegrations(options); + } if (options.dsn === undefined && process.env.SENTRY_DSN) { options.dsn = process.env.SENTRY_DSN; diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 08f1b7b8f8fe..2312cf8ce981 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -23,7 +23,7 @@ import { } from '../src'; import { setNodeAsyncContextStrategy } from '../src/async'; import { ContextLines } from '../src/integrations'; -import { defaultStackParser } from '../src/sdk'; +import { defaultStackParser, getDefaultIntegrations } from '../src/sdk'; import type { NodeClientOptions } from '../src/types'; import { getDefaultNodeClientOptions } from './helper/node-client-options'; @@ -436,23 +436,13 @@ describe('SentryNode initialization', () => { }); describe('autoloaded integrations', () => { - it('should attach single integration to default integrations', () => { + it('should attach integrations to default integrations', () => { withAutoloadedIntegrations([new MockIntegration('foo')], () => { init({ - defaultIntegrations: [new MockIntegration('bar')], + defaultIntegrations: [...getDefaultIntegrations({}), new MockIntegration('bar')], }); const integrations = (initAndBind as jest.Mock).mock.calls[0][1].defaultIntegrations; - expect(integrations.map((i: { name: string }) => i.name)).toEqual(['bar', 'foo']); - }); - }); - - it('should attach multiple integrations to default integrations', () => { - withAutoloadedIntegrations([new MockIntegration('foo'), new MockIntegration('bar')], () => { - init({ - defaultIntegrations: [new MockIntegration('baz'), new MockIntegration('qux')], - }); - const integrations = (initAndBind as jest.Mock).mock.calls[0][1].defaultIntegrations; - expect(integrations.map((i: { name: string }) => i.name)).toEqual(['baz', 'qux', 'foo', 'bar']); + expect(integrations.map((i: { name: string }) => i.name)).toEqual(expect.arrayContaining(['foo', 'bar'])); }); }); @@ -462,7 +452,7 @@ describe('SentryNode initialization', () => { defaultIntegrations: false, }); const integrations = (initAndBind as jest.Mock).mock.calls[0][1].defaultIntegrations; - expect(integrations).toEqual([]); + expect(integrations).toEqual(false); }); }); }); diff --git a/packages/node/test/sdk.test.ts b/packages/node/test/sdk.test.ts index b4423d8138b9..e29b7d2a7c31 100644 --- a/packages/node/test/sdk.test.ts +++ b/packages/node/test/sdk.test.ts @@ -1,7 +1,7 @@ import type { Integration } from '@sentry/types'; +import * as SentryCore from '@sentry/core'; import { init } from '../src/sdk'; -import * as sdk from '../src/sdk'; // eslint-disable-next-line no-var declare var global: any; @@ -16,31 +16,22 @@ class MockIntegration implements Integration { } } -const defaultIntegrationsBackup = sdk.defaultIntegrations; - describe('init()', () => { + const initAndBindSpy = jest.spyOn(SentryCore, 'initAndBind'); + beforeEach(() => { global.__SENTRY__ = {}; }); - afterEach(() => { - // @ts-expect-error - Reset the default integrations of node sdk to original - sdk.defaultIntegrations = defaultIntegrationsBackup; - }); - it("doesn't install default integrations if told not to", () => { - const mockDefaultIntegrations = [ - new MockIntegration('Mock integration 1.1'), - new MockIntegration('Mock integration 1.2'), - ]; - - // @ts-expect-error - Replace default integrations with mock integrations, needs ts-ignore because imports are readonly - sdk.defaultIntegrations = mockDefaultIntegrations; - init({ dsn: PUBLIC_DSN, defaultIntegrations: false }); - expect(mockDefaultIntegrations[0].setupOnce as jest.Mock).toHaveBeenCalledTimes(0); - expect(mockDefaultIntegrations[1].setupOnce as jest.Mock).toHaveBeenCalledTimes(0); + expect(initAndBindSpy).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + integrations: [], + }), + ); }); it('installs merged default integrations, with overrides provided through options', () => { @@ -49,15 +40,12 @@ describe('init()', () => { new MockIntegration('Some mock integration 2.2'), ]; - // @ts-expect-error - Replace default integrations with mock integrations, needs ts-ignore because imports are readonly - sdk.defaultIntegrations = mockDefaultIntegrations; - const mockIntegrations = [ new MockIntegration('Some mock integration 2.1'), new MockIntegration('Some mock integration 2.3'), ]; - init({ dsn: PUBLIC_DSN, integrations: mockIntegrations }); + init({ dsn: PUBLIC_DSN, integrations: mockIntegrations, defaultIntegrations: mockDefaultIntegrations }); expect(mockDefaultIntegrations[0].setupOnce as jest.Mock).toHaveBeenCalledTimes(0); expect(mockDefaultIntegrations[1].setupOnce as jest.Mock).toHaveBeenCalledTimes(1); @@ -71,13 +59,11 @@ describe('init()', () => { new MockIntegration('Some mock integration 3.2'), ]; - // @ts-expect-error - Replace default integrations with mock integrations, needs ts-ignore because imports are readonly - sdk.defaultIntegrations = mockDefaultIntegrations; - const newIntegration = new MockIntegration('Some mock integration 3.3'); init({ dsn: PUBLIC_DSN, + defaultIntegrations: mockDefaultIntegrations, integrations: integrations => { const newIntegrations = [...integrations]; newIntegrations[1] = newIntegration; diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index 86be1ff8dd2d..ef61c9b2285d 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -51,7 +51,9 @@ export { withIsolationScope, autoDiscoverNodePerformanceMonitoringIntegrations, makeNodeTransport, + // eslint-disable-next-line deprecation/deprecation defaultIntegrations, + getDefaultIntegrations, defaultStackParser, // eslint-disable-next-line deprecation/deprecation lastEventId, diff --git a/packages/remix/src/index.types.ts b/packages/remix/src/index.types.ts index c8266d8db62e..1a7c7fb847e5 100644 --- a/packages/remix/src/index.types.ts +++ b/packages/remix/src/index.types.ts @@ -3,7 +3,7 @@ export * from './index.client'; export * from './index.server'; -import type { Integration, StackParser } from '@sentry/types'; +import type { Integration, Options, StackParser } from '@sentry/types'; import * as clientSdk from './index.client'; import * as serverSdk from './index.server'; @@ -16,6 +16,7 @@ export declare function init(options: RemixOptions): void; export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations; export declare const defaultIntegrations: Integration[]; +export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; // This variable is not a runtime variable but just a type to tell typescript that the methods below can either come diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 9cf05355ee93..e51ecd56fa4d 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -12,11 +12,12 @@ import { defaultIntegrations as nodeDefaultIntegrations, flush, getCurrentScope, + getDefaultIntegrations as getNodeDefaultIntegrations, init as initNode, startSpanManual, withScope, } from '@sentry/node'; -import type { Integration, SdkMetadata, Span } from '@sentry/types'; +import type { Integration, Options, SdkMetadata, Span } from '@sentry/types'; import { isString, logger } from '@sentry/utils'; // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil import type { Context, Handler } from 'aws-lambda'; @@ -66,7 +67,17 @@ export interface WrapperOptions { startTrace: boolean; } -export const defaultIntegrations: Integration[] = [...nodeDefaultIntegrations, new AWSServices({ optional: true })]; +/** @deprecated Use `getDefaultIntegrations(options)` instead. */ +export const defaultIntegrations: Integration[] = [ + // eslint-disable-next-line deprecation/deprecation + ...nodeDefaultIntegrations, + new AWSServices({ optional: true }), +]; + +/** Get the default integrations for the AWSLambda SDK. */ +export function getDefaultIntegrations(options: Options): Integration[] { + return [...getNodeDefaultIntegrations(options), new AWSServices({ optional: true })]; +} interface AWSLambdaOptions extends NodeOptions { /** @@ -84,7 +95,7 @@ interface AWSLambdaOptions extends NodeOptions { export function init(options: AWSLambdaOptions = {}): void { const opts = { _metadata: {} as SdkMetadata, - defaultIntegrations, + defaultIntegrations: getDefaultIntegrations(options), ...options, }; diff --git a/packages/serverless/src/gcpfunction/index.ts b/packages/serverless/src/gcpfunction/index.ts index dc4358b30368..3907e84aabc8 100644 --- a/packages/serverless/src/gcpfunction/index.ts +++ b/packages/serverless/src/gcpfunction/index.ts @@ -1,5 +1,11 @@ -import * as Sentry from '@sentry/node'; -import type { Integration, SdkMetadata } from '@sentry/types'; +import type { NodeOptions } from '@sentry/node'; +import { + SDK_VERSION, + defaultIntegrations as defaultNodeIntegrations, + getDefaultIntegrations as getDefaultNodeIntegrations, + init as initNode, +} from '@sentry/node'; +import type { Integration, Options, SdkMetadata } from '@sentry/types'; import { GoogleCloudGrpc } from '../google-cloud-grpc'; import { GoogleCloudHttp } from '../google-cloud-http'; @@ -8,19 +14,30 @@ export * from './http'; export * from './events'; export * from './cloud_events'; +/** @deprecated Use `getDefaultIntegrations(options)` instead. */ export const defaultIntegrations: Integration[] = [ - ...Sentry.defaultIntegrations, + // eslint-disable-next-line deprecation/deprecation + ...defaultNodeIntegrations, new GoogleCloudHttp({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing. new GoogleCloudGrpc({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing. ]; +/** Get the default integrations for the GCP SDK. */ +export function getDefaultIntegrations(options: Options): Integration[] { + return [ + ...getDefaultNodeIntegrations(options), + new GoogleCloudHttp({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing. + new GoogleCloudGrpc({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing. + ]; +} + /** * @see {@link Sentry.init} */ -export function init(options: Sentry.NodeOptions = {}): void { +export function init(options: NodeOptions = {}): void { const opts = { _metadata: {} as SdkMetadata, - defaultIntegrations, + defaultIntegrations: getDefaultIntegrations(options), ...options, }; @@ -30,11 +47,11 @@ export function init(options: Sentry.NodeOptions = {}): void { packages: [ { name: 'npm:@sentry/serverless', - version: Sentry.SDK_VERSION, + version: SDK_VERSION, }, ], - version: Sentry.SDK_VERSION, + version: SDK_VERSION, }; - Sentry.init(opts); + initNode(opts); } diff --git a/packages/serverless/src/index.ts b/packages/serverless/src/index.ts index 824f2d1374f6..5edbde50fa6c 100644 --- a/packages/serverless/src/index.ts +++ b/packages/serverless/src/index.ts @@ -52,7 +52,9 @@ export { NodeClient, makeNodeTransport, close, + // eslint-disable-next-line deprecation/deprecation defaultIntegrations, + getDefaultIntegrations, defaultStackParser, flush, getSentryRelease, diff --git a/packages/serverless/test/__mocks__/@sentry/node.ts b/packages/serverless/test/__mocks__/@sentry/node.ts index b3ff9edeaa9f..5181b8a0a535 100644 --- a/packages/serverless/test/__mocks__/@sentry/node.ts +++ b/packages/serverless/test/__mocks__/@sentry/node.ts @@ -1,5 +1,6 @@ const origSentry = jest.requireActual('@sentry/node'); export const defaultIntegrations = origSentry.defaultIntegrations; // eslint-disable-line @typescript-eslint/no-unsafe-member-access +export const getDefaultIntegrations = origSentry.getDefaultIntegrations; // eslint-disable-line @typescript-eslint/no-unsafe-member-access export const Handlers = origSentry.Handlers; // eslint-disable-line @typescript-eslint/no-unsafe-member-access export const Integrations = origSentry.Integrations; export const addRequestDataToEvent = origSentry.addRequestDataToEvent; diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index 6a79546869d7..3e57a08c2538 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/packages/sveltekit/src/index.types.ts @@ -40,6 +40,7 @@ export declare function wrapLoadWithSentry any>(orig export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations; export declare const defaultIntegrations: Integration[]; +export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; export declare function close(timeout?: number | undefined): PromiseLike; diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index f57f7c817b91..87bd11c8b7eb 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -49,7 +49,9 @@ export { withIsolationScope, autoDiscoverNodePerformanceMonitoringIntegrations, makeNodeTransport, + // eslint-disable-next-line deprecation/deprecation defaultIntegrations, + getDefaultIntegrations, defaultStackParser, // eslint-disable-next-line deprecation/deprecation lastEventId, diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index 1855b2f90593..b9c18cb6dc1b 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -86,7 +86,12 @@ export { export type { SpanStatusType } from '@sentry/core'; export { VercelEdgeClient } from './client'; -export { defaultIntegrations, init } from './sdk'; +export { + // eslint-disable-next-line deprecation/deprecation + defaultIntegrations, + getDefaultIntegrations, + init, +} from './sdk'; import { Integrations as CoreIntegrations, RequestData } from '@sentry/core'; diff --git a/packages/vercel-edge/src/sdk.ts b/packages/vercel-edge/src/sdk.ts index 690dfbbe1159..ae263b7b01b7 100644 --- a/packages/vercel-edge/src/sdk.ts +++ b/packages/vercel-edge/src/sdk.ts @@ -6,7 +6,7 @@ import { getIntegrationsToSetup, initAndBind, } from '@sentry/core'; -import type { Integration } from '@sentry/types'; +import type { Integration, Options } from '@sentry/types'; import { GLOBAL_OBJ, createStackParser, nodeStackLineParser, stackParserFromStackParserOptions } from '@sentry/utils'; import { setAsyncLocalStorageAsyncContextStrategy } from './async'; @@ -22,6 +22,7 @@ declare const process: { const nodeStackParser = createStackParser(nodeStackLineParser()); +/** @deprecated Use `getDefaultIntegrations(options)` instead. */ export const defaultIntegrations = [ /* eslint-disable deprecation/deprecation */ new InboundFilters(), @@ -31,21 +32,22 @@ export const defaultIntegrations = [ new WinterCGFetch(), ]; +/** Get the default integrations for the browser SDK. */ +export function getDefaultIntegrations(options: Options): Integration[] { + return [ + // eslint-disable-next-line deprecation/deprecation + ...defaultIntegrations, + // eslint-disable-next-line deprecation/deprecation + ...(options.sendDefaultPii ? [new RequestData()] : []), + ]; +} + /** Inits the Sentry NextJS SDK on the Edge Runtime. */ export function init(options: VercelEdgeOptions = {}): void { setAsyncLocalStorageAsyncContextStrategy(); - const sdkDefaultIntegrations: Integration[] = [...defaultIntegrations]; - - // TODO(v8): Add the request data integration by default. - // We don't want to add this functionality OOTB without a breaking change because it might contain PII - if (options.sendDefaultPii) { - // eslint-disable-next-line deprecation/deprecation - sdkDefaultIntegrations.push(new RequestData()); - } - if (options.defaultIntegrations === undefined) { - options.defaultIntegrations = sdkDefaultIntegrations; + options.defaultIntegrations = getDefaultIntegrations(options); } if (options.dsn === undefined && process.env.SENTRY_DSN) { diff --git a/packages/vue/src/sdk.ts b/packages/vue/src/sdk.ts index d448ad117a25..bc307955fa17 100644 --- a/packages/vue/src/sdk.ts +++ b/packages/vue/src/sdk.ts @@ -1,4 +1,4 @@ -import { SDK_VERSION, defaultIntegrations, init as browserInit } from '@sentry/browser'; +import { SDK_VERSION, getDefaultIntegrations, init as browserInit } from '@sentry/browser'; import { vueIntegration } from './integration'; import type { Options, TracingOptions } from './types'; @@ -22,7 +22,7 @@ export function init( version: SDK_VERSION, }, }, - defaultIntegrations: [...defaultIntegrations, vueIntegration()], + defaultIntegrations: [...getDefaultIntegrations(config), vueIntegration()], ...config, }; From 4c0481b3dd79795bf1e6b7483cc2eb7b24d2acd9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 19 Jan 2024 12:48:20 +0100 Subject: [PATCH 02/16] fix(next): Fix custom integrations (#10220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The usage of this was not really working well to begin with, and even worse with the new functional integrations. Because if the user adds the integration themselves (e.g. `integrations: [new RewriteFrames()]`), it will not actually get the correct iteratee at all. Overall it is much cleaner anyhow to just fork the integrations properly and use them instead of the default one - then we can rely on the standard behavior of merging integrations etc. We need to do the same for basically all usages of `addOrUpdateIntegration`, as that actually does not work at all anymore with the functional integrations 😬 (and in many instances never really worked properly if users passed in a custom integration themselves). Co-authored-by: Abhijeet Prasad --- .../src/client/browserTracingIntegration.ts | 26 ++++ packages/nextjs/src/client/index.ts | 125 ++++++++---------- .../src/client/rewriteFramesIntegration.ts | 54 ++++++++ packages/nextjs/src/edge/index.ts | 39 +----- .../src/edge/rewriteFramesIntegration.ts | 52 ++++++++ packages/nextjs/src/index.types.ts | 3 + packages/nextjs/src/server/httpIntegration.ts | 14 ++ packages/nextjs/src/server/index.ts | 78 ++++------- .../server/onUncaughtExceptionIntegration.ts | 14 ++ .../src/server/rewriteFramesIntegration.ts | 52 ++++++++ packages/nextjs/test/clientSdk.test.ts | 87 ++++++------ packages/nextjs/test/serverSdk.test.ts | 47 ++----- 12 files changed, 361 insertions(+), 230 deletions(-) create mode 100644 packages/nextjs/src/client/browserTracingIntegration.ts create mode 100644 packages/nextjs/src/client/rewriteFramesIntegration.ts create mode 100644 packages/nextjs/src/edge/rewriteFramesIntegration.ts create mode 100644 packages/nextjs/src/server/httpIntegration.ts create mode 100644 packages/nextjs/src/server/onUncaughtExceptionIntegration.ts create mode 100644 packages/nextjs/src/server/rewriteFramesIntegration.ts diff --git a/packages/nextjs/src/client/browserTracingIntegration.ts b/packages/nextjs/src/client/browserTracingIntegration.ts new file mode 100644 index 000000000000..c3eb18887301 --- /dev/null +++ b/packages/nextjs/src/client/browserTracingIntegration.ts @@ -0,0 +1,26 @@ +import { BrowserTracing as OriginalBrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/react'; +import { nextRouterInstrumentation } from '../index.client'; + +/** + * A custom BrowserTracing integration for Next.js. + */ +export class BrowserTracing extends OriginalBrowserTracing { + public constructor(options?: ConstructorParameters[0]) { + super({ + // eslint-disable-next-line deprecation/deprecation + tracingOrigins: + process.env.NODE_ENV === 'development' + ? [ + // Will match any URL that contains "localhost" but not "webpack.hot-update.json" - The webpack dev-server + // has cors and it doesn't like extra headers when it's accessed from a different URL. + // TODO(v8): Ideally we rework our tracePropagationTargets logic so this hack won't be necessary anymore (see issue #9764) + /^(?=.*localhost)(?!.*webpack\.hot-update\.json).*/, + /^\/(?!\/)/, + ] + : // eslint-disable-next-line deprecation/deprecation + [...defaultRequestInstrumentationOptions.tracingOrigins, /^(api\/)/], + routingInstrumentation: nextRouterInstrumentation, + ...options, + }); + } +} diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index 46719e47f4f7..f9a2fe5c9b97 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -1,27 +1,28 @@ import { hasTracingEnabled } from '@sentry/core'; -import { RewriteFrames } from '@sentry/integrations'; import type { BrowserOptions } from '@sentry/react'; import { - BrowserTracing, - Integrations, - defaultRequestInstrumentationOptions, + Integrations as OriginalIntegrations, getCurrentScope, + getDefaultIntegrations as getReactDefaultIntegrations, init as reactInit, } from '@sentry/react'; -import type { EventProcessor } from '@sentry/types'; -import { addOrUpdateIntegration } from '@sentry/utils'; +import type { EventProcessor, Integration } from '@sentry/types'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; import { buildMetadata } from '../common/metadata'; -import { nextRouterInstrumentation } from './routing/nextRoutingInstrumentation'; +import { BrowserTracing } from './browserTracingIntegration'; +import { rewriteFramesIntegration } from './rewriteFramesIntegration'; import { applyTunnelRouteOption } from './tunnelRoute'; export * from '@sentry/react'; export { nextRouterInstrumentation } from './routing/nextRoutingInstrumentation'; export { captureUnderscoreErrorException } from '../common/_error'; -export { Integrations }; +export const Integrations = { + ...OriginalIntegrations, + BrowserTracing, +}; // Previously we expected users to import `BrowserTracing` like this: // @@ -33,27 +34,24 @@ export { Integrations }; // // import { BrowserTracing } from '@sentry/nextjs'; // const instance = new BrowserTracing(); -export { BrowserTracing }; +export { BrowserTracing, rewriteFramesIntegration }; // Treeshakable guard to remove all code related to tracing declare const __SENTRY_TRACING__: boolean; -const globalWithInjectedValues = global as typeof global & { - __rewriteFramesAssetPrefixPath__: string; -}; - /** Inits the Sentry NextJS SDK on the browser with the React SDK. */ export function init(options: BrowserOptions): void { const opts = { environment: getVercelEnv(true) || process.env.NODE_ENV, + defaultIntegrations: getDefaultIntegrations(options), ...options, }; + fixBrowserTracingIntegration(opts); + applyTunnelRouteOption(opts); buildMetadata(opts, ['nextjs', 'react']); - addClientIntegrations(opts); - reactInit(opts); const scope = getCurrentScope(); @@ -68,72 +66,53 @@ export function init(options: BrowserOptions): void { } } -function addClientIntegrations(options: BrowserOptions): void { - let integrations = options.integrations || []; - - // This value is injected at build time, based on the output directory specified in the build config. Though a default - // is set there, we set it here as well, just in case something has gone wrong with the injection. - const assetPrefixPath = globalWithInjectedValues.__rewriteFramesAssetPrefixPath__ || ''; - - // eslint-disable-next-line deprecation/deprecation - const defaultRewriteFramesIntegration = new RewriteFrames({ - // Turn `//_next/static/...` into `app:///_next/static/...` - iteratee: frame => { - try { - const { origin } = new URL(frame.filename as string); - frame.filename = frame.filename?.replace(origin, 'app://').replace(assetPrefixPath, ''); - } catch (err) { - // Filename wasn't a properly formed URL, so there's nothing we can do - } - - // We need to URI-decode the filename because Next.js has wildcard routes like "/users/[id].js" which show up as "/users/%5id%5.js" in Error stacktraces. - // The corresponding sources that Next.js generates have proper brackets so we also need proper brackets in the frame so that source map resolving works. - if (frame.filename && frame.filename.startsWith('app:///_next')) { - frame.filename = decodeURI(frame.filename); - } - - if ( - frame.filename && - frame.filename.match( - /^app:\/\/\/_next\/static\/chunks\/(main-|main-app-|polyfills-|webpack-|framework-|framework\.)[0-9a-f]+\.js$/, - ) - ) { - // We don't care about these frames. It's Next.js internal code. - frame.in_app = false; - } - - return frame; - }, - }); - integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations); +// TODO v8: Remove this again +// We need to handle BrowserTracing passed to `integrations` that comes from `@sentry/tracing`, not `@sentry/sveltekit` :( +function fixBrowserTracingIntegration(options: BrowserOptions): void { + const { integrations } = options; + if (!integrations) { + return; + } + + if (Array.isArray(integrations)) { + options.integrations = maybeUpdateBrowserTracingIntegration(integrations); + } else { + options.integrations = defaultIntegrations => { + const userFinalIntegrations = integrations(defaultIntegrations); + + return maybeUpdateBrowserTracingIntegration(userFinalIntegrations); + }; + } +} + +function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Integration[] { + 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 + if (browserTracing && !(browserTracing instanceof BrowserTracing)) { + 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); + } + + return integrations; +} + +function getDefaultIntegrations(options: BrowserOptions): Integration[] { + const customDefaultIntegrations = [...getReactDefaultIntegrations(options), rewriteFramesIntegration()]; // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", in which case everything inside // will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - const defaultBrowserTracingIntegration = new BrowserTracing({ - // eslint-disable-next-line deprecation/deprecation - tracingOrigins: - process.env.NODE_ENV === 'development' - ? [ - // Will match any URL that contains "localhost" but not "webpack.hot-update.json" - The webpack dev-server - // has cors and it doesn't like extra headers when it's accessed from a different URL. - // TODO(v8): Ideally we rework our tracePropagationTargets logic so this hack won't be necessary anymore (see issue #9764) - /^(?=.*localhost)(?!.*webpack\.hot-update\.json).*/, - /^\/(?!\/)/, - ] - : // eslint-disable-next-line deprecation/deprecation - [...defaultRequestInstrumentationOptions.tracingOrigins, /^(api\/)/], - routingInstrumentation: nextRouterInstrumentation, - }); - - integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations, { - 'options.routingInstrumentation': nextRouterInstrumentation, - }); + customDefaultIntegrations.push(new BrowserTracing()); } } - options.integrations = integrations; + return customDefaultIntegrations; } /** diff --git a/packages/nextjs/src/client/rewriteFramesIntegration.ts b/packages/nextjs/src/client/rewriteFramesIntegration.ts new file mode 100644 index 000000000000..5c45ff63d983 --- /dev/null +++ b/packages/nextjs/src/client/rewriteFramesIntegration.ts @@ -0,0 +1,54 @@ +import { defineIntegration } from '@sentry/core'; +import { rewriteFramesIntegration as originalRewriteFramesIntegration } from '@sentry/integrations'; +import type { IntegrationFn, StackFrame } from '@sentry/types'; + +const globalWithInjectedValues = global as typeof global & { + __rewriteFramesAssetPrefixPath__: string; +}; + +type StackFrameIteratee = (frame: StackFrame) => StackFrame; + +interface RewriteFramesOptions { + root?: string; + prefix?: string; + iteratee?: StackFrameIteratee; +} + +export const customRewriteFramesIntegration = ((options?: RewriteFramesOptions) => { + // This value is injected at build time, based on the output directory specified in the build config. Though a default + // is set there, we set it here as well, just in case something has gone wrong with the injection. + const assetPrefixPath = globalWithInjectedValues.__rewriteFramesAssetPrefixPath__ || ''; + + return originalRewriteFramesIntegration({ + // Turn `//_next/static/...` into `app:///_next/static/...` + iteratee: frame => { + try { + const { origin } = new URL(frame.filename as string); + frame.filename = frame.filename?.replace(origin, 'app://').replace(assetPrefixPath, ''); + } catch (err) { + // Filename wasn't a properly formed URL, so there's nothing we can do + } + + // We need to URI-decode the filename because Next.js has wildcard routes like "/users/[id].js" which show up as "/users/%5id%5.js" in Error stacktraces. + // The corresponding sources that Next.js generates have proper brackets so we also need proper brackets in the frame so that source map resolving works. + if (frame.filename && frame.filename.startsWith('app:///_next')) { + frame.filename = decodeURI(frame.filename); + } + + if ( + frame.filename && + frame.filename.match( + /^app:\/\/\/_next\/static\/chunks\/(main-|main-app-|polyfills-|webpack-|framework-|framework\.)[0-9a-f]+\.js$/, + ) + ) { + // We don't care about these frames. It's Next.js internal code. + frame.in_app = false; + } + + return frame; + }, + ...options, + }); +}) satisfies IntegrationFn; + +export const rewriteFramesIntegration = defineIntegration(customRewriteFramesIntegration); diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index fa0e49f7ab59..a322dbed07df 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -1,18 +1,14 @@ import { SDK_VERSION, addTracingExtensions } from '@sentry/core'; -import { RewriteFrames } from '@sentry/integrations'; import type { SdkMetadata } from '@sentry/types'; -import { GLOBAL_OBJ, addOrUpdateIntegration, escapeStringForRegex } from '@sentry/utils'; import type { VercelEdgeOptions } from '@sentry/vercel-edge'; -import { init as vercelEdgeInit } from '@sentry/vercel-edge'; +import { getDefaultIntegrations, init as vercelEdgeInit } from '@sentry/vercel-edge'; import { isBuild } from '../common/utils/isBuild'; +import { rewriteFramesIntegration } from './rewriteFramesIntegration'; export type EdgeOptions = VercelEdgeOptions; -const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & { - __rewriteFramesDistDir__?: string; - fetch: (...args: unknown[]) => unknown; -}; +export { rewriteFramesIntegration }; /** Inits the Sentry NextJS SDK on the Edge Runtime. */ export function init(options: VercelEdgeOptions = {}): void { @@ -22,8 +18,11 @@ export function init(options: VercelEdgeOptions = {}): void { return; } + const customDefaultIntegrations = [...getDefaultIntegrations(options), rewriteFramesIntegration()]; + const opts = { _metadata: {} as SdkMetadata, + defaultIntegrations: customDefaultIntegrations, ...options, }; @@ -38,32 +37,6 @@ export function init(options: VercelEdgeOptions = {}): void { version: SDK_VERSION, }; - let integrations = opts.integrations || []; - - // This value is injected at build time, based on the output directory specified in the build config. Though a default - // is set there, we set it here as well, just in case something has gone wrong with the injection. - const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__; - if (distDirName) { - const distDirAbsPath = distDirName.replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one - - // Normally we would use `path.resolve` to obtain the absolute path we will strip from the stack frame to align with - // the uploaded artifacts, however we don't have access to that API in edge so we need to be a bit more lax. - // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped - const SOURCEMAP_FILENAME_REGEX = new RegExp(`.*${escapeStringForRegex(distDirAbsPath)}`); - - // eslint-disable-next-line deprecation/deprecation - const defaultRewriteFramesIntegration = new RewriteFrames({ - iteratee: frame => { - frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next'); - return frame; - }, - }); - - integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations); - } - - opts.integrations = integrations; - vercelEdgeInit(opts); } diff --git a/packages/nextjs/src/edge/rewriteFramesIntegration.ts b/packages/nextjs/src/edge/rewriteFramesIntegration.ts new file mode 100644 index 000000000000..96e626178c4b --- /dev/null +++ b/packages/nextjs/src/edge/rewriteFramesIntegration.ts @@ -0,0 +1,52 @@ +import { defineIntegration } from '@sentry/core'; +import { + RewriteFrames as OriginalRewriteFrames, + rewriteFramesIntegration as originalRewriteFramesIntegration, +} from '@sentry/integrations'; +import type { IntegrationFn, StackFrame } from '@sentry/types'; +import { GLOBAL_OBJ, escapeStringForRegex } from '@sentry/utils'; + +const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & { + __rewriteFramesDistDir__?: string; +}; + +type StackFrameIteratee = (frame: StackFrame) => StackFrame; +interface RewriteFramesOptions { + root?: string; + prefix?: string; + iteratee?: StackFrameIteratee; +} + +export const customRewriteFramesIntegration = ((options?: RewriteFramesOptions) => { + // This value is injected at build time, based on the output directory specified in the build config. Though a default + // is set there, we set it here as well, just in case something has gone wrong with the injection. + const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__; + + if (distDirName) { + const distDirAbsPath = distDirName.replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one + + // Normally we would use `path.resolve` to obtain the absolute path we will strip from the stack frame to align with + // the uploaded artifacts, however we don't have access to that API in edge so we need to be a bit more lax. + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped + const SOURCEMAP_FILENAME_REGEX = new RegExp(`.*${escapeStringForRegex(distDirAbsPath)}`); + + return originalRewriteFramesIntegration({ + iteratee: frame => { + frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next'); + return frame; + }, + ...options, + }); + } + + // Do nothing if we can't find a distDirName + return { + // eslint-disable-next-line deprecation/deprecation + name: OriginalRewriteFrames.id, + // eslint-disable-next-line @typescript-eslint/no-empty-function + setupOnce: () => {}, + processEvent: event => event, + }; +}) satisfies IntegrationFn; + +export const rewriteFramesIntegration = defineIntegration(customRewriteFramesIntegration); diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index 8914296cd3a1..b15bb6c40bab 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -28,6 +28,9 @@ export declare const defaultIntegrations: Integration[]; export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; +// eslint-disable-next-line deprecation/deprecation +export declare const rewriteFramesIntegration: typeof clientSdk.rewriteFramesIntegration; + export declare function getSentryRelease(fallback?: string): string | undefined; export declare const ErrorBoundary: typeof clientSdk.ErrorBoundary; diff --git a/packages/nextjs/src/server/httpIntegration.ts b/packages/nextjs/src/server/httpIntegration.ts new file mode 100644 index 000000000000..d4e405586e96 --- /dev/null +++ b/packages/nextjs/src/server/httpIntegration.ts @@ -0,0 +1,14 @@ +import { Integrations } from '@sentry/node'; +const { Http: OriginalHttp } = Integrations; + +/** + * A custom HTTP integration where we always enable tracing. + */ +export class Http extends OriginalHttp { + public constructor(options?: ConstructorParameters[0]) { + super({ + ...options, + tracing: true, + }); + } +} diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index fed85cb3adb8..ca1167ecfcb0 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -1,22 +1,35 @@ -import * as path from 'path'; import { addTracingExtensions, getClient } from '@sentry/core'; -import { RewriteFrames } from '@sentry/integrations'; import type { NodeOptions } from '@sentry/node'; -import { Integrations, getCurrentScope, init as nodeInit } from '@sentry/node'; +import { + Integrations as OriginalIntegrations, + getCurrentScope, + getDefaultIntegrations, + init as nodeInit, +} from '@sentry/node'; import type { EventProcessor } from '@sentry/types'; -import type { IntegrationWithExclusionOption } from '@sentry/utils'; -import { addOrUpdateIntegration, escapeStringForRegex, logger } from '@sentry/utils'; +import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../common/debug-build'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; import { buildMetadata } from '../common/metadata'; import { isBuild } from '../common/utils/isBuild'; +import { Http } from './httpIntegration'; +import { OnUncaughtException } from './onUncaughtExceptionIntegration'; +import { rewriteFramesIntegration } from './rewriteFramesIntegration'; export { createReduxEnhancer } from '@sentry/react'; export * from '@sentry/node'; export { captureUnderscoreErrorException } from '../common/_error'; +export const Integrations = { + ...OriginalIntegrations, + Http, + OnUncaughtException, +}; + +export { rewriteFramesIntegration }; + /** * A passthrough error boundary for the server that doesn't depend on any react. Error boundaries don't catch SSR errors * so they should simply be a passthrough. @@ -52,10 +65,6 @@ export function showReportDialog(): void { return; } -const globalWithInjectedValues = global as typeof global & { - __rewriteFramesDistDir__?: string; -}; - // TODO (v8): Remove this /** * @deprecated This constant will be removed in the next major update. @@ -72,8 +81,18 @@ export function init(options: NodeOptions): void { return; } + const customDefaultIntegrations = [ + ...getDefaultIntegrations(options).filter( + integration => !['Http', 'OnUncaughtException'].includes(integration.name), + ), + rewriteFramesIntegration(), + new Http(), + new OnUncaughtException(), + ]; + const opts = { environment: process.env.SENTRY_ENVIRONMENT || getVercelEnv(false) || process.env.NODE_ENV, + defaultIntegrations: customDefaultIntegrations, ...options, // Right now we only capture frontend sessions for Next.js autoSessionTracking: false, @@ -92,8 +111,6 @@ export function init(options: NodeOptions): void { buildMetadata(opts, ['nextjs', 'node']); - addServerIntegrations(opts); - nodeInit(opts); const filterTransactions: EventProcessor = event => { @@ -121,45 +138,6 @@ function sdkAlreadyInitialized(): boolean { return !!getClient(); } -function addServerIntegrations(options: NodeOptions): void { - let integrations = options.integrations || []; - - // This value is injected at build time, based on the output directory specified in the build config. Though a default - // is set there, we set it here as well, just in case something has gone wrong with the injection. - const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__; - if (distDirName) { - // nextjs always puts the build directory at the project root level, which is also where you run `next start` from, so - // we can read in the project directory from the currently running process - const distDirAbsPath = path.resolve(distDirName).replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one - // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped - const SOURCEMAP_FILENAME_REGEX = new RegExp(escapeStringForRegex(distDirAbsPath)); - - // eslint-disable-next-line deprecation/deprecation - const defaultRewriteFramesIntegration = new RewriteFrames({ - iteratee: frame => { - frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next'); - return frame; - }, - }); - integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations); - } - - const defaultOnUncaughtExceptionIntegration: IntegrationWithExclusionOption = new Integrations.OnUncaughtException({ - exitEvenIfOtherHandlersAreRegistered: false, - }); - defaultOnUncaughtExceptionIntegration.allowExclusionByUser = true; - integrations = addOrUpdateIntegration(defaultOnUncaughtExceptionIntegration, integrations, { - _options: { exitEvenIfOtherHandlersAreRegistered: false }, - }); - - const defaultHttpTracingIntegration = new Integrations.Http({ tracing: true }); - integrations = addOrUpdateIntegration(defaultHttpTracingIntegration, integrations, { - _tracing: {}, - }); - - options.integrations = integrations; -} - // TODO (v8): Remove this /** * @deprecated This constant will be removed in the next major update. diff --git a/packages/nextjs/src/server/onUncaughtExceptionIntegration.ts b/packages/nextjs/src/server/onUncaughtExceptionIntegration.ts new file mode 100644 index 000000000000..4659eb682fdb --- /dev/null +++ b/packages/nextjs/src/server/onUncaughtExceptionIntegration.ts @@ -0,0 +1,14 @@ +import { Integrations } from '@sentry/node'; +const { OnUncaughtException: OriginalOnUncaughtException } = Integrations; + +/** + * A custom OnUncaughtException integration that does not exit by default. + */ +export class OnUncaughtException extends OriginalOnUncaughtException { + public constructor(options?: ConstructorParameters[0]) { + super({ + exitEvenIfOtherHandlersAreRegistered: false, + ...options, + }); + } +} diff --git a/packages/nextjs/src/server/rewriteFramesIntegration.ts b/packages/nextjs/src/server/rewriteFramesIntegration.ts new file mode 100644 index 000000000000..f27ff9a9993d --- /dev/null +++ b/packages/nextjs/src/server/rewriteFramesIntegration.ts @@ -0,0 +1,52 @@ +import * as path from 'path'; +import { defineIntegration } from '@sentry/core'; +import { + RewriteFrames as OriginalRewriteFrames, + rewriteFramesIntegration as originalRewriteFramesIntegration, +} from '@sentry/integrations'; +import type { IntegrationFn, StackFrame } from '@sentry/types'; +import { escapeStringForRegex } from '@sentry/utils'; + +const globalWithInjectedValues = global as typeof global & { + __rewriteFramesDistDir__?: string; +}; + +type StackFrameIteratee = (frame: StackFrame) => StackFrame; +interface RewriteFramesOptions { + root?: string; + prefix?: string; + iteratee?: StackFrameIteratee; +} + +export const customRewriteFramesIntegration = ((options?: RewriteFramesOptions) => { + // This value is injected at build time, based on the output directory specified in the build config. Though a default + // is set there, we set it here as well, just in case something has gone wrong with the injection. + const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__; + + if (distDirName) { + // nextjs always puts the build directory at the project root level, which is also where you run `next start` from, so + // we can read in the project directory from the currently running process + const distDirAbsPath = path.resolve(distDirName).replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped + const SOURCEMAP_FILENAME_REGEX = new RegExp(escapeStringForRegex(distDirAbsPath)); + + return originalRewriteFramesIntegration({ + iteratee: frame => { + frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next'); + return frame; + }, + ...options, + }); + } + + // Do nothing if we can't find a distDirName + return { + // eslint-disable-next-line deprecation/deprecation + name: OriginalRewriteFrames.id, + // eslint-disable-next-line @typescript-eslint/no-empty-function + setupOnce: () => {}, + processEvent: event => event, + }; +}) satisfies IntegrationFn; + +export const rewriteFramesIntegration = defineIntegration(customRewriteFramesIntegration); diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts index 16ee177d4294..d0f0de959170 100644 --- a/packages/nextjs/test/clientSdk.test.ts +++ b/packages/nextjs/test/clientSdk.test.ts @@ -1,12 +1,12 @@ -import { BaseClient, getClient } from '@sentry/core'; +import { BaseClient } from '@sentry/core'; import * as SentryReact from '@sentry/react'; -import { BrowserTracing, WINDOW, getCurrentScope } from '@sentry/react'; +import type { BrowserClient } from '@sentry/react'; +import { WINDOW, getClient, getCurrentScope } from '@sentry/react'; import type { Integration } from '@sentry/types'; -import type { UserIntegrationsFunction } from '@sentry/utils'; import { logger } from '@sentry/utils'; import { JSDOM } from 'jsdom'; -import { Integrations, init, nextRouterInstrumentation } from '../src/client'; +import { BrowserTracing, Integrations, init, nextRouterInstrumentation } from '../src/client'; const reactInit = jest.spyOn(SentryReact, 'init'); const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent'); @@ -31,6 +31,8 @@ function findIntegrationByName(integrations: Integration[] = [], name: string): return integrations.find(integration => integration.name === name); } +const TEST_DSN = 'https://public@dsn.ingest.sentry.io/1337'; + describe('Client init()', () => { afterEach(() => { jest.clearAllMocks(); @@ -60,7 +62,7 @@ describe('Client init()', () => { }, }, environment: 'test', - integrations: expect.arrayContaining([ + defaultIntegrations: expect.arrayContaining([ expect.objectContaining({ name: 'RewriteFrames', }), @@ -103,8 +105,7 @@ describe('Client init()', () => { describe('integrations', () => { // Options passed by `@sentry/nextjs`'s `init` to `@sentry/react`'s `init` after modifying them - type ModifiedInitOptionsIntegrationArray = { integrations: Integration[] }; - type ModifiedInitOptionsIntegrationFunction = { integrations: UserIntegrationsFunction }; + type ModifiedInitOptionsIntegrationArray = { defaultIntegrations: Integration[]; integrations: Integration[] }; it('supports passing unrelated integrations through options', () => { init({ integrations: [new Integrations.Breadcrumbs({ console: false })] }); @@ -117,83 +118,87 @@ describe('Client init()', () => { describe('`BrowserTracing` integration', () => { it('adds `BrowserTracing` integration if `tracesSampleRate` is set', () => { - init({ tracesSampleRate: 1.0 }); + init({ + dsn: TEST_DSN, + tracesSampleRate: 1.0, + }); - const reactInitOptions = reactInit.mock.calls[0][0] as ModifiedInitOptionsIntegrationArray; - const browserTracingIntegration = findIntegrationByName(reactInitOptions.integrations, 'BrowserTracing'); + const client = getClient()!; + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeDefined(); - expect(browserTracingIntegration).toEqual( + expect(browserTracingIntegration?.options).toEqual( expect.objectContaining({ - options: expect.objectContaining({ - routingInstrumentation: nextRouterInstrumentation, - }), + routingInstrumentation: nextRouterInstrumentation, }), ); }); it('adds `BrowserTracing` integration if `tracesSampler` is set', () => { - init({ tracesSampler: () => true }); + init({ + dsn: TEST_DSN, + tracesSampler: () => true, + }); - const reactInitOptions = reactInit.mock.calls[0][0] as ModifiedInitOptionsIntegrationArray; - const browserTracingIntegration = findIntegrationByName(reactInitOptions.integrations, 'BrowserTracing'); + const client = getClient()!; + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeDefined(); - expect(browserTracingIntegration).toEqual( + expect(browserTracingIntegration?.options).toEqual( expect.objectContaining({ - options: expect.objectContaining({ - routingInstrumentation: nextRouterInstrumentation, - }), + routingInstrumentation: nextRouterInstrumentation, }), ); }); it('does not add `BrowserTracing` integration if tracing not enabled in SDK', () => { - init({}); + init({ + dsn: TEST_DSN, + }); - const reactInitOptions = reactInit.mock.calls[0][0] as ModifiedInitOptionsIntegrationArray; - const browserTracingIntegration = findIntegrationByName(reactInitOptions.integrations, 'BrowserTracing'); + const client = getClient()!; + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeUndefined(); }); it('forces correct router instrumentation if user provides `BrowserTracing` in an array', () => { init({ + dsn: TEST_DSN, tracesSampleRate: 1.0, integrations: [new BrowserTracing({ startTransactionOnLocationChange: false })], }); - const reactInitOptions = reactInit.mock.calls[0][0] as ModifiedInitOptionsIntegrationArray; - const browserTracingIntegration = findIntegrationByName(reactInitOptions.integrations, 'BrowserTracing'); + const client = getClient()!; + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); - expect(browserTracingIntegration).toEqual( + expect(browserTracingIntegration).toBeDefined(); + expect(browserTracingIntegration?.options).toEqual( expect.objectContaining({ - options: expect.objectContaining({ - routingInstrumentation: nextRouterInstrumentation, - // This proves it's still the user's copy - startTransactionOnLocationChange: false, - }), + routingInstrumentation: nextRouterInstrumentation, + // This proves it's still the user's copy + startTransactionOnLocationChange: false, }), ); }); it('forces correct router instrumentation if user provides `BrowserTracing` in a function', () => { init({ + dsn: TEST_DSN, tracesSampleRate: 1.0, integrations: defaults => [...defaults, new BrowserTracing({ startTransactionOnLocationChange: false })], }); - const reactInitOptions = reactInit.mock.calls[0][0] as ModifiedInitOptionsIntegrationFunction; - const materializedIntegrations = reactInitOptions.integrations(SentryReact.getDefaultIntegrations({})); - const browserTracingIntegration = findIntegrationByName(materializedIntegrations, 'BrowserTracing'); + const client = getClient()!; - expect(browserTracingIntegration).toEqual( + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + + expect(browserTracingIntegration).toBeDefined(); + expect(browserTracingIntegration?.options).toEqual( expect.objectContaining({ - options: expect.objectContaining({ - routingInstrumentation: nextRouterInstrumentation, - // This proves it's still the user's copy - startTransactionOnLocationChange: false, - }), + routingInstrumentation: nextRouterInstrumentation, + // This proves it's still the user's copy + startTransactionOnLocationChange: false, }), ); }); diff --git a/packages/nextjs/test/serverSdk.test.ts b/packages/nextjs/test/serverSdk.test.ts index 5b69f3034d55..1c7c9e384657 100644 --- a/packages/nextjs/test/serverSdk.test.ts +++ b/packages/nextjs/test/serverSdk.test.ts @@ -4,9 +4,7 @@ import { NodeClient, getClient, getCurrentHub, getCurrentScope } from '@sentry/n import type { Integration } from '@sentry/types'; import { GLOBAL_OBJ, logger } from '@sentry/utils'; -import { init } from '../src/server'; - -const { Integrations } = SentryNode; +import { Integrations, init } from '../src/server'; // normally this is set as part of the build process, so mock it here (GLOBAL_OBJ as typeof GLOBAL_OBJ & { __rewriteFramesDistDir__: string }).__rewriteFramesDistDir__ = '.next'; @@ -57,7 +55,7 @@ describe('Server init()', () => { // TODO: If we upgrde to Jest 28+, we can follow Jest's example matcher and create an // `expect.ArrayContainingInAnyOrder`. See // https://github.com/facebook/jest/blob/main/examples/expect-extend/toBeWithinRange.ts. - integrations: expect.any(Array), + defaultIntegrations: expect.any(Array), }), ); }); @@ -155,15 +153,22 @@ describe('Server init()', () => { describe('integrations', () => { // Options passed by `@sentry/nextjs`'s `init` to `@sentry/node`'s `init` after modifying them - type ModifiedInitOptions = { integrations: Integration[] }; + type ModifiedInitOptions = { integrations: Integration[]; defaultIntegrations: Integration[] }; it('adds default integrations', () => { init({}); const nodeInitOptions = nodeInit.mock.calls[0][0] as ModifiedInitOptions; - const rewriteFramesIntegration = findIntegrationByName(nodeInitOptions.integrations, 'RewriteFrames'); + const rewriteFramesIntegration = findIntegrationByName(nodeInitOptions.defaultIntegrations, 'RewriteFrames'); + const httpIntegration = findIntegrationByName(nodeInitOptions.defaultIntegrations, 'Http'); + const onUncaughtExceptionIntegration = findIntegrationByName( + nodeInitOptions.defaultIntegrations, + 'OnUncaughtException', + ); expect(rewriteFramesIntegration).toBeDefined(); + expect(httpIntegration).toBeDefined(); + expect(onUncaughtExceptionIntegration).toBeDefined(); }); it('supports passing unrelated integrations through options', () => { @@ -176,42 +181,18 @@ describe('Server init()', () => { }); describe('`Http` integration', () => { - it('adds `Http` integration with tracing enabled if `tracesSampleRate` is set', () => { + it('adds `Http` integration with tracing enabled by default', () => { init({ tracesSampleRate: 1.0 }); const nodeInitOptions = nodeInit.mock.calls[0][0] as ModifiedInitOptions; - const httpIntegration = findIntegrationByName(nodeInitOptions.integrations, 'Http'); - - expect(httpIntegration).toBeDefined(); - expect(httpIntegration).toEqual(expect.objectContaining({ _tracing: {} })); - }); - - it('adds `Http` integration with tracing enabled if `tracesSampler` is set', () => { - init({ tracesSampler: () => true }); - - const nodeInitOptions = nodeInit.mock.calls[0][0] as ModifiedInitOptions; - const httpIntegration = findIntegrationByName(nodeInitOptions.integrations, 'Http'); - - expect(httpIntegration).toBeDefined(); - expect(httpIntegration).toEqual(expect.objectContaining({ _tracing: {} })); - }); - - it('forces `_tracing = true` if `tracesSampleRate` is set', () => { - init({ - tracesSampleRate: 1.0, - integrations: [new Integrations.Http({ tracing: false })], - }); - - const nodeInitOptions = nodeInit.mock.calls[0][0] as ModifiedInitOptions; - const httpIntegration = findIntegrationByName(nodeInitOptions.integrations, 'Http'); + const httpIntegration = findIntegrationByName(nodeInitOptions.defaultIntegrations, 'Http'); expect(httpIntegration).toBeDefined(); expect(httpIntegration).toEqual(expect.objectContaining({ _tracing: {} })); }); - it('forces `_tracing = true` if `tracesSampler` is set', () => { + it('forces `_tracing = true` even if set to false', () => { init({ - tracesSampler: () => true, integrations: [new Integrations.Http({ tracing: false })], }); From 7688b60ca0788359c615e2bc5592c854185109ba Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 19 Jan 2024 12:50:36 +0100 Subject: [PATCH 03/16] ref(astro): Streamline how we add browser tracing (#10253) We should wait for https://github.com/getsentry/sentry-javascript/pull/10243 to merge this, as otherwise we'll get a deprecation/eslint error there. --- packages/astro/src/client/sdk.ts | 32 +++++++++++++++----------- packages/astro/src/server/sdk.ts | 5 ++-- packages/astro/test/client/sdk.test.ts | 8 +++---- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/packages/astro/src/client/sdk.ts b/packages/astro/src/client/sdk.ts index 2fd98b8a96cd..65cc9d54fa9e 100644 --- a/packages/astro/src/client/sdk.ts +++ b/packages/astro/src/client/sdk.ts @@ -1,7 +1,12 @@ import type { BrowserOptions } from '@sentry/browser'; -import { BrowserTracing, init as initBrowserSdk } from '@sentry/browser'; -import { getCurrentScope, hasTracingEnabled } from '@sentry/core'; -import { addOrUpdateIntegration } from '@sentry/utils'; +import { + BrowserTracing, + getDefaultIntegrations as getBrowserDefaultIntegrations, + init as initBrowserSdk, + setTag, +} from '@sentry/browser'; +import { hasTracingEnabled } from '@sentry/core'; +import type { Integration } from '@sentry/types'; import { applySdkMetadata } from '../common/metadata'; @@ -14,27 +19,26 @@ declare const __SENTRY_TRACING__: boolean; * @param options Configuration options for the SDK. */ export function init(options: BrowserOptions): void { - applySdkMetadata(options, ['astro', 'browser']); + const opts = { + defaultIntegrations: getDefaultIntegrations(options), + ...options, + }; - addClientIntegrations(options); + applySdkMetadata(opts, ['astro', 'browser']); - initBrowserSdk(options); + initBrowserSdk(opts); - getCurrentScope().setTag('runtime', 'browser'); + setTag('runtime', 'browser'); } -function addClientIntegrations(options: BrowserOptions): void { - let integrations = options.integrations || []; - +function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefined { // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", // in which case everything inside will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - const defaultBrowserTracingIntegration = new BrowserTracing({}); - - integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations); + return [...getBrowserDefaultIntegrations(options), new BrowserTracing()]; } } - options.integrations = integrations; + return undefined; } diff --git a/packages/astro/src/server/sdk.ts b/packages/astro/src/server/sdk.ts index e69d27781ed5..2628e94ac9c8 100644 --- a/packages/astro/src/server/sdk.ts +++ b/packages/astro/src/server/sdk.ts @@ -1,6 +1,5 @@ -import { getCurrentScope } from '@sentry/core'; import type { NodeOptions } from '@sentry/node'; -import { init as initNodeSdk } from '@sentry/node'; +import { init as initNodeSdk, setTag } from '@sentry/node'; import { applySdkMetadata } from '../common/metadata'; @@ -13,5 +12,5 @@ export function init(options: NodeOptions): void { initNodeSdk(options); - getCurrentScope().setTag('runtime', 'node'); + setTag('runtime', 'node'); } diff --git a/packages/astro/test/client/sdk.test.ts b/packages/astro/test/client/sdk.test.ts index 161213ea6564..2e10d4210953 100644 --- a/packages/astro/test/client/sdk.test.ts +++ b/packages/astro/test/client/sdk.test.ts @@ -60,7 +60,7 @@ describe('Sentry client SDK', () => { ...tracingOptions, }); - const integrationsToInit = browserInit.mock.calls[0][0]?.integrations; + const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); @@ -76,7 +76,7 @@ describe('Sentry client SDK', () => { ...tracingOptions, }); - const integrationsToInit = browserInit.mock.calls[0][0]?.integrations; + const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); @@ -91,7 +91,7 @@ describe('Sentry client SDK', () => { enableTracing: true, }); - const integrationsToInit = browserInit.mock.calls[0][0]?.integrations; + const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); @@ -107,7 +107,7 @@ describe('Sentry client SDK', () => { enableTracing: true, }); - const integrationsToInit = browserInit.mock.calls[0][0]?.integrations; + const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing') as BrowserTracing; const options = browserTracing.options; From f3b2e7df5af7e5b19b80cf5ffe9617ca380fd8e9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 19 Jan 2024 13:54:29 +0100 Subject: [PATCH 04/16] ref: Streamline SDK metadata handling (#10251) There have been tries to do this before, but let's see how things stand today... --- packages/angular-ivy/src/sdk.ts | 17 ++--------- packages/angular/src/sdk.ts | 17 ++--------- packages/astro/src/client/sdk.ts | 6 ++-- packages/astro/src/server/sdk.ts | 5 ++-- packages/browser/src/client.ts | 16 ++--------- packages/bun/src/client.ts | 15 ++-------- packages/core/src/index.ts | 1 + .../src/utils/sdkMetadata.ts} | 25 +++++++++-------- packages/ember/addon/index.ts | 14 ++-------- packages/gatsby/src/sdk.ts | 16 ++--------- packages/nextjs/src/client/index.ts | 5 ++-- packages/nextjs/src/common/metadata.ts | 23 --------------- packages/nextjs/src/edge/index.ts | 15 ++-------- packages/nextjs/src/server/index.ts | 5 ++-- packages/node-experimental/src/sdk/client.ts | 13 ++------- packages/node/src/client.ts | 14 ++-------- packages/react/src/sdk.ts | 17 +++-------- packages/remix/src/index.client.tsx | 4 +-- packages/remix/src/index.server.ts | 4 +-- packages/remix/src/utils/metadata.ts | 25 ----------------- packages/svelte/src/sdk.ts | 18 ++++-------- packages/sveltekit/src/client/sdk.ts | 5 ++-- packages/sveltekit/src/common/metadata.ts | 28 ------------------- packages/sveltekit/src/server/sdk.ts | 5 ++-- packages/vercel-edge/src/client.ts | 14 ++-------- 25 files changed, 65 insertions(+), 262 deletions(-) rename packages/{astro/src/common/metadata.ts => core/src/utils/sdkMetadata.ts} (59%) delete mode 100644 packages/nextjs/src/common/metadata.ts delete mode 100644 packages/remix/src/utils/metadata.ts delete mode 100644 packages/sveltekit/src/common/metadata.ts diff --git a/packages/angular-ivy/src/sdk.ts b/packages/angular-ivy/src/sdk.ts index 3cbf570bcb5d..da73fb49a1a2 100644 --- a/packages/angular-ivy/src/sdk.ts +++ b/packages/angular-ivy/src/sdk.ts @@ -1,8 +1,7 @@ import { VERSION } from '@angular/core'; import type { BrowserOptions } from '@sentry/browser'; -import { getDefaultIntegrations } from '@sentry/browser'; -import { SDK_VERSION, init as browserInit, setContext } from '@sentry/browser'; -import type { SdkMetadata } from '@sentry/types'; +import { getDefaultIntegrations, init as browserInit, setContext } from '@sentry/browser'; +import { applySdkMetadata } from '@sentry/core'; import { logger } from '@sentry/utils'; import { IS_DEBUG_BUILD } from './flags'; @@ -12,7 +11,6 @@ import { IS_DEBUG_BUILD } from './flags'; */ export function init(options: BrowserOptions): void { const opts = { - _metadata: {} as SdkMetadata, // Filter out TryCatch integration as it interferes with our Angular `ErrorHandler`: // TryCatch would catch certain errors before they reach the `ErrorHandler` and thus provide a // lower fidelity error than what `SentryErrorHandler` (see errorhandler.ts) would provide. @@ -25,16 +23,7 @@ export function init(options: BrowserOptions): void { ...options, }; - opts._metadata.sdk = opts._metadata.sdk || { - name: 'sentry.javascript.angular-ivy', - packages: [ - { - name: 'npm:@sentry/angular-ivy', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(opts, 'angular-ivy'); checkAndSetAngularVersion(); browserInit(opts); diff --git a/packages/angular/src/sdk.ts b/packages/angular/src/sdk.ts index 78d3d4ba632b..8e6bbc5eb0d6 100755 --- a/packages/angular/src/sdk.ts +++ b/packages/angular/src/sdk.ts @@ -1,8 +1,7 @@ import { VERSION } from '@angular/core'; import type { BrowserOptions } from '@sentry/browser'; -import { getDefaultIntegrations } from '@sentry/browser'; -import { SDK_VERSION, init as browserInit, setContext } from '@sentry/browser'; -import type { SdkMetadata } from '@sentry/types'; +import { getDefaultIntegrations, init as browserInit, setContext } from '@sentry/browser'; +import { applySdkMetadata } from '@sentry/core'; import { logger } from '@sentry/utils'; import { IS_DEBUG_BUILD } from './flags'; @@ -12,7 +11,6 @@ import { IS_DEBUG_BUILD } from './flags'; */ export function init(options: BrowserOptions): void { const opts = { - _metadata: {} as SdkMetadata, // Filter out TryCatch integration as it interferes with our Angular `ErrorHandler`: // TryCatch would catch certain errors before they reach the `ErrorHandler` and thus provide a // lower fidelity error than what `SentryErrorHandler` (see errorhandler.ts) would provide. @@ -25,16 +23,7 @@ export function init(options: BrowserOptions): void { ...options, }; - opts._metadata.sdk = opts._metadata.sdk || { - name: 'sentry.javascript.angular', - packages: [ - { - name: 'npm:@sentry/angular', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(opts, 'angular'); checkAndSetAngularVersion(); browserInit(opts); diff --git a/packages/astro/src/client/sdk.ts b/packages/astro/src/client/sdk.ts index 65cc9d54fa9e..8d2b70ee6751 100644 --- a/packages/astro/src/client/sdk.ts +++ b/packages/astro/src/client/sdk.ts @@ -5,11 +5,9 @@ import { init as initBrowserSdk, setTag, } from '@sentry/browser'; -import { hasTracingEnabled } from '@sentry/core'; +import { applySdkMetadata, hasTracingEnabled } from '@sentry/core'; import type { Integration } from '@sentry/types'; -import { applySdkMetadata } from '../common/metadata'; - // Treeshakable guard to remove all code related to tracing declare const __SENTRY_TRACING__: boolean; @@ -24,7 +22,7 @@ export function init(options: BrowserOptions): void { ...options, }; - applySdkMetadata(opts, ['astro', 'browser']); + applySdkMetadata(opts, 'astro', ['astro', 'browser']); initBrowserSdk(opts); diff --git a/packages/astro/src/server/sdk.ts b/packages/astro/src/server/sdk.ts index 2628e94ac9c8..6a529b60b74e 100644 --- a/packages/astro/src/server/sdk.ts +++ b/packages/astro/src/server/sdk.ts @@ -1,14 +1,13 @@ +import { applySdkMetadata } from '@sentry/core'; import type { NodeOptions } from '@sentry/node'; import { init as initNodeSdk, setTag } from '@sentry/node'; -import { applySdkMetadata } from '../common/metadata'; - /** * * @param options */ export function init(options: NodeOptions): void { - applySdkMetadata(options, ['astro', 'node']); + applySdkMetadata(options, 'astro', ['astro', 'node']); initNodeSdk(options); diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 14d4660b8482..2f1c5ba2fdbb 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,5 +1,6 @@ import type { Scope } from '@sentry/core'; -import { BaseClient, SDK_VERSION } from '@sentry/core'; +import { applySdkMetadata } from '@sentry/core'; +import { BaseClient } from '@sentry/core'; import type { BrowserClientProfilingOptions, BrowserClientReplayOptions, @@ -50,18 +51,7 @@ export class BrowserClient extends BaseClient { */ public constructor(options: BrowserClientOptions) { const sdkSource = WINDOW.SENTRY_SDK_SOURCE || getSDKSource(); - - options._metadata = options._metadata || {}; - options._metadata.sdk = options._metadata.sdk || { - name: 'sentry.javascript.browser', - packages: [ - { - name: `${sdkSource}:@sentry/browser`, - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(options, 'browser', ['browser'], sdkSource); super(options); diff --git a/packages/bun/src/client.ts b/packages/bun/src/client.ts index cb77b37b1111..7b3a350f2821 100644 --- a/packages/bun/src/client.ts +++ b/packages/bun/src/client.ts @@ -1,6 +1,7 @@ import * as os from 'os'; import type { ServerRuntimeClientOptions } from '@sentry/core'; -import { SDK_VERSION, ServerRuntimeClient } from '@sentry/core'; +import { applySdkMetadata } from '@sentry/core'; +import { ServerRuntimeClient } from '@sentry/core'; import type { BunClientOptions } from './types'; @@ -16,17 +17,7 @@ export class BunClient extends ServerRuntimeClient { * @param options Configuration options for this SDK. */ public constructor(options: BunClientOptions) { - options._metadata = options._metadata || {}; - options._metadata.sdk = options._metadata.sdk || { - name: 'sentry.javascript.bun', - packages: [ - { - name: 'npm:@sentry/bun', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(options, 'bun'); const clientOptions: ServerRuntimeClientOptions = { ...options, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 871332a20ad6..3eec5fe4ca0a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -86,6 +86,7 @@ export { spanIsSampled, } from './utils/spanUtils'; export { getRootSpan } from './utils/getRootSpan'; +export { applySdkMetadata } from './utils/sdkMetadata'; export { DEFAULT_ENVIRONMENT } from './constants'; /* eslint-disable deprecation/deprecation */ export { ModuleMetadata } from './integrations/metadata'; diff --git a/packages/astro/src/common/metadata.ts b/packages/core/src/utils/sdkMetadata.ts similarity index 59% rename from packages/astro/src/common/metadata.ts rename to packages/core/src/utils/sdkMetadata.ts index ddd53f27362a..e865861f4f55 100644 --- a/packages/astro/src/common/metadata.ts +++ b/packages/core/src/utils/sdkMetadata.ts @@ -1,7 +1,5 @@ -import { SDK_VERSION } from '@sentry/core'; -import type { Options, SdkInfo } from '@sentry/types'; - -const PACKAGE_NAME_PREFIX = 'npm:@sentry/'; +import type { Options } from '@sentry/types'; +import { SDK_VERSION } from '../version'; /** * A builder for the SDK metadata in the options for the SDK initialization. @@ -16,16 +14,19 @@ const PACKAGE_NAME_PREFIX = 'npm:@sentry/'; * @param options SDK options object that gets mutated * @param names list of package names */ -export function applySdkMetadata(options: Options, names: string[]): void { - options._metadata = options._metadata || {}; - options._metadata.sdk = - options._metadata.sdk || - ({ - name: 'sentry.javascript.astro', +export function applySdkMetadata(options: Options, name: string, names = [name], source = 'npm'): void { + const metadata = options._metadata || {}; + + if (!metadata.sdk) { + metadata.sdk = { + name: `sentry.javascript.${name}`, packages: names.map(name => ({ - name: `${PACKAGE_NAME_PREFIX}${name}`, + name: `${source}:@sentry/${name}`, version: SDK_VERSION, })), version: SDK_VERSION, - } as SdkInfo); + }; + } + + options._metadata = metadata; } diff --git a/packages/ember/addon/index.ts b/packages/ember/addon/index.ts index 009ab2dfe6e0..07b13697676e 100644 --- a/packages/ember/addon/index.ts +++ b/packages/ember/addon/index.ts @@ -5,7 +5,7 @@ import { getOwnConfig, isDevelopingApp, macroCondition } from '@embroider/macros import { startSpan } from '@sentry/browser'; import type { BrowserOptions } from '@sentry/browser'; import * as Sentry from '@sentry/browser'; -import { SDK_VERSION } from '@sentry/browser'; +import { applySdkMetadata } from '@sentry/core'; import { GLOBAL_OBJ } from '@sentry/utils'; import Ember from 'ember'; @@ -33,17 +33,7 @@ export function InitSentryForEmber(_runtimeConfig?: BrowserOptions): void { Object.assign(environmentConfig.sentry, _runtimeConfig || {}); const initConfig = Object.assign({}, environmentConfig.sentry); - initConfig._metadata = initConfig._metadata || {}; - initConfig._metadata.sdk = { - name: 'sentry.javascript.ember', - packages: [ - { - name: 'npm:@sentry/ember', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(initConfig, 'ember'); // Persist Sentry init options so they are identical when performance initializers call init again. const sentryInitConfig = _getSentryInitConfig(); diff --git a/packages/gatsby/src/sdk.ts b/packages/gatsby/src/sdk.ts index 26c9ab831da3..d615daa8682a 100644 --- a/packages/gatsby/src/sdk.ts +++ b/packages/gatsby/src/sdk.ts @@ -1,4 +1,5 @@ -import { SDK_VERSION, init as reactInit } from '@sentry/react'; +import { applySdkMetadata } from '@sentry/core'; +import { init as reactInit } from '@sentry/react'; import { getIntegrationsFromOptions } from './utils/integrations'; import type { GatsbyOptions } from './utils/types'; @@ -7,18 +8,7 @@ import type { GatsbyOptions } from './utils/types'; * Inits the Sentry Gatsby SDK. */ export function init(options: GatsbyOptions): void { - options._metadata = options._metadata || {}; - options._metadata.sdk = options._metadata.sdk || { - name: 'sentry.javascript.gatsby', - packages: [ - { - name: 'npm:@sentry/gatsby', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; - + applySdkMetadata(options, 'gatsby'); const integrations = getIntegrationsFromOptions(options); reactInit({ ...options, diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index f9a2fe5c9b97..14c788a44307 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -1,4 +1,4 @@ -import { hasTracingEnabled } from '@sentry/core'; +import { applySdkMetadata, hasTracingEnabled } from '@sentry/core'; import type { BrowserOptions } from '@sentry/react'; import { Integrations as OriginalIntegrations, @@ -10,7 +10,6 @@ import type { EventProcessor, Integration } from '@sentry/types'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; -import { buildMetadata } from '../common/metadata'; import { BrowserTracing } from './browserTracingIntegration'; import { rewriteFramesIntegration } from './rewriteFramesIntegration'; import { applyTunnelRouteOption } from './tunnelRoute'; @@ -50,7 +49,7 @@ export function init(options: BrowserOptions): void { fixBrowserTracingIntegration(opts); applyTunnelRouteOption(opts); - buildMetadata(opts, ['nextjs', 'react']); + applySdkMetadata(opts, 'nextjs', ['nextjs', 'react']); reactInit(opts); diff --git a/packages/nextjs/src/common/metadata.ts b/packages/nextjs/src/common/metadata.ts deleted file mode 100644 index 9216c6eaec07..000000000000 --- a/packages/nextjs/src/common/metadata.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { SDK_VERSION } from '@sentry/core'; -import type { Options, SdkInfo } from '@sentry/types'; - -const PACKAGE_NAME_PREFIX = 'npm:@sentry/'; - -/** - * A builder for the SDK metadata in the options for the SDK initialization. - * @param options sdk options object that gets mutated - * @param names list of package names - */ -export function buildMetadata(options: Options, names: string[]): void { - options._metadata = options._metadata || {}; - options._metadata.sdk = - options._metadata.sdk || - ({ - name: 'sentry.javascript.nextjs', - packages: names.map(name => ({ - name: `${PACKAGE_NAME_PREFIX}${name}`, - version: SDK_VERSION, - })), - version: SDK_VERSION, - } as SdkInfo); -} diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index a322dbed07df..42599277c451 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -1,5 +1,4 @@ -import { SDK_VERSION, addTracingExtensions } from '@sentry/core'; -import type { SdkMetadata } from '@sentry/types'; +import { addTracingExtensions, applySdkMetadata } from '@sentry/core'; import type { VercelEdgeOptions } from '@sentry/vercel-edge'; import { getDefaultIntegrations, init as vercelEdgeInit } from '@sentry/vercel-edge'; @@ -21,21 +20,11 @@ export function init(options: VercelEdgeOptions = {}): void { const customDefaultIntegrations = [...getDefaultIntegrations(options), rewriteFramesIntegration()]; const opts = { - _metadata: {} as SdkMetadata, defaultIntegrations: customDefaultIntegrations, ...options, }; - opts._metadata.sdk = opts._metadata.sdk || { - name: 'sentry.javascript.nextjs', - packages: [ - { - name: 'npm:@sentry/nextjs', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(opts, 'nextjs'); vercelEdgeInit(opts); } diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index ca1167ecfcb0..1373bb7a0905 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -1,4 +1,4 @@ -import { addTracingExtensions, getClient } from '@sentry/core'; +import { addTracingExtensions, applySdkMetadata, getClient } from '@sentry/core'; import type { NodeOptions } from '@sentry/node'; import { Integrations as OriginalIntegrations, @@ -12,7 +12,6 @@ import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../common/debug-build'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; -import { buildMetadata } from '../common/metadata'; import { isBuild } from '../common/utils/isBuild'; import { Http } from './httpIntegration'; import { OnUncaughtException } from './onUncaughtExceptionIntegration'; @@ -109,7 +108,7 @@ export function init(options: NodeOptions): void { return; } - buildMetadata(opts, ['nextjs', 'node']); + applySdkMetadata(opts, 'nextjs', ['nextjs', 'node']); nodeInit(opts); diff --git a/packages/node-experimental/src/sdk/client.ts b/packages/node-experimental/src/sdk/client.ts index 6f1012bdb422..15d5720e4ac9 100644 --- a/packages/node-experimental/src/sdk/client.ts +++ b/packages/node-experimental/src/sdk/client.ts @@ -3,6 +3,7 @@ import { NodeClient, SDK_VERSION } from '@sentry/node'; import type { Tracer } from '@opentelemetry/api'; import { trace } from '@opentelemetry/api'; import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base'; +import { applySdkMetadata } from '@sentry/core'; import type { CaptureContext, Event, EventHint } from '@sentry/types'; import { Scope, getIsolationScope } from './scope'; @@ -12,17 +13,7 @@ export class NodeExperimentalClient extends NodeClient { private _tracer: Tracer | undefined; public constructor(options: ConstructorParameters[0]) { - options._metadata = options._metadata || {}; - options._metadata.sdk = options._metadata.sdk || { - name: 'sentry.javascript.node-experimental', - packages: [ - { - name: 'npm:@sentry/node-experimental', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(options, 'node-experimental'); super(options); } diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 70fcd043d537..8611f902ab0e 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,7 +1,7 @@ import * as os from 'os'; import { TextEncoder } from 'util'; import type { ServerRuntimeClientOptions } from '@sentry/core'; -import { SDK_VERSION, ServerRuntimeClient } from '@sentry/core'; +import { ServerRuntimeClient, applySdkMetadata } from '@sentry/core'; import type { NodeClientOptions } from './types'; @@ -17,17 +17,7 @@ export class NodeClient extends ServerRuntimeClient { * @param options Configuration options for this SDK. */ public constructor(options: NodeClientOptions) { - options._metadata = options._metadata || {}; - options._metadata.sdk = options._metadata.sdk || { - name: 'sentry.javascript.node', - packages: [ - { - name: 'npm:@sentry/node', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(options, 'node'); // Until node supports global TextEncoder in all versions we support, we are forced to pass it from util options.transportOptions = { diff --git a/packages/react/src/sdk.ts b/packages/react/src/sdk.ts index eeba4c58907d..24e75e8556f5 100644 --- a/packages/react/src/sdk.ts +++ b/packages/react/src/sdk.ts @@ -1,25 +1,16 @@ import type { BrowserOptions } from '@sentry/browser'; -import { SDK_VERSION, init as browserInit } from '@sentry/browser'; -import type { SdkMetadata } from '@sentry/types'; +import { init as browserInit } from '@sentry/browser'; +import { applySdkMetadata } from '@sentry/core'; /** * Inits the React SDK */ export function init(options: BrowserOptions): void { const opts = { - _metadata: {} as SdkMetadata, ...options, }; - opts._metadata.sdk = opts._metadata.sdk || { - name: 'sentry.javascript.react', - packages: [ - { - name: 'npm:@sentry/react', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(opts, 'react'); + browserInit(opts); } diff --git a/packages/remix/src/index.client.tsx b/packages/remix/src/index.client.tsx index 63b39253416d..9c619ff1d851 100644 --- a/packages/remix/src/index.client.tsx +++ b/packages/remix/src/index.client.tsx @@ -1,13 +1,13 @@ +import { applySdkMetadata } from '@sentry/core'; import { getCurrentScope, init as reactInit } from '@sentry/react'; -import { buildMetadata } from './utils/metadata'; import type { RemixOptions } from './utils/remixOptions'; export { remixRouterInstrumentation, withSentry } from './client/performance'; export { captureRemixErrorBoundaryError } from './client/errors'; export * from '@sentry/react'; export function init(options: RemixOptions): void { - buildMetadata(options, ['remix', 'react']); + applySdkMetadata(options, 'remix', ['remix', 'react']); options.environment = options.environment || process.env.NODE_ENV; reactInit(options); diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index ef61c9b2285d..240a7d3787e1 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -1,3 +1,4 @@ +import { applySdkMetadata } from '@sentry/core'; import type { NodeOptions } from '@sentry/node'; import { getClient } from '@sentry/node'; import { getCurrentScope, init as nodeInit } from '@sentry/node'; @@ -5,7 +6,6 @@ import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from './utils/debug-build'; import { instrumentServer } from './utils/instrumentServer'; -import { buildMetadata } from './utils/metadata'; import type { RemixOptions } from './utils/remixOptions'; // We need to explicitly export @sentry/node as they end up under `default` in ESM builds @@ -87,7 +87,7 @@ function sdkAlreadyInitialized(): boolean { /** Initializes Sentry Remix SDK on Node. */ export function init(options: RemixOptions): void { - buildMetadata(options, ['remix', 'node']); + applySdkMetadata(options, 'remix', ['remix', 'node']); if (sdkAlreadyInitialized()) { DEBUG_BUILD && logger.log('SDK already initialized'); diff --git a/packages/remix/src/utils/metadata.ts b/packages/remix/src/utils/metadata.ts deleted file mode 100644 index 3270e1c983cc..000000000000 --- a/packages/remix/src/utils/metadata.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { SDK_VERSION } from '@sentry/core'; -import type { SdkInfo } from '@sentry/types'; - -import type { RemixOptions } from './remixOptions'; - -const PACKAGE_NAME_PREFIX = 'npm:@sentry/'; - -/** - * A builder for the SDK metadata in the options for the SDK initialization. - * @param options sdk options object that gets mutated - * @param names list of package names - */ -export function buildMetadata(options: RemixOptions, names: string[]): void { - options._metadata = options._metadata || {}; - options._metadata.sdk = - options._metadata.sdk || - ({ - name: 'sentry.javascript.remix', - packages: names.map(name => ({ - name: `${PACKAGE_NAME_PREFIX}${name}`, - version: SDK_VERSION, - })), - version: SDK_VERSION, - } as SdkInfo); -} diff --git a/packages/svelte/src/sdk.ts b/packages/svelte/src/sdk.ts index fb6cb575cdcd..30d35d7963f4 100644 --- a/packages/svelte/src/sdk.ts +++ b/packages/svelte/src/sdk.ts @@ -1,26 +1,18 @@ import type { BrowserOptions } from '@sentry/browser'; -import { SDK_VERSION, addEventProcessor, init as browserInit } from '@sentry/browser'; -import type { EventProcessor, SdkMetadata } from '@sentry/types'; +import { addEventProcessor, init as browserInit } from '@sentry/browser'; +import { applySdkMetadata } from '@sentry/core'; +import type { EventProcessor } from '@sentry/types'; import { getDomElement } from '@sentry/utils'; /** * Inits the Svelte SDK */ export function init(options: BrowserOptions): void { const opts = { - _metadata: {} as SdkMetadata, ...options, }; - opts._metadata.sdk = opts._metadata.sdk || { - name: 'sentry.javascript.svelte', - packages: [ - { - name: 'npm:@sentry/svelte', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + applySdkMetadata(opts, 'svelte'); + browserInit(opts); detectAndReportSvelteKit(); diff --git a/packages/sveltekit/src/client/sdk.ts b/packages/sveltekit/src/client/sdk.ts index ebfd1f281404..daf8cd3ed7d2 100644 --- a/packages/sveltekit/src/client/sdk.ts +++ b/packages/sveltekit/src/client/sdk.ts @@ -1,9 +1,8 @@ -import { hasTracingEnabled } from '@sentry/core'; +import { applySdkMetadata, hasTracingEnabled } from '@sentry/core'; import type { BrowserOptions } from '@sentry/svelte'; import { BrowserTracing, WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte'; import { addOrUpdateIntegration } from '@sentry/utils'; -import { applySdkMetadata } from '../common/metadata'; import { svelteKitRoutingInstrumentation } from './router'; type WindowWithSentryFetchProxy = typeof WINDOW & { @@ -19,7 +18,7 @@ declare const __SENTRY_TRACING__: boolean; * @param options Configuration options for the SDK. */ export function init(options: BrowserOptions): void { - applySdkMetadata(options, ['sveltekit', 'svelte']); + applySdkMetadata(options, 'sveltekit', ['sveltekit', 'svelte']); addClientIntegrations(options); diff --git a/packages/sveltekit/src/common/metadata.ts b/packages/sveltekit/src/common/metadata.ts deleted file mode 100644 index d6acf72510cb..000000000000 --- a/packages/sveltekit/src/common/metadata.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { SDK_VERSION } from '@sentry/core'; -import type { Options, SdkInfo } from '@sentry/types'; - -const PACKAGE_NAME_PREFIX = 'npm:@sentry/'; - -/** - * A builder for the SDK metadata in the options for the SDK initialization. - * - * Note: This function is identical to `buildMetadata` in Remix and NextJS. - * We don't extract it for bundle size reasons. - * If you make changes to this function consider updating the others as well. - * - * @param options SDK options object that gets mutated - * @param names list of package names - */ -export function applySdkMetadata(options: Options, names: string[]): void { - options._metadata = options._metadata || {}; - options._metadata.sdk = - options._metadata.sdk || - ({ - name: 'sentry.javascript.sveltekit', - packages: names.map(name => ({ - name: `${PACKAGE_NAME_PREFIX}${name}`, - version: SDK_VERSION, - })), - version: SDK_VERSION, - } as SdkInfo); -} diff --git a/packages/sveltekit/src/server/sdk.ts b/packages/sveltekit/src/server/sdk.ts index b4e216b271ca..1001e922a031 100644 --- a/packages/sveltekit/src/server/sdk.ts +++ b/packages/sveltekit/src/server/sdk.ts @@ -1,10 +1,9 @@ -import { getCurrentScope } from '@sentry/core'; +import { applySdkMetadata, getCurrentScope } from '@sentry/core'; import { RewriteFrames } from '@sentry/integrations'; import type { NodeOptions } from '@sentry/node'; import { init as initNodeSdk } from '@sentry/node'; import { addOrUpdateIntegration } from '@sentry/utils'; -import { applySdkMetadata } from '../common/metadata'; import { rewriteFramesIteratee } from './utils'; /** @@ -12,7 +11,7 @@ import { rewriteFramesIteratee } from './utils'; * @param options */ export function init(options: NodeOptions): void { - applySdkMetadata(options, ['sveltekit', 'node']); + applySdkMetadata(options, 'sveltekit', ['sveltekit', 'node']); addServerIntegrations(options); diff --git a/packages/vercel-edge/src/client.ts b/packages/vercel-edge/src/client.ts index 2ded776719b4..b2c7416130bc 100644 --- a/packages/vercel-edge/src/client.ts +++ b/packages/vercel-edge/src/client.ts @@ -1,5 +1,6 @@ import type { ServerRuntimeClientOptions } from '@sentry/core'; -import { SDK_VERSION, ServerRuntimeClient } from '@sentry/core'; +import { applySdkMetadata } from '@sentry/core'; +import { ServerRuntimeClient } from '@sentry/core'; import type { VercelEdgeClientOptions } from './types'; @@ -19,17 +20,8 @@ export class VercelEdgeClient extends ServerRuntimeClient Date: Fri, 19 Jan 2024 14:24:23 +0100 Subject: [PATCH 05/16] feat(core): Deprecate `Span.parentSpanId` (#10244) Deprecate the `Span.parentSpanId` field on the interface and class. This required only a couple of code replacements and a bunch of test adjustments. Also went ahead and changed the integration test event type in the tests I was modifying. --- MIGRATION.md | 1 + .../suites/public-api/startSpan/basic/test.ts | 11 +++---- .../startTransaction/basic_usage/test.ts | 30 ++++++++--------- .../tracing/metrics/web-vitals-fid/test.ts | 8 ++--- .../tracing/metrics/web-vitals-fp-fcp/test.ts | 14 +++----- docs/v8-new-performance-apis.md | 2 +- packages/core/src/tracing/span.ts | 32 +++++++++++++------ packages/core/test/lib/scope.test.ts | 5 ++- packages/core/test/lib/tracing/trace.test.ts | 6 ++++ .../test/spanprocessor.test.ts | 23 +++++++++++++ .../src/browser/browsertracing.ts | 3 +- packages/types/src/span.ts | 7 ++++ 12 files changed, 94 insertions(+), 48 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index df21637dc6a4..0d27fa476ffd 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -193,6 +193,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar - `span.getTraceContext()`: Use `spanToTraceContext(span)` utility function instead. - `span.sampled`: Use `span.isRecording()` instead. - `span.spanId`: Use `span.spanContext().spanId` instead. +- `span.parentSpanId`: Use `spanToJSON(span).parent_span_id` instead. - `span.traceId`: Use `span.spanContext().traceId` instead. - `span.name`: Use `spanToJSON(span).description` instead. - `span.description`: Use `spanToJSON(span).description` instead. diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts index 95d09927e463..9df0e24f3a38 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts @@ -1,5 +1,5 @@ import { expect } from '@playwright/test'; -import type { Event } from '@sentry/types'; +import type { SerializedEvent } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; @@ -10,7 +10,7 @@ sentryTest('should send a transaction in an envelope', async ({ getLocalTestPath } const url = await getLocalTestPath({ testDir: __dirname }); - const transaction = await getFirstSentryEnvelopeRequest(page, url); + const transaction = await getFirstSentryEnvelopeRequest(page, url); expect(transaction.transaction).toBe('parent_span'); expect(transaction.spans).toBeDefined(); @@ -22,14 +22,13 @@ sentryTest('should report finished spans as children of the root transaction', a } const url = await getLocalTestPath({ testDir: __dirname }); - const transaction = await getFirstSentryEnvelopeRequest(page, url); + const transaction = await getFirstSentryEnvelopeRequest(page, url); - const rootSpanId = transaction?.contexts?.trace?.spanId; + const rootSpanId = transaction?.contexts?.trace?.span_id; expect(transaction.spans).toHaveLength(1); const span_1 = transaction.spans?.[0]; - // eslint-disable-next-line deprecation/deprecation expect(span_1?.description).toBe('child_span'); - expect(span_1?.parentSpanId).toEqual(rootSpanId); + expect(span_1?.parent_span_id).toEqual(rootSpanId); }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts index 7176b951225f..aa5a332ac748 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts @@ -1,5 +1,5 @@ import { expect } from '@playwright/test'; -import type { Event } from '@sentry/types'; +import type { SerializedEvent } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; @@ -10,7 +10,7 @@ sentryTest('should report a transaction in an envelope', async ({ getLocalTestPa } const url = await getLocalTestPath({ testDir: __dirname }); - const transaction = await getFirstSentryEnvelopeRequest(page, url); + const transaction = await getFirstSentryEnvelopeRequest(page, url); expect(transaction.transaction).toBe('test_transaction_1'); expect(transaction.spans).toBeDefined(); @@ -22,28 +22,26 @@ sentryTest('should report finished spans as children of the root transaction', a } const url = await getLocalTestPath({ testDir: __dirname }); - const transaction = await getFirstSentryEnvelopeRequest(page, url); + const transaction = await getFirstSentryEnvelopeRequest(page, url); - const rootSpanId = transaction?.contexts?.trace?.spanId; + const rootSpanId = transaction?.contexts?.trace?.span_id; expect(transaction.spans).toHaveLength(3); const span_1 = transaction.spans?.[0]; - // eslint-disable-next-line deprecation/deprecation - expect(span_1?.op).toBe('span_1'); - expect(span_1?.parentSpanId).toEqual(rootSpanId); - // eslint-disable-next-line deprecation/deprecation - expect(span_1?.data).toMatchObject({ foo: 'bar', baz: [1, 2, 3] }); + expect(span_1).toBeDefined(); + expect(span_1!.op).toBe('span_1'); + expect(span_1!.parent_span_id).toEqual(rootSpanId); + expect(span_1!.data).toMatchObject({ foo: 'bar', baz: [1, 2, 3] }); const span_3 = transaction.spans?.[1]; - // eslint-disable-next-line deprecation/deprecation - expect(span_3?.op).toBe('span_3'); - expect(span_3?.parentSpanId).toEqual(rootSpanId); + expect(span_3).toBeDefined(); + expect(span_3!.op).toBe('span_3'); + expect(span_3!.parent_span_id).toEqual(rootSpanId); const span_5 = transaction.spans?.[2]; - // eslint-disable-next-line deprecation/deprecation - expect(span_5?.op).toBe('span_5'); - // eslint-disable-next-line deprecation/deprecation - expect(span_5?.parentSpanId).toEqual(span_3?.spanId); + expect(span_5).toBeDefined(); + expect(span_5!.op).toBe('span_5'); + expect(span_5!.parent_span_id).toEqual(span_3!.span_id); }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts index e61588f91e73..90dde1ec2ecd 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts @@ -1,5 +1,5 @@ import { expect } from '@playwright/test'; -import type { Event } from '@sentry/types'; +import type { SerializedEvent } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; @@ -16,16 +16,14 @@ sentryTest('should capture a FID vital.', async ({ browserName, getLocalTestPath // To trigger FID await page.click('#fid-btn'); - const eventData = await getFirstSentryEnvelopeRequest(page); + const eventData = await getFirstSentryEnvelopeRequest(page); expect(eventData.measurements).toBeDefined(); expect(eventData.measurements?.fid?.value).toBeDefined(); - // eslint-disable-next-line deprecation/deprecation const fidSpan = eventData.spans?.filter(({ description }) => description === 'first input delay')[0]; expect(fidSpan).toBeDefined(); - // eslint-disable-next-line deprecation/deprecation expect(fidSpan?.op).toBe('ui.action'); - expect(fidSpan?.parentSpanId).toBe(eventData.contexts?.trace_span_id); + expect(fidSpan?.parent_span_id).toBe(eventData.contexts?.trace?.span_id); }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts index 2d8b77b61c7a..2ba417a84c18 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts @@ -1,5 +1,5 @@ import { expect } from '@playwright/test'; -import type { Event } from '@sentry/types'; +import type { SerializedEvent } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; @@ -11,18 +11,16 @@ sentryTest('should capture FP vital.', async ({ browserName, getLocalTestPath, p } const url = await getLocalTestPath({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); + const eventData = await getFirstSentryEnvelopeRequest(page, url); expect(eventData.measurements).toBeDefined(); expect(eventData.measurements?.fp?.value).toBeDefined(); - // eslint-disable-next-line deprecation/deprecation const fpSpan = eventData.spans?.filter(({ description }) => description === 'first-paint')[0]; expect(fpSpan).toBeDefined(); - // eslint-disable-next-line deprecation/deprecation expect(fpSpan?.op).toBe('paint'); - expect(fpSpan?.parentSpanId).toBe(eventData.contexts?.trace_span_id); + expect(fpSpan?.parent_span_id).toBe(eventData.contexts?.trace?.span_id); }); sentryTest('should capture FCP vital.', async ({ getLocalTestPath, page }) => { @@ -31,16 +29,14 @@ sentryTest('should capture FCP vital.', async ({ getLocalTestPath, page }) => { } const url = await getLocalTestPath({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); + const eventData = await getFirstSentryEnvelopeRequest(page, url); expect(eventData.measurements).toBeDefined(); expect(eventData.measurements?.fcp?.value).toBeDefined(); - // eslint-disable-next-line deprecation/deprecation const fcpSpan = eventData.spans?.filter(({ description }) => description === 'first-contentful-paint')[0]; expect(fcpSpan).toBeDefined(); - // eslint-disable-next-line deprecation/deprecation expect(fcpSpan?.op).toBe('paint'); - expect(fcpSpan?.parentSpanId).toBe(eventData.contexts?.trace_span_id); + expect(fcpSpan?.parent_span_id).toBe(eventData.contexts?.trace?.span_id); }); diff --git a/docs/v8-new-performance-apis.md b/docs/v8-new-performance-apis.md index 2f258ec386cb..4b3c10f55a2a 100644 --- a/docs/v8-new-performance-apis.md +++ b/docs/v8-new-performance-apis.md @@ -46,7 +46,7 @@ below to see which things used to exist, and how they can/should be mapped going | --------------------- | ---------------------------------------------------- | | `traceId` | `spanContext().traceId` | | `spanId` | `spanContext().spanId` | -| `parentSpanId` | Unchanged | +| `parentSpanId` | `spanToJSON(span).parent_span_id` | | `status` | use utility method TODO | | `sampled` | `spanIsSampled(span)` | | `startTimestamp` | `startTime` - note that this has a different format! | diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 6938ef00e47a..6371435e2d58 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -63,11 +63,6 @@ export class SpanRecorder { * Span contains all data about a span */ export class Span implements SpanInterface { - /** - * @inheritDoc - */ - public parentSpanId?: string; - /** * Tags for the span. * @deprecated Use `getSpanAttributes(span)` instead. @@ -112,6 +107,7 @@ export class Span implements SpanInterface { protected _traceId: string; protected _spanId: string; + protected _parentSpanId?: string; protected _sampled: boolean | undefined; protected _name?: string; protected _attributes: SpanAttributes; @@ -147,7 +143,7 @@ export class Span implements SpanInterface { this._name = spanContext.name || spanContext.description; if (spanContext.parentSpanId) { - this.parentSpanId = spanContext.parentSpanId; + this._parentSpanId = spanContext.parentSpanId; } // We want to include booleans as well here if ('sampled' in spanContext) { @@ -231,6 +227,24 @@ export class Span implements SpanInterface { this._spanId = spanId; } + /** + * @inheritDoc + * + * @deprecated Use `startSpan` functions instead. + */ + public set parentSpanId(string) { + this._parentSpanId = string; + } + + /** + * @inheritDoc + * + * @deprecated Use `spanToJSON(span).parent_span_id` instead. + */ + public get parentSpanId(): string | undefined { + return this._parentSpanId; + } + /** * Was this span chosen to be sent as part of the sample? * @deprecated Use `isRecording()` instead. @@ -531,7 +545,7 @@ export class Span implements SpanInterface { endTimestamp: this._endTime, // eslint-disable-next-line deprecation/deprecation op: this.op, - parentSpanId: this.parentSpanId, + parentSpanId: this._parentSpanId, sampled: this._sampled, spanId: this._spanId, startTimestamp: this._startTime, @@ -555,7 +569,7 @@ export class Span implements SpanInterface { this._endTime = spanContext.endTimestamp; // eslint-disable-next-line deprecation/deprecation this.op = spanContext.op; - this.parentSpanId = spanContext.parentSpanId; + this._parentSpanId = spanContext.parentSpanId; this._sampled = spanContext.sampled; this._spanId = spanContext.spanId || this._spanId; this._startTime = spanContext.startTimestamp || this._startTime; @@ -589,7 +603,7 @@ export class Span implements SpanInterface { data: this._getData(), description: this._name, op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] as string | undefined, - parent_span_id: this.parentSpanId, + parent_span_id: this._parentSpanId, span_id: this._spanId, start_timestamp: this._startTime, status: this._status, diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts index 90678eb5062b..e4510ef58e3b 100644 --- a/packages/core/test/lib/scope.test.ts +++ b/packages/core/test/lib/scope.test.ts @@ -7,6 +7,7 @@ import { getCurrentScope, getIsolationScope, makeMain, + spanToJSON, startInactiveSpan, startSpan, withIsolationScope, @@ -543,12 +544,14 @@ describe('withActiveSpan()', () => { }); it('should create child spans when calling startSpan within the callback', done => { - expect.assertions(1); + expect.assertions(2); const inactiveSpan = startInactiveSpan({ name: 'inactive-span' }); withActiveSpan(inactiveSpan!, () => { startSpan({ name: 'child-span' }, childSpan => { + // eslint-disable-next-line deprecation/deprecation expect(childSpan?.parentSpanId).toBe(inactiveSpan?.spanContext().spanId); + expect(spanToJSON(childSpan!).parent_span_id).toBe(inactiveSpan?.spanContext().spanId); done(); }); }); diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 344bea5b7818..a2bdac43b6dd 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -276,6 +276,8 @@ describe('startSpan', () => { expect(getCurrentScope()).toBe(manualScope); expect(getActiveSpan()).toBe(span); + expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id'); + // eslint-disable-next-line deprecation/deprecation expect(span?.parentSpanId).toBe('parent-span-id'); }); @@ -323,6 +325,8 @@ describe('startSpanManual', () => { expect(getCurrentScope()).not.toBe(initialScope); expect(getCurrentScope()).toBe(manualScope); expect(getActiveSpan()).toBe(span); + expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id'); + // eslint-disable-next-line deprecation/deprecation expect(span?.parentSpanId).toBe('parent-span-id'); finish(); @@ -377,6 +381,8 @@ describe('startInactiveSpan', () => { const span = startInactiveSpan({ name: 'GET users/[id]', scope: manualScope }); expect(span).toBeDefined(); + expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id'); + // eslint-disable-next-line deprecation/deprecation expect(span?.parentSpanId).toBe('parent-span-id'); expect(getActiveSpan()).toBeUndefined(); diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index 85e56a92f814..23818b17442e 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -93,6 +93,8 @@ describe('SentrySpanProcessor', () => { expect(spanToJSON(sentrySpanTransaction!).description).toBe('GET /users'); expect(spanToJSON(sentrySpanTransaction!).start_timestamp).toEqual(startTimestampMs / 1000); expect(sentrySpanTransaction?.spanContext().traceId).toEqual(otelSpan.spanContext().traceId); + expect(spanToJSON(sentrySpanTransaction!).parent_span_id).toEqual(otelSpan.parentSpanId); + // eslint-disable-next-line deprecation/deprecation expect(sentrySpanTransaction?.parentSpanId).toEqual(otelSpan.parentSpanId); expect(sentrySpanTransaction?.spanContext().spanId).toEqual(otelSpan.spanContext().spanId); @@ -121,6 +123,9 @@ describe('SentrySpanProcessor', () => { expect(sentrySpan ? spanToJSON(sentrySpan).description : undefined).toBe('SELECT * FROM users;'); expect(spanToJSON(sentrySpan!).start_timestamp).toEqual(startTimestampMs / 1000); expect(sentrySpan?.spanContext().spanId).toEqual(childOtelSpan.spanContext().spanId); + + expect(spanToJSON(sentrySpan!).parent_span_id).toEqual(sentrySpanTransaction?.spanContext().spanId); + // eslint-disable-next-line deprecation/deprecation expect(sentrySpan?.parentSpanId).toEqual(sentrySpanTransaction?.spanContext().spanId); // eslint-disable-next-line deprecation/deprecation @@ -162,6 +167,9 @@ describe('SentrySpanProcessor', () => { expect(spanToJSON(sentrySpan!).description).toBe('SELECT * FROM users;'); expect(spanToJSON(sentrySpan!).start_timestamp).toEqual(startTimestampMs / 1000); expect(sentrySpan?.spanContext().spanId).toEqual(childOtelSpan.spanContext().spanId); + + expect(spanToJSON(sentrySpan!).parent_span_id).toEqual(parentOtelSpan.spanContext().spanId); + // eslint-disable-next-line deprecation/deprecation expect(sentrySpan?.parentSpanId).toEqual(parentOtelSpan.spanContext().spanId); // eslint-disable-next-line deprecation/deprecation @@ -194,8 +202,16 @@ describe('SentrySpanProcessor', () => { const sentrySpan2 = getSpanForOtelSpan(span2); const sentrySpan3 = getSpanForOtelSpan(span3); + expect(spanToJSON(sentrySpan1!).parent_span_id).toEqual(sentrySpanTransaction?.spanContext().spanId); + // eslint-disable-next-line deprecation/deprecation expect(sentrySpan1?.parentSpanId).toEqual(sentrySpanTransaction?.spanContext().spanId); + + expect(spanToJSON(sentrySpan2!).parent_span_id).toEqual(sentrySpanTransaction?.spanContext().spanId); + // eslint-disable-next-line deprecation/deprecation expect(sentrySpan2?.parentSpanId).toEqual(sentrySpanTransaction?.spanContext().spanId); + + expect(spanToJSON(sentrySpan3!).parent_span_id).toEqual(sentrySpanTransaction?.spanContext().spanId); + // eslint-disable-next-line deprecation/deprecation expect(sentrySpan3?.parentSpanId).toEqual(sentrySpanTransaction?.spanContext().spanId); expect(spanToJSON(sentrySpan1!).description).toEqual('SELECT * FROM users;'); @@ -255,7 +271,14 @@ describe('SentrySpanProcessor', () => { expect(childSpan).toBeInstanceOf(Transaction); expect(spanToJSON(parentSpan!).timestamp).toBeDefined(); expect(spanToJSON(childSpan!).timestamp).toBeDefined(); + expect(spanToJSON(parentSpan!).parent_span_id).toBeUndefined(); + + expect(spanToJSON(parentSpan!).parent_span_id).toBeUndefined(); + // eslint-disable-next-line deprecation/deprecation expect(parentSpan?.parentSpanId).toBeUndefined(); + + expect(spanToJSON(childSpan!).parent_span_id).toEqual(parentSpan?.spanContext().spanId); + // eslint-disable-next-line deprecation/deprecation expect(childSpan?.parentSpanId).toEqual(parentSpan?.spanContext().spanId); }); }); diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index b09fb87a2dab..e9f61c73c0f3 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -6,6 +6,7 @@ import { addTracingExtensions, getActiveTransaction, spanIsSampled, + spanToJSON, startIdleTransaction, } from '@sentry/core'; import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; @@ -396,7 +397,7 @@ export class BrowserTracing implements Integration { scope.setPropagationContext({ traceId: idleTransaction.spanContext().traceId, spanId: idleTransaction.spanContext().spanId, - parentSpanId: idleTransaction.parentSpanId, + parentSpanId: spanToJSON(idleTransaction).parent_span_id, sampled: spanIsSampled(idleTransaction), }); } diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index bed08b56b6ee..f4eb56c88194 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -187,6 +187,13 @@ export interface Span extends Omit { */ spanId: string; + /** + * Parent Span ID + * + * @deprecated Use `spanToJSON(span).parent_span_id` instead. + */ + parentSpanId?: string; + /** * The ID of the trace. * @deprecated Use `spanContext().traceId` instead. From 1cceeae30832667f402a88d70f5325caa3c44f7f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Jan 2024 15:02:12 +0100 Subject: [PATCH 06/16] feat(core): Deprecate `Span.origin` in favor of `sentry.origin` attribute (#10260) Deprecate the `Span.origin` field on the class and the interface. It will be replaced in v8 by the semantic `sentry.op` attribute. --- .../suites/public-api/startSpan/basic/test.ts | 6 +-- .../nextjs-app-dir/tests/middleware.test.ts | 1 + .../tests/propagation.test.ts | 4 ++ .../tests/transactions.test.ts | 4 ++ packages/core/src/semanticAttributes.ts | 5 ++ packages/core/src/tracing/span.ts | 31 ++++++++--- packages/core/test/lib/tracing/span.test.ts | 51 +++++++++++++++---- .../core/test/lib/utils/spanUtils.test.ts | 4 ++ .../test/client/tracingFetch.test.ts | 7 +-- .../test/integration/scope.test.ts | 2 +- .../test/integration/transactions.test.ts | 45 +++++++++++++--- .../test/spanprocessor.test.ts | 7 ++- .../test/custom/transaction.test.ts | 6 +++ .../test/integration/scope.test.ts | 5 +- .../test/integration/transactions.test.ts | 36 ++++++++++--- .../test/browser/request.test.ts | 1 + packages/tracing/test/hub.test.ts | 3 +- packages/tracing/test/span.test.ts | 22 +++++++- packages/tracing/test/transaction.test.ts | 16 ++++-- packages/types/src/span.ts | 9 +++- 20 files changed, 216 insertions(+), 49 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts index 9df0e24f3a38..2567f5c018d3 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts @@ -24,11 +24,11 @@ sentryTest('should report finished spans as children of the root transaction', a const url = await getLocalTestPath({ testDir: __dirname }); const transaction = await getFirstSentryEnvelopeRequest(page, url); - const rootSpanId = transaction?.contexts?.trace?.span_id; - expect(transaction.spans).toHaveLength(1); const span_1 = transaction.spans?.[0]; expect(span_1?.description).toBe('child_span'); - expect(span_1?.parent_span_id).toEqual(rootSpanId); + expect(span_1?.parent_span_id).toEqual(transaction?.contexts?.trace?.span_id); + expect(span_1?.origin).toEqual('manual'); + expect(span_1?.data?.['sentry.origin']).toEqual('manual'); }); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts index b2346db58450..69fe5a8670c8 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts @@ -69,6 +69,7 @@ test('Should trace outgoing fetch requests inside middleware and create breadcru type: 'fetch', url: 'http://localhost:3030/', 'sentry.op': 'http.client', + 'sentry.origin': 'auto.http.wintercg_fetch', }, description: 'GET http://localhost:3030/', op: 'http.client', diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts index 4c9310228eb3..9b83e882a739 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts @@ -64,6 +64,7 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', span_id: expect.any(String), @@ -87,6 +88,7 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', parent_span_id: outgoingHttpSpanId, @@ -159,6 +161,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', span_id: expect.any(String), @@ -182,6 +185,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', parent_span_id: outgoingHttpSpanId, diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts index e29637a87df8..d2ab9e3d664f 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts @@ -29,6 +29,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', span_id: expect.any(String), @@ -48,6 +49,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { 'fastify.type': 'request_handler', 'http.route': '/test-transaction', 'otel.kind': 'INTERNAL', + 'sentry.origin': 'auto.http.otel.fastify', }, description: 'request handler - fastify -> app-auto-0', parent_span_id: expect.any(String), @@ -61,6 +63,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { { data: { 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', }, description: 'test-span', parent_span_id: expect.any(String), @@ -74,6 +77,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { { data: { 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', }, description: 'child-span', parent_span_id: expect.any(String), diff --git a/packages/core/src/semanticAttributes.ts b/packages/core/src/semanticAttributes.ts index 884eb0e5e9ef..afd0d123090f 100644 --- a/packages/core/src/semanticAttributes.ts +++ b/packages/core/src/semanticAttributes.ts @@ -14,3 +14,8 @@ export const SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE = 'sentry.sample_rate'; * Use this attribute to represent the operation of a span. */ export const SEMANTIC_ATTRIBUTE_SENTRY_OP = 'sentry.op'; + +/** + * Use this attribute to represent the origin of a span. + */ +export const SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN = 'sentry.origin'; diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 6371435e2d58..8ddfddb3b353 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -16,7 +16,7 @@ import type { import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; -import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '../semanticAttributes'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes'; import { getRootSpan } from '../utils/getRootSpan'; import { TRACE_FLAG_NONE, @@ -100,11 +100,6 @@ export class Span implements SpanInterface { */ public instrumenter: Instrumenter; - /** - * The origin of the span, giving context about what created the span. - */ - public origin?: SpanOrigin; - protected _traceId: string; protected _spanId: string; protected _parentSpanId?: string; @@ -138,7 +133,9 @@ export class Span implements SpanInterface { this._attributes = spanContext.attributes ? { ...spanContext.attributes } : {}; // eslint-disable-next-line deprecation/deprecation this.instrumenter = spanContext.instrumenter || 'sentry'; - this.origin = spanContext.origin || 'manual'; + + this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanContext.origin || 'manual'); + // eslint-disable-next-line deprecation/deprecation this._name = spanContext.name || spanContext.description; @@ -346,6 +343,24 @@ export class Span implements SpanInterface { this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, op); } + /** + * The origin of the span, giving context about what created the span. + * + * @deprecated Use `spanToJSON().origin` to read the origin instead. + */ + public get origin(): SpanOrigin | undefined { + return this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined; + } + + /** + * The origin of the span, giving context about what created the span. + * + * @deprecated Use `startSpan()` functions to set the origin instead. + */ + public set origin(origin: SpanOrigin | undefined) { + this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, origin); + } + /* eslint-enable @typescript-eslint/member-ordering */ /** @inheritdoc */ @@ -611,7 +626,7 @@ export class Span implements SpanInterface { tags: Object.keys(this.tags).length > 0 ? this.tags : undefined, timestamp: this._endTime, trace_id: this._traceId, - origin: this.origin, + origin: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined, }); } diff --git a/packages/core/test/lib/tracing/span.test.ts b/packages/core/test/lib/tracing/span.test.ts index 2896ae494ad1..b3c08987d4b2 100644 --- a/packages/core/test/lib/tracing/span.test.ts +++ b/packages/core/test/lib/tracing/span.test.ts @@ -84,6 +84,8 @@ describe('span', () => { strArray: ['aa', 'bb'], boolArray: [true, false], arrayWithUndefined: [1, undefined, 2], + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', }); }); @@ -92,11 +94,12 @@ describe('span', () => { span.setAttribute('str', 'bar'); - expect(Object.keys(span['_attributes']).length).toEqual(1); + // length 2 because `sentry.origin` is always set by default + expect(Object.keys(span['_attributes']).length).toEqual(2); span.setAttribute('str', undefined); - expect(Object.keys(span['_attributes']).length).toEqual(0); + expect(Object.keys(span['_attributes']).length).toEqual(1); }); it('disallows invalid attribute types', () => { @@ -119,7 +122,10 @@ describe('span', () => { const initialAttributes = span['_attributes']; - expect(initialAttributes).toEqual({}); + expect(initialAttributes).toEqual({ + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); const newAttributes = { str: 'bar', @@ -145,6 +151,7 @@ describe('span', () => { strArray: ['aa', 'bb'], boolArray: [true, false], arrayWithUndefined: [1, undefined, 2], + 'sentry.origin': 'manual', }); expect(span['_attributes']).not.toBe(newAttributes); @@ -164,6 +171,7 @@ describe('span', () => { strArray: ['aa', 'bb'], boolArray: [true, false], arrayWithUndefined: [1, undefined, 2], + 'sentry.origin': 'manual', }); }); @@ -172,11 +180,12 @@ describe('span', () => { span.setAttribute('str', 'bar'); - expect(Object.keys(span['_attributes']).length).toEqual(1); + // length 2 because `sentry.origin` is always set by default + expect(Object.keys(span['_attributes']).length).toEqual(2); span.setAttributes({ str: undefined }); - expect(Object.keys(span['_attributes']).length).toEqual(0); + expect(Object.keys(span['_attributes']).length).toEqual(1); }); }); @@ -275,7 +284,10 @@ describe('span', () => { it('works without data & attributes', () => { const span = new Span(); - expect(span['_getData']()).toEqual(undefined); + expect(span['_getData']()).toEqual({ + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); }); it('works with data only', () => { @@ -283,16 +295,27 @@ describe('span', () => { // eslint-disable-next-line deprecation/deprecation span.setData('foo', 'bar'); - expect(span['_getData']()).toEqual({ foo: 'bar' }); - // eslint-disable-next-line deprecation/deprecation - expect(span['_getData']()).toBe(span.data); + expect(span['_getData']()).toEqual({ + foo: 'bar', + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); + expect(span['_getData']()).toStrictEqual({ + // eslint-disable-next-line deprecation/deprecation + ...span.data, + 'sentry.origin': 'manual', + }); }); it('works with attributes only', () => { const span = new Span(); span.setAttribute('foo', 'bar'); - expect(span['_getData']()).toEqual({ foo: 'bar' }); + expect(span['_getData']()).toEqual({ + foo: 'bar', + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); // eslint-disable-next-line deprecation/deprecation expect(span['_getData']()).toBe(span.attributes); }); @@ -306,7 +329,13 @@ describe('span', () => { // eslint-disable-next-line deprecation/deprecation span.setData('baz', 'baz'); - expect(span['_getData']()).toEqual({ foo: 'foo', bar: 'bar', baz: 'baz' }); + expect(span['_getData']()).toEqual({ + foo: 'foo', + bar: 'bar', + baz: 'baz', + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); // eslint-disable-next-line deprecation/deprecation expect(span['_getData']()).not.toBe(span.attributes); // eslint-disable-next-line deprecation/deprecation diff --git a/packages/core/test/lib/utils/spanUtils.test.ts b/packages/core/test/lib/utils/spanUtils.test.ts index 5699fa391c2b..0afb59530623 100644 --- a/packages/core/test/lib/utils/spanUtils.test.ts +++ b/packages/core/test/lib/utils/spanUtils.test.ts @@ -55,6 +55,9 @@ describe('spanToJSON', () => { trace_id: span.spanContext().traceId, origin: 'manual', start_timestamp: span['_startTime'], + data: { + 'sentry.origin': 'manual', + }, }); }); @@ -89,6 +92,7 @@ describe('spanToJSON', () => { timestamp: 456, data: { 'sentry.op': 'test op', + 'sentry.origin': 'auto', }, }); }); diff --git a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts index 579ccf4ca547..8517b4ab0fce 100644 --- a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts +++ b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts @@ -1,5 +1,5 @@ import { expect, test } from '@playwright/test'; -import { Transaction } from '@sentry/types'; +import { SerializedEvent } from '@sentry/types'; import { countEnvelopes, getMultipleSentryEnvelopeRequests } from './utils/helpers'; test('should correctly instrument `fetch` for performance tracing', async ({ page }) => { @@ -12,7 +12,7 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag }); }); - const transaction = await getMultipleSentryEnvelopeRequests(page, 1, { + const transaction = await getMultipleSentryEnvelopeRequests(page, 1, { url: '/fetch', envelopeType: 'transaction', }); @@ -27,7 +27,6 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag }, }); - // @ts-expect-error - We know that `spans` is inside Transaction envelopes expect(transaction[0].spans).toEqual( expect.arrayContaining([ expect.objectContaining({ @@ -38,6 +37,7 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag 'http.response_content_length': expect.any(Number), 'http.response.status_code': 200, 'sentry.op': 'http.client', + 'sentry.origin': 'auto.http.browser', }, description: 'GET http://example.com', op: 'http.client', @@ -47,6 +47,7 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag timestamp: expect.any(Number), trace_id: expect.any(String), status: expect.any(String), + origin: 'auto.http.browser', }), ]), ); diff --git a/packages/node-experimental/test/integration/scope.test.ts b/packages/node-experimental/test/integration/scope.test.ts index d1170dd636e8..6dfcad34ff81 100644 --- a/packages/node-experimental/test/integration/scope.test.ts +++ b/packages/node-experimental/test/integration/scope.test.ts @@ -88,7 +88,7 @@ describe('Integration | Scope', () => { expect.objectContaining({ contexts: expect.objectContaining({ trace: { - data: { 'otel.kind': 'INTERNAL' }, + data: { 'otel.kind': 'INTERNAL', 'sentry.origin': 'manual' }, span_id: spanId, status: 'ok', trace_id: traceId, diff --git a/packages/node-experimental/test/integration/transactions.test.ts b/packages/node-experimental/test/integration/transactions.test.ts index 8a9464c1fd03..b1038a53a276 100644 --- a/packages/node-experimental/test/integration/transactions.test.ts +++ b/packages/node-experimental/test/integration/transactions.test.ts @@ -84,7 +84,11 @@ describe('Integration | Transactions', () => { }, runtime: { name: 'node', version: expect.any(String) }, trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), status: 'ok', @@ -148,7 +152,10 @@ describe('Integration | Transactions', () => { // This is the same behavior as for the "regular" SDKs expect(spans.map(span => spanToJSON(span))).toEqual([ { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 1', origin: 'manual', parent_span_id: expect.any(String), @@ -159,7 +166,11 @@ describe('Integration | Transactions', () => { trace_id: expect.any(String), }, { - data: { 'otel.kind': 'INTERNAL', 'test.inner': 'test value' }, + data: { + 'otel.kind': 'INTERNAL', + 'test.inner': 'test value', + 'sentry.origin': 'manual', + }, description: 'inner span 2', origin: 'manual', parent_span_id: expect.any(String), @@ -244,7 +255,11 @@ describe('Integration | Transactions', () => { }, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), status: 'ok', @@ -286,7 +301,11 @@ describe('Integration | Transactions', () => { }, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op b' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op b', + 'sentry.origin': 'manual', + }, op: 'test op b', span_id: expect.any(String), status: 'ok', @@ -363,7 +382,11 @@ describe('Integration | Transactions', () => { attributes: {}, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), parent_span_id: parentSpanId, @@ -401,7 +424,10 @@ describe('Integration | Transactions', () => { // This is the same behavior as for the "regular" SDKs expect(spans.map(span => spanToJSON(span))).toEqual([ { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 1', origin: 'manual', parent_span_id: expect.any(String), @@ -412,7 +438,10 @@ describe('Integration | Transactions', () => { trace_id: traceId, }, { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 2', origin: 'manual', parent_span_id: expect.any(String), diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index 23818b17442e..0bd7d852f7a5 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -344,7 +344,8 @@ describe('SentrySpanProcessor', () => { const sentrySpan = getSpanForOtelSpan(child); - expect(spanToJSON(sentrySpan!).data).toEqual(undefined); + // origin is set by default to 'manual' + expect(spanToJSON(sentrySpan!).data).toEqual({ 'sentry.origin': 'manual' }); child.end(); @@ -354,6 +355,7 @@ describe('SentrySpanProcessor', () => { 'test-attribute-2': [1, 2, 3], 'test-attribute-3': 0, 'test-attribute-4': false, + 'sentry.origin': 'manual', }); }); @@ -592,6 +594,7 @@ describe('SentrySpanProcessor', () => { 'otel.kind': 'INTERNAL', url: 'http://example.com/my/route/123', 'sentry.op': 'http', + 'sentry.origin': 'manual', }); parentOtelSpan.end(); @@ -622,6 +625,7 @@ describe('SentrySpanProcessor', () => { 'otel.kind': 'INTERNAL', url: 'http://example.com/my/route/123', 'sentry.op': 'http', + 'sentry.origin': 'manual', }); expect(op).toBe('http'); @@ -655,6 +659,7 @@ describe('SentrySpanProcessor', () => { 'http.query': '?what=123', 'http.fragment': '#myHash', 'sentry.op': 'http', + 'sentry.origin': 'manual', }); expect(op).toBe('http'); diff --git a/packages/opentelemetry/test/custom/transaction.test.ts b/packages/opentelemetry/test/custom/transaction.test.ts index b949ee82d91d..cb0cd2efd332 100644 --- a/packages/opentelemetry/test/custom/transaction.test.ts +++ b/packages/opentelemetry/test/custom/transaction.test.ts @@ -28,6 +28,9 @@ describe('NodeExperimentalTransaction', () => { expect.objectContaining({ contexts: { trace: { + data: { + 'sentry.origin': 'manual', + }, span_id: expect.any(String), trace_id: expect.any(String), origin: 'manual', @@ -109,6 +112,9 @@ describe('NodeExperimentalTransaction', () => { expect.objectContaining({ contexts: { trace: { + data: { + 'sentry.origin': 'manual', + }, span_id: expect.any(String), trace_id: expect.any(String), origin: 'manual', diff --git a/packages/opentelemetry/test/integration/scope.test.ts b/packages/opentelemetry/test/integration/scope.test.ts index f313dcba352e..6ab7c6d35621 100644 --- a/packages/opentelemetry/test/integration/scope.test.ts +++ b/packages/opentelemetry/test/integration/scope.test.ts @@ -94,7 +94,10 @@ describe('Integration | Scope', () => { expect.objectContaining({ contexts: expect.objectContaining({ trace: { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, span_id: spanId, status: 'ok', trace_id: traceId, diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index cf172d048b59..a87816691a45 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -83,7 +83,11 @@ describe('Integration | Transactions', () => { }, }, trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), status: 'ok', @@ -146,6 +150,7 @@ describe('Integration | Transactions', () => { { data: { 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', }, description: 'inner span 1', origin: 'manual', @@ -157,7 +162,11 @@ describe('Integration | Transactions', () => { trace_id: expect.any(String), }, { - data: { 'otel.kind': 'INTERNAL', 'test.inner': 'test value' }, + data: { + 'otel.kind': 'INTERNAL', + 'test.inner': 'test value', + 'sentry.origin': 'manual', + }, description: 'inner span 2', origin: 'manual', parent_span_id: expect.any(String), @@ -238,7 +247,11 @@ describe('Integration | Transactions', () => { }, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), status: 'ok', @@ -280,7 +293,11 @@ describe('Integration | Transactions', () => { }, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op b' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op b', + 'sentry.origin': 'manual', + }, op: 'test op b', span_id: expect.any(String), status: 'ok', @@ -360,6 +377,7 @@ describe('Integration | Transactions', () => { data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', }, op: 'test op', span_id: expect.any(String), @@ -398,7 +416,10 @@ describe('Integration | Transactions', () => { // This is the same behavior as for the "regular" SDKs expect(spans.map(span => spanToJSON(span))).toEqual([ { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 1', origin: 'manual', parent_span_id: expect.any(String), @@ -409,7 +430,10 @@ describe('Integration | Transactions', () => { trace_id: traceId, }, { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 2', origin: 'manual', parent_span_id: expect.any(String), diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index 6641be671bfc..782c0890e156 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -264,6 +264,7 @@ describe('callbacks', () => { type: 'fetch', url: 'http://dogs.are.great/', 'sentry.op': 'http.client', + 'sentry.origin': 'auto.http.browser', }); expect(finishedSpan.op).toBe('http.client'); }); diff --git a/packages/tracing/test/hub.test.ts b/packages/tracing/test/hub.test.ts index a7fbf5e86b62..7b73d9bb2d51 100644 --- a/packages/tracing/test/hub.test.ts +++ b/packages/tracing/test/hub.test.ts @@ -305,7 +305,8 @@ describe('Hub', () => { makeMain(hub); hub.startTransaction({ name: 'dogpark', parentSampled: true }); - expect(Transaction.prototype.setAttribute).toHaveBeenCalledTimes(0); + // length 1 because `sentry.origin` is set on span initialization + expect(Transaction.prototype.setAttribute).toHaveBeenCalledTimes(1); }); it('should record sampling method and rate when sampling decision comes from traceSampleRate', () => { diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 1cf33c7c435f..4a7e1537f394 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -173,6 +173,9 @@ describe('Span', () => { span_id: 'd', trace_id: 'c', origin: 'manual', + data: { + 'sentry.origin': 'manual', + }, }); }); }); @@ -474,6 +477,9 @@ describe('Span', () => { expect(context).toStrictEqual({ span_id: 'd', trace_id: 'c', + data: { + 'sentry.origin': 'manual', + }, origin: 'manual', }); }); @@ -481,7 +487,13 @@ describe('Span', () => { describe('toContext and updateWithContext', () => { test('toContext should return correct context', () => { - const originalContext = { traceId: 'a', spanId: 'b', sampled: false, description: 'test', op: 'op' }; + const originalContext = { + traceId: 'a', + spanId: 'b', + sampled: false, + description: 'test', + op: 'op', + }; const span = new Span(originalContext); const newContext = span.toContext(); @@ -494,6 +506,7 @@ describe('Span', () => { traceId: expect.any(String), data: { 'sentry.op': 'op', + 'sentry.origin': 'manual', }, }); }); @@ -562,7 +575,12 @@ describe('Span', () => { expect(span.op).toBe('new-op'); expect(span.sampled).toBe(true); expect(span.tags).toStrictEqual({ tag1: 'bye' }); - expect(span.data).toStrictEqual({ data0: 'foo', data1: 'bar', 'sentry.op': 'op' }); + expect(span.data).toStrictEqual({ + data0: 'foo', + data1: 'bar', + 'sentry.op': 'op', + 'sentry.origin': 'manual', + }); }); }); diff --git a/packages/tracing/test/transaction.test.ts b/packages/tracing/test/transaction.test.ts index 3d048c9a3c3f..f9e950dd9cb9 100644 --- a/packages/tracing/test/transaction.test.ts +++ b/packages/tracing/test/transaction.test.ts @@ -1,6 +1,10 @@ /* eslint-disable deprecation/deprecation */ import { BrowserClient, Hub } from '@sentry/browser'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/core'; import { Transaction, addExtensionMethods } from '../src'; import { getDefaultBrowserClientOptions } from './testutils'; @@ -163,7 +167,10 @@ describe('`Transaction` class', () => { contexts: { foo: { key: 'val' }, trace: { - data: { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1 }, + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', + }, span_id: transaction.spanId, trace_id: transaction.traceId, origin: 'manual', @@ -191,7 +198,10 @@ describe('`Transaction` class', () => { expect.objectContaining({ contexts: { trace: { - data: { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1 }, + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', + }, span_id: transaction.spanId, trace_id: transaction.traceId, origin: 'manual', diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index f4eb56c88194..e34940b35d65 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -166,7 +166,7 @@ export interface SpanContext { } /** Span holding trace_id, span_id */ -export interface Span extends Omit { +export interface Span extends Omit { /** * Human-readable identifier for the span. Identical to span.description. * @deprecated Use `spanToJSON(span).description` instead. @@ -258,6 +258,13 @@ export interface Span extends Omit { */ status?: string; + /** + * The origin of the span, giving context about what created the span. + * + * @deprecated Use `startSpan` function to set and `spanToJSON(span).origin` to read the origin instead. + */ + origin?: SpanOrigin; + /** * Get context data for this span. * This includes the spanId & the traceId. From 5f0b50624df7b21e95d6c825fb75ca544163d3de Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 19 Jan 2024 09:15:31 -0500 Subject: [PATCH 07/16] ref(wasm): Convert wasm integration to functional integration (#10230) We should think about moving the wasm integration into `@sentry/browser` and removing `@sentry/wasm` all together. What do you think? --- MIGRATION.md | 15 +++--- packages/wasm/src/index.ts | 107 +++++++++++++++++++------------------ 2 files changed, 64 insertions(+), 58 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 0d27fa476ffd..3ec81623c175 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -28,13 +28,14 @@ integrations from the `Integrations.XXX` hash, is deprecated in favor of using t The following list shows how integrations should be migrated: -| Old | New | -| ------------------------ | ------------------------------- | -| `new InboundFilters()` | `inboundFiltersIntegrations()` | -| `new FunctionToString()` | `functionToStringIntegration()` | -| `new LinkedErrors()` | `linkedErrorsIntegration()` | -| `new ModuleMetadata()` | `moduleMetadataIntegration()` | -| `new RequestData()` | `requestDataIntegration()` | +| 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` | ## Deprecate `hub.bindClient()` and `makeMain()` diff --git a/packages/wasm/src/index.ts b/packages/wasm/src/index.ts index 49487167e346..e84d4c880bd5 100644 --- a/packages/wasm/src/index.ts +++ b/packages/wasm/src/index.ts @@ -1,9 +1,63 @@ -import type { Event, Integration, StackFrame } from '@sentry/types'; +import { convertIntegrationFnToClass, defineIntegration } from '@sentry/core'; +import type { Event, Integration, IntegrationClass, IntegrationFn, StackFrame } from '@sentry/types'; import { patchWebAssembly } from './patchWebAssembly'; import { getImage, getImages } from './registry'; -/** plz don't */ +const INTEGRATION_NAME = 'Wasm'; + +const _wasmIntegration = (() => { + return { + name: INTEGRATION_NAME, + setupOnce() { + patchWebAssembly(); + }, + processEvent(event: Event): Event { + let haveWasm = false; + + if (event.exception && event.exception.values) { + event.exception.values.forEach(exception => { + if (exception.stacktrace && exception.stacktrace.frames) { + haveWasm = haveWasm || patchFrames(exception.stacktrace.frames); + } + }); + } + + if (haveWasm) { + event.debug_meta = event.debug_meta || {}; + event.debug_meta.images = [...(event.debug_meta.images || []), ...getImages()]; + } + + return event; + }, + }; +}) satisfies IntegrationFn; + +export const wasmIntegration = defineIntegration(_wasmIntegration); + +/** + * Process WASM stack traces to support server-side symbolication. + * + * This also hooks the WebAssembly loading browser API so that module + * registrations are intercepted. + * + * @deprecated Use `wasmIntegration` export instead + * + * import { wasmIntegration } from '@sentry/wasm'; + * + * ``` + * Sentry.init({ integrations: [wasmIntegration()] }); + * ``` + */ +// eslint-disable-next-line deprecation/deprecation +export const Wasm = convertIntegrationFnToClass(INTEGRATION_NAME, wasmIntegration) as IntegrationClass< + Integration & { processEvent: (event: Event) => Event } +>; + +/** + * Patches a list of stackframes with wasm data needed for server-side symbolication + * if applicable. Returns true if any frames were patched. + */ function patchFrames(frames: Array): boolean { let haveWasm = false; frames.forEach(frame => { @@ -24,52 +78,3 @@ function patchFrames(frames: Array): boolean { }); return haveWasm; } - -/** - * Process WASM stack traces to support server-side symbolication. - * - * This also hooks the WebAssembly loading browser API so that module - * registraitons are intercepted. - */ -export class Wasm implements Integration { - /** - * @inheritDoc - */ - public static id: string = 'Wasm'; - - /** - * @inheritDoc - */ - public name: string; - - public constructor() { - this.name = Wasm.id; - } - - /** - * @inheritDoc - */ - public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { - patchWebAssembly(); - } - - /** @inheritDoc */ - public processEvent(event: Event): Event { - let haveWasm = false; - - if (event.exception && event.exception.values) { - event.exception.values.forEach(exception => { - if (exception?.stacktrace?.frames) { - haveWasm = haveWasm || patchFrames(exception.stacktrace.frames); - } - }); - } - - if (haveWasm) { - event.debug_meta = event.debug_meta || {}; - event.debug_meta.images = [...(event.debug_meta.images || []), ...getImages()]; - } - - return event; - } -} From 7e3207de0634a00d458a4249d6b69542d5583c03 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 22 Jan 2024 14:19:08 +0100 Subject: [PATCH 08/16] fix(wasm): Add `@sentry/core` as a dependency (#10283) By using functional integrations in #10230, we started importing from `@sentry/core` in the WASM integration. However, we didn't register `@sentry/core` as a dependency, making rollup bundle core into the package output. This changed the `build/npm` directory structure, making our entry points in `package.json` invalid. This PR fixes things by simply registering core as a dependency of wasm. If we move WASM to core (which I strongly think we should do), we'll be able to get rid of this again. --- .github/workflows/build.yml | 1 + packages/wasm/package.json | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 088a5b19404b..d66831462844 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -102,6 +102,7 @@ jobs: - 'packages/replay/**' - 'packages/replay-canvas/**' - 'packages/feedback/**' + - 'packages/wasm/**' browser_integration: - *shared - *browser diff --git a/packages/wasm/package.json b/packages/wasm/package.json index 899141b4f6a7..6cea967b1583 100644 --- a/packages/wasm/package.json +++ b/packages/wasm/package.json @@ -31,7 +31,8 @@ "dependencies": { "@sentry/browser": "7.94.1", "@sentry/types": "7.94.1", - "@sentry/utils": "7.94.1" + "@sentry/utils": "7.94.1", + "@sentry/core": "7.94.1" }, "scripts": { "build": "run-p build:transpile build:bundle build:types", From a27decab3a4fa146f061c79c74ce045013ebfeff Mon Sep 17 00:00:00 2001 From: Oleh Aloshkin Date: Tue, 23 Jan 2024 09:12:05 +0100 Subject: [PATCH 09/16] feat: Make `parameterize` function available through browser and node API (#10085) Move `parameterize` function to core, export it to upstream packages via browser and node. --- .../captureMessage/parameterized_message/subject.js | 4 +--- .../public-api/captureMessage/parameterized_message/test.ts | 6 ------ .../captureMessage/parameterized_message/scenario.ts | 3 +-- packages/astro/src/index.server.ts | 1 + packages/browser/src/exports.ts | 1 + packages/bun/src/index.ts | 1 + packages/core/src/index.ts | 1 + packages/{utils/src => core/src/utils}/parameterize.ts | 4 ++-- .../test => core/test/lib/utils}/parameterize.test.ts | 2 +- packages/node/src/index.ts | 1 + packages/remix/src/index.server.ts | 1 + packages/serverless/src/index.ts | 1 + packages/sveltekit/src/server/index.ts | 1 + packages/utils/src/index.ts | 1 - 14 files changed, 13 insertions(+), 15 deletions(-) rename packages/{utils/src => core/src/utils}/parameterize.ts (86%) rename packages/{utils/test => core/test/lib/utils}/parameterize.test.ts (94%) diff --git a/dev-packages/browser-integration-tests/suites/public-api/captureMessage/parameterized_message/subject.js b/dev-packages/browser-integration-tests/suites/public-api/captureMessage/parameterized_message/subject.js index a86616cd52dc..3553e4dcd9fd 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/captureMessage/parameterized_message/subject.js +++ b/dev-packages/browser-integration-tests/suites/public-api/captureMessage/parameterized_message/subject.js @@ -1,6 +1,4 @@ -import { parameterize } from '@sentry/utils'; - const x = 'first'; const y = 'second'; -Sentry.captureMessage(parameterize`This is a log statement with ${x} and ${y} params`); +Sentry.captureMessage(Sentry.parameterize`This is a log statement with ${x} and ${y} params`); diff --git a/dev-packages/browser-integration-tests/suites/public-api/captureMessage/parameterized_message/test.ts b/dev-packages/browser-integration-tests/suites/public-api/captureMessage/parameterized_message/test.ts index 4c948d439bff..db84460b85b5 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/captureMessage/parameterized_message/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/captureMessage/parameterized_message/test.ts @@ -5,12 +5,6 @@ import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; sentryTest('should capture a parameterized representation of the message', async ({ getLocalTestPath, page }) => { - const bundle = process.env.PW_BUNDLE; - - if (bundle && bundle.startsWith('bundle_')) { - sentryTest.skip(); - } - const url = await getLocalTestPath({ testDir: __dirname }); const eventData = await getFirstSentryEnvelopeRequest(page, url); diff --git a/dev-packages/node-integration-tests/suites/public-api/captureMessage/parameterized_message/scenario.ts b/dev-packages/node-integration-tests/suites/public-api/captureMessage/parameterized_message/scenario.ts index db81bb18d331..1d92f9dcd769 100644 --- a/dev-packages/node-integration-tests/suites/public-api/captureMessage/parameterized_message/scenario.ts +++ b/dev-packages/node-integration-tests/suites/public-api/captureMessage/parameterized_message/scenario.ts @@ -1,5 +1,4 @@ import * as Sentry from '@sentry/node'; -import { parameterize } from '@sentry/utils'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', @@ -9,4 +8,4 @@ Sentry.init({ const x = 'first'; const y = 'second'; -Sentry.captureMessage(parameterize`This is a log statement with ${x} and ${y} params`); +Sentry.captureMessage(Sentry.parameterize`This is a log statement with ${x} and ${y} params`); diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 61e2bf2f6c98..3074a061a2d8 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -77,6 +77,7 @@ export { startSpanManual, continueTrace, cron, + parameterize, } from '@sentry/node'; // We can still leave this for the carrier init and type exports diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 41338b4047df..50a282cd1406 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -72,6 +72,7 @@ export { metrics, functionToStringIntegration, inboundFiltersIntegration, + parameterize, } from '@sentry/core'; export { WINDOW } from './helpers'; diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index 725eecb24753..5d5abce58f48 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -83,6 +83,7 @@ export { inboundFiltersIntegration, linkedErrorsIntegration, requestDataIntegration, + parameterize, } from '@sentry/core'; export type { SpanStatusType } from '@sentry/core'; export { autoDiscoverNodePerformanceMonitoringIntegrations, cron } from '@sentry/node'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 3eec5fe4ca0a..f2f33a884123 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -80,6 +80,7 @@ export { createCheckInEnvelope } from './checkin'; export { hasTracingEnabled } from './utils/hasTracingEnabled'; export { isSentryRequestUrl } from './utils/isSentryRequestUrl'; export { handleCallbackErrors } from './utils/handleCallbackErrors'; +export { parameterize } from './utils/parameterize'; export { spanToTraceHeader, spanToJSON, diff --git a/packages/utils/src/parameterize.ts b/packages/core/src/utils/parameterize.ts similarity index 86% rename from packages/utils/src/parameterize.ts rename to packages/core/src/utils/parameterize.ts index 2cfa63e92677..5783a61fa3c9 100644 --- a/packages/utils/src/parameterize.ts +++ b/packages/core/src/utils/parameterize.ts @@ -3,8 +3,8 @@ import type { ParameterizedString } from '@sentry/types'; /** * Tagged template function which returns paramaterized representation of the message * For example: parameterize`This is a log statement with ${x} and ${y} params`, would return: - * "__sentry_template_string__": "My raw message with interpreted strings like %s", - * "__sentry_template_values__": ["this"] + * "__sentry_template_string__": 'This is a log statement with %s and %s params', + * "__sentry_template_values__": ['first', 'second'] * @param strings An array of string values splitted between expressions * @param values Expressions extracted from template string * @returns String with template information in __sentry_template_string__ and __sentry_template_values__ properties diff --git a/packages/utils/test/parameterize.test.ts b/packages/core/test/lib/utils/parameterize.test.ts similarity index 94% rename from packages/utils/test/parameterize.test.ts rename to packages/core/test/lib/utils/parameterize.test.ts index a199e0939271..413ba8043c49 100644 --- a/packages/utils/test/parameterize.test.ts +++ b/packages/core/test/lib/utils/parameterize.test.ts @@ -1,6 +1,6 @@ import type { ParameterizedString } from '@sentry/types'; -import { parameterize } from '../src/parameterize'; +import { parameterize } from '../../../src/utils/parameterize'; describe('parameterize()', () => { test('works with empty string', () => { diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index a48bc13a6d4b..86ee3e78b961 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -79,6 +79,7 @@ export { startInactiveSpan, startSpanManual, continueTrace, + parameterize, metrics, functionToStringIntegration, inboundFiltersIntegration, diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index 240a7d3787e1..1cd04720b4dc 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -68,6 +68,7 @@ export { Integrations, Handlers, cron, + parameterize, } from '@sentry/node'; // Keeping the `*` exports for backwards compatibility and types diff --git a/packages/serverless/src/index.ts b/packages/serverless/src/index.ts index 5edbde50fa6c..3fe65ee74ea1 100644 --- a/packages/serverless/src/index.ts +++ b/packages/serverless/src/index.ts @@ -76,4 +76,5 @@ export { startInactiveSpan, startSpanManual, continueTrace, + parameterize, } from '@sentry/node'; diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 87bd11c8b7eb..91fb9e2d7771 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -74,6 +74,7 @@ export { startSpanManual, continueTrace, cron, + parameterize, } from '@sentry/node'; // We can still leave this for the carrier init and type exports diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 5483f2aa7e41..d19991b7d401 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -32,7 +32,6 @@ export * from './url'; export * from './userIntegrations'; export * from './cache'; export * from './eventbuilder'; -export * from './parameterize'; export * from './anr'; export * from './lru'; export * from './buildPolyfills'; From 6868256df420fee152a8e8fd4b94f5c2aa425f4e Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Tue, 23 Jan 2024 04:47:41 -0500 Subject: [PATCH 10/16] fix(replay-canvas): Add missing dependency on @sentry/utils (#10279) Co-authored-by: Luca Forstner --- packages/replay-canvas/package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/replay-canvas/package.json b/packages/replay-canvas/package.json index 486f4880eeef..97661b93383d 100644 --- a/packages/replay-canvas/package.json +++ b/packages/replay-canvas/package.json @@ -61,7 +61,8 @@ "dependencies": { "@sentry/core": "7.94.1", "@sentry/replay": "7.94.1", - "@sentry/types": "7.94.1" + "@sentry/types": "7.94.1", + "@sentry/utils": "7.94.1" }, "engines": { "node": ">=12" From a68253420fed41d5195d6fed2a640ff98274817f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 23 Jan 2024 15:06:14 +0100 Subject: [PATCH 11/16] feat(replay): Deprecate Replay, ReplayCanvas, Feedback classes (#10270) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead, users should use the new functional styles. Note that we'll probably actually un-deprecate `Replay` in some form in v8, as we'll be keeping the class around there for sure (as there is a lot of logic in there...). But users should not use it, so deprecating this now! While at it, I also deprecated the old `InitSentryForEmber` method in favor of `init()`. It's slightly unfortunate that I missed this, but we probably shouldn't have exposed `ReplayCanvas` as a class anymore at all 😬 maybe we wait before we document this etc. until we merged the functional style. cc @billyvg --- MIGRATION.md | 3 +++ .../suites/replay/bufferMode/test.ts | 6 ++++++ .../suites/replay/dsc/test.ts | 6 +++++- packages/browser/src/index.bundle.feedback.ts | 16 +++++++++++--- packages/browser/src/index.bundle.replay.ts | 21 ++++++++++++++++--- .../index.bundle.tracing.replay.feedback.ts | 17 ++++++++++++--- .../src/index.bundle.tracing.replay.ts | 17 ++++++++++++--- packages/browser/src/index.bundle.tracing.ts | 15 +++++++++++-- packages/browser/src/index.bundle.ts | 21 +++++++++++++++++-- packages/browser/src/index.ts | 19 ++++++++++++++--- .../test/unit/index.bundle.feedback.test.ts | 11 ++++++++-- .../test/unit/index.bundle.replay.test.ts | 11 ++++++++-- .../browser/test/unit/index.bundle.test.ts | 5 +++++ ...dex.bundle.tracing.replay.feedback.test.ts | 5 ++++- .../unit/index.bundle.tracing.replay.test.ts | 10 +++++++-- .../test/unit/index.bundle.tracing.test.ts | 10 ++++++++- packages/ember/addon/index.ts | 11 +++++++--- .../tests/acceptance/sentry-replay-test.ts | 7 ++++--- .../ember/tests/dummy/app/routes/replay.ts | 4 ++-- packages/feedback/src/index.ts | 6 +++++- packages/feedback/src/integration.ts | 11 ++++++++-- packages/feedback/test/integration.test.ts | 14 ++++++------- packages/integration-shims/src/Feedback.ts | 14 +++++++++++++ packages/integration-shims/src/Replay.ts | 14 +++++++++++++ packages/integration-shims/src/index.ts | 12 +++++++++-- packages/replay-canvas/src/canvas.ts | 17 +++++++++------ packages/replay-canvas/src/index.ts | 6 +++++- packages/replay-canvas/test/canvas.test.ts | 6 +++--- packages/replay/src/index.ts | 6 +++++- packages/replay/src/integration.ts | 9 +++++++- .../beforeAddRecordingEvent.test.ts | 1 + .../coreHandlers/handleGlobalEvent.test.ts | 4 +--- packages/replay/test/integration/stop.test.ts | 1 + packages/replay/test/mocks/mockSdk.ts | 2 ++ packages/replay/test/mocks/resetSdkMock.ts | 1 + .../test/unit/multipleInstances.test.ts | 6 +++--- 36 files changed, 279 insertions(+), 66 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 3ec81623c175..88b27b25bc16 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -36,6 +36,9 @@ The following list shows how integrations should be migrated: | `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` | ## Deprecate `hub.bindClient()` and `makeMain()` diff --git a/dev-packages/browser-integration-tests/suites/replay/bufferMode/test.ts b/dev-packages/browser-integration-tests/suites/replay/bufferMode/test.ts index c4da79faa1d9..1ff15a3d3ff2 100644 --- a/dev-packages/browser-integration-tests/suites/replay/bufferMode/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/bufferMode/test.ts @@ -67,6 +67,7 @@ sentryTest( // Start buffering and assert that it is enabled expect( await page.evaluate(() => { + // eslint-disable-next-line deprecation/deprecation const replayIntegration = (window as unknown as Window & { Replay: InstanceType }).Replay; // @ts-expect-error private const replay = replayIntegration._replay; @@ -87,6 +88,7 @@ sentryTest( const [req0] = await Promise.all([ reqPromise0, page.evaluate(async () => { + // eslint-disable-next-line deprecation/deprecation const replayIntegration = (window as unknown as Window & { Replay: Replay }).Replay; await replayIntegration.flush(); }), @@ -210,6 +212,7 @@ sentryTest( // Start buffering and assert that it is enabled expect( await page.evaluate(() => { + // eslint-disable-next-line deprecation/deprecation const replayIntegration = (window as unknown as Window & { Replay: InstanceType }).Replay; // @ts-expect-error private const replay = replayIntegration._replay; @@ -230,6 +233,7 @@ sentryTest( const [req0] = await Promise.all([ reqPromise0, page.evaluate(async () => { + // eslint-disable-next-line deprecation/deprecation const replayIntegration = (window as unknown as Window & { Replay: Replay }).Replay; await replayIntegration.flush({ continueRecording: false }); }), @@ -324,6 +328,7 @@ sentryTest( // Start buffering and assert that it is enabled expect( await page.evaluate(() => { + // eslint-disable-next-line deprecation/deprecation const replayIntegration = (window as unknown as Window & { Replay: InstanceType }).Replay; const replay = replayIntegration['_replay']; replayIntegration.startBuffering(); @@ -342,6 +347,7 @@ sentryTest( expect(errorEvent0.tags?.replayId).toBeUndefined(); await page.evaluate(async () => { + // eslint-disable-next-line deprecation/deprecation const replayIntegration = (window as unknown as Window & { Replay: Replay }).Replay; replayIntegration['_replay'].getOptions().errorSampleRate = 1.0; }); diff --git a/dev-packages/browser-integration-tests/suites/replay/dsc/test.ts b/dev-packages/browser-integration-tests/suites/replay/dsc/test.ts index 63c103e51259..0588e165604e 100644 --- a/dev-packages/browser-integration-tests/suites/replay/dsc/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/dsc/test.ts @@ -6,7 +6,11 @@ import { sentryTest } from '../../../utils/fixtures'; import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../utils/helpers'; import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRunning } from '../../../utils/replayHelpers'; -type TestWindow = Window & { Sentry: typeof Sentry; Replay: Sentry.Replay }; +type TestWindow = Window & { + Sentry: typeof Sentry; + // eslint-disable-next-line deprecation/deprecation + Replay: Sentry.Replay; +}; sentryTest( 'should add replay_id to dsc of transactions when in session mode', diff --git a/packages/browser/src/index.bundle.feedback.ts b/packages/browser/src/index.bundle.feedback.ts index 7aab6485d254..5d3612106286 100644 --- a/packages/browser/src/index.bundle.feedback.ts +++ b/packages/browser/src/index.bundle.feedback.ts @@ -1,14 +1,24 @@ // This is exported so the loader does not fail when switching off Replay/Tracing -import { Feedback } from '@sentry-internal/feedback'; -import { BrowserTracing, Replay, addTracingExtensions } from '@sentry-internal/integration-shims'; +import { Feedback, feedbackIntegration } from '@sentry-internal/feedback'; +import { BrowserTracing, Replay, addTracingExtensions, replayIntegration } from '@sentry-internal/integration-shims'; import * as Sentry from './index.bundle.base'; // TODO (v8): Remove this as it was only needed for backwards compatibility +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; -export { BrowserTracing, addTracingExtensions, Replay, Feedback }; +export { + BrowserTracing, + addTracingExtensions, + // eslint-disable-next-line deprecation/deprecation + Replay, + replayIntegration, + // eslint-disable-next-line deprecation/deprecation + Feedback, + feedbackIntegration, +}; // Note: We do not export a shim for `Span` here, as that is quite complex and would blow up the bundle diff --git a/packages/browser/src/index.bundle.replay.ts b/packages/browser/src/index.bundle.replay.ts index 0b556bc71b8e..2609e7d9b48c 100644 --- a/packages/browser/src/index.bundle.replay.ts +++ b/packages/browser/src/index.bundle.replay.ts @@ -1,14 +1,29 @@ // This is exported so the loader does not fail when switching off Replay/Tracing -import { BrowserTracing, Feedback, addTracingExtensions } from '@sentry-internal/integration-shims'; -import { Replay } from '@sentry/replay'; +import { + BrowserTracing, + Feedback, + addTracingExtensions, + feedbackIntegration, +} from '@sentry-internal/integration-shims'; +import { Replay, replayIntegration } from '@sentry/replay'; import * as Sentry from './index.bundle.base'; // TODO (v8): Remove this as it was only needed for backwards compatibility +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; -export { BrowserTracing, addTracingExtensions, Replay, Feedback }; +export { + BrowserTracing, + addTracingExtensions, + // eslint-disable-next-line deprecation/deprecation + Replay, + replayIntegration, + // eslint-disable-next-line deprecation/deprecation + Feedback, + feedbackIntegration, +}; // Note: We do not export a shim for `Span` here, as that is quite complex and would blow up the bundle diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.ts index 5558703f7703..e17c7de4159a 100644 --- a/packages/browser/src/index.bundle.tracing.replay.feedback.ts +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.ts @@ -1,12 +1,13 @@ -import { Feedback } from '@sentry-internal/feedback'; +import { Feedback, feedbackIntegration } from '@sentry-internal/feedback'; import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; -import { Replay } from '@sentry/replay'; +import { Replay, replayIntegration } from '@sentry/replay'; import * as Sentry from './index.bundle.base'; // TODO (v8): Remove this as it was only needed for backwards compatibility // We want replay to be available under Sentry.Replay, to be consistent // with the NPM package version. +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; Sentry.Integrations.BrowserTracing = BrowserTracing; @@ -14,5 +15,15 @@ Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods addExtensionMethods(); -export { Feedback, Replay, BrowserTracing, Span, addExtensionMethods }; +export { + // eslint-disable-next-line deprecation/deprecation + Feedback, + // eslint-disable-next-line deprecation/deprecation + Replay, + feedbackIntegration, + replayIntegration, + BrowserTracing, + Span, + addExtensionMethods, +}; export * from './index.bundle.base'; diff --git a/packages/browser/src/index.bundle.tracing.replay.ts b/packages/browser/src/index.bundle.tracing.replay.ts index 0e7aa1ec19bc..5dc0537be064 100644 --- a/packages/browser/src/index.bundle.tracing.replay.ts +++ b/packages/browser/src/index.bundle.tracing.replay.ts @@ -1,12 +1,13 @@ -import { Feedback } from '@sentry-internal/integration-shims'; +import { Feedback, feedbackIntegration } from '@sentry-internal/integration-shims'; import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; -import { Replay } from '@sentry/replay'; +import { Replay, replayIntegration } from '@sentry/replay'; import * as Sentry from './index.bundle.base'; // TODO (v8): Remove this as it was only needed for backwards compatibility // We want replay to be available under Sentry.Replay, to be consistent // with the NPM package version. +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; Sentry.Integrations.BrowserTracing = BrowserTracing; @@ -14,5 +15,15 @@ Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods addExtensionMethods(); -export { Feedback, Replay, BrowserTracing, Span, addExtensionMethods }; +export { + // eslint-disable-next-line deprecation/deprecation + Feedback, + // eslint-disable-next-line deprecation/deprecation + Replay, + replayIntegration, + feedbackIntegration, + BrowserTracing, + Span, + addExtensionMethods, +}; export * from './index.bundle.base'; diff --git a/packages/browser/src/index.bundle.tracing.ts b/packages/browser/src/index.bundle.tracing.ts index 22b33962e860..f810b61b92a7 100644 --- a/packages/browser/src/index.bundle.tracing.ts +++ b/packages/browser/src/index.bundle.tracing.ts @@ -1,5 +1,5 @@ // This is exported so the loader does not fail when switching off Replay -import { Feedback, Replay } from '@sentry-internal/integration-shims'; +import { Feedback, Replay, feedbackIntegration, replayIntegration } from '@sentry-internal/integration-shims'; import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; import * as Sentry from './index.bundle.base'; @@ -7,6 +7,7 @@ import * as Sentry from './index.bundle.base'; // TODO (v8): Remove this as it was only needed for backwards compatibility // We want replay to be available under Sentry.Replay, to be consistent // with the NPM package version. +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; Sentry.Integrations.BrowserTracing = BrowserTracing; @@ -14,5 +15,15 @@ Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods addExtensionMethods(); -export { Feedback, Replay, BrowserTracing, Span, addExtensionMethods }; +export { + // eslint-disable-next-line deprecation/deprecation + Feedback, + // eslint-disable-next-line deprecation/deprecation + Replay, + feedbackIntegration, + replayIntegration, + BrowserTracing, + Span, + addExtensionMethods, +}; export * from './index.bundle.base'; diff --git a/packages/browser/src/index.bundle.ts b/packages/browser/src/index.bundle.ts index a2541ceadda1..a92ff6bf66ec 100644 --- a/packages/browser/src/index.bundle.ts +++ b/packages/browser/src/index.bundle.ts @@ -1,13 +1,30 @@ // This is exported so the loader does not fail when switching off Replay/Tracing -import { BrowserTracing, Feedback, Replay, addTracingExtensions } from '@sentry-internal/integration-shims'; +import { + BrowserTracing, + Feedback, + Replay, + addTracingExtensions, + feedbackIntegration, + replayIntegration, +} from '@sentry-internal/integration-shims'; import * as Sentry from './index.bundle.base'; // TODO (v8): Remove this as it was only needed for backwards compatibility +// eslint-disable-next-line deprecation/deprecation Sentry.Integrations.Replay = Replay; Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; -export { BrowserTracing, addTracingExtensions, Replay, Feedback }; +export { + BrowserTracing, + addTracingExtensions, + // eslint-disable-next-line deprecation/deprecation + Replay, + // eslint-disable-next-line deprecation/deprecation + Feedback, + feedbackIntegration, + replayIntegration, +}; // Note: We do not export a shim for `Span` here, as that is quite complex and would blow up the bundle diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 3dbec66f1e75..6c91e141149a 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -21,7 +21,11 @@ const INTEGRATIONS = { export { INTEGRATIONS as Integrations }; -export { Replay } from '@sentry/replay'; +export { + // eslint-disable-next-line deprecation/deprecation + Replay, + replayIntegration, +} from '@sentry/replay'; export type { ReplayEventType, ReplayEventWithTime, @@ -34,9 +38,18 @@ export type { ReplaySpanFrameEvent, } from '@sentry/replay'; -export { ReplayCanvas } from '@sentry-internal/replay-canvas'; +export { + // eslint-disable-next-line deprecation/deprecation + ReplayCanvas, + replayCanvasIntegration, +} from '@sentry-internal/replay-canvas'; -export { Feedback, sendFeedback } from '@sentry-internal/feedback'; +export { + // eslint-disable-next-line deprecation/deprecation + Feedback, + feedbackIntegration, + sendFeedback, +} from '@sentry-internal/feedback'; export { BrowserTracing, diff --git a/packages/browser/test/unit/index.bundle.feedback.test.ts b/packages/browser/test/unit/index.bundle.feedback.test.ts index 4c5f26e5f313..5fe2940d1881 100644 --- a/packages/browser/test/unit/index.bundle.feedback.test.ts +++ b/packages/browser/test/unit/index.bundle.feedback.test.ts @@ -1,5 +1,10 @@ -import { BrowserTracing as BrowserTracingShim, Replay as ReplayShim } from '@sentry-internal/integration-shims'; -import { Feedback } from '@sentry/browser'; +/* eslint-disable deprecation/deprecation */ +import { + BrowserTracing as BrowserTracingShim, + Replay as ReplayShim, + replayIntegration as replayIntegrationShim, +} from '@sentry-internal/integration-shims'; +import { Feedback, feedbackIntegration } from '@sentry/browser'; import * as TracingReplayBundle from '../../src/index.bundle.feedback'; @@ -16,10 +21,12 @@ describe('index.bundle.feedback', () => { expect(TracingReplayBundle.Integrations.Replay).toBe(ReplayShim); expect(TracingReplayBundle.Replay).toBe(ReplayShim); + expect(TracingReplayBundle.replayIntegration).toBe(replayIntegrationShim); expect(TracingReplayBundle.Integrations.BrowserTracing).toBe(BrowserTracingShim); expect(TracingReplayBundle.BrowserTracing).toBe(BrowserTracingShim); expect(TracingReplayBundle.Feedback).toBe(Feedback); + expect(TracingReplayBundle.feedbackIntegration).toBe(feedbackIntegration); }); }); diff --git a/packages/browser/test/unit/index.bundle.replay.test.ts b/packages/browser/test/unit/index.bundle.replay.test.ts index b14eb70f6330..e2d5640a2183 100644 --- a/packages/browser/test/unit/index.bundle.replay.test.ts +++ b/packages/browser/test/unit/index.bundle.replay.test.ts @@ -1,5 +1,10 @@ -import { BrowserTracing as BrowserTracingShim, Feedback as FeedbackShim } from '@sentry-internal/integration-shims'; -import { Replay } from '@sentry/browser'; +/* eslint-disable deprecation/deprecation */ +import { + BrowserTracing as BrowserTracingShim, + Feedback as FeedbackShim, + feedbackIntegration as feedbackIntegrationShim, +} from '@sentry-internal/integration-shims'; +import { Replay, replayIntegration } from '@sentry/browser'; import * as TracingReplayBundle from '../../src/index.bundle.replay'; @@ -16,10 +21,12 @@ describe('index.bundle.replay', () => { expect(TracingReplayBundle.Integrations.Replay).toBe(Replay); expect(TracingReplayBundle.Replay).toBe(Replay); + expect(TracingReplayBundle.replayIntegration).toBe(replayIntegration); expect(TracingReplayBundle.Integrations.BrowserTracing).toBe(BrowserTracingShim); expect(TracingReplayBundle.BrowserTracing).toBe(BrowserTracingShim); expect(TracingReplayBundle.Feedback).toBe(FeedbackShim); + expect(TracingReplayBundle.feedbackIntegration).toBe(feedbackIntegrationShim); }); }); diff --git a/packages/browser/test/unit/index.bundle.test.ts b/packages/browser/test/unit/index.bundle.test.ts index f61987732858..d637c8de1c3d 100644 --- a/packages/browser/test/unit/index.bundle.test.ts +++ b/packages/browser/test/unit/index.bundle.test.ts @@ -1,7 +1,10 @@ +/* eslint-disable deprecation/deprecation */ import { BrowserTracing as BrowserTracingShim, Feedback as FeedbackShim, Replay as ReplayShim, + feedbackIntegration as feedbackIntegrationShim, + replayIntegration as replayIntegrationShim, } from '@sentry-internal/integration-shims'; import * as TracingBundle from '../../src/index.bundle'; @@ -19,10 +22,12 @@ describe('index.bundle', () => { expect(TracingBundle.Integrations.Replay).toBe(ReplayShim); expect(TracingBundle.Replay).toBe(ReplayShim); + expect(TracingBundle.replayIntegration).toBe(replayIntegrationShim); expect(TracingBundle.Integrations.BrowserTracing).toBe(BrowserTracingShim); expect(TracingBundle.BrowserTracing).toBe(BrowserTracingShim); expect(TracingBundle.Feedback).toBe(FeedbackShim); + expect(TracingBundle.feedbackIntegration).toBe(feedbackIntegrationShim); }); }); diff --git a/packages/browser/test/unit/index.bundle.tracing.replay.feedback.test.ts b/packages/browser/test/unit/index.bundle.tracing.replay.feedback.test.ts index 0bd50453da43..29b700773d92 100644 --- a/packages/browser/test/unit/index.bundle.tracing.replay.feedback.test.ts +++ b/packages/browser/test/unit/index.bundle.tracing.replay.feedback.test.ts @@ -1,5 +1,6 @@ +/* eslint-disable deprecation/deprecation */ import { BrowserTracing } from '@sentry-internal/tracing'; -import { Feedback, Replay } from '@sentry/browser'; +import { Feedback, Replay, feedbackIntegration, replayIntegration } from '@sentry/browser'; import * as TracingReplayFeedbackBundle from '../../src/index.bundle.tracing.replay.feedback'; @@ -16,10 +17,12 @@ describe('index.bundle.tracing.replay.feedback', () => { expect(TracingReplayFeedbackBundle.Integrations.Replay).toBe(Replay); expect(TracingReplayFeedbackBundle.Replay).toBe(Replay); + expect(TracingReplayFeedbackBundle.replayIntegration).toBe(replayIntegration); expect(TracingReplayFeedbackBundle.Integrations.BrowserTracing).toBe(BrowserTracing); expect(TracingReplayFeedbackBundle.BrowserTracing).toBe(BrowserTracing); expect(TracingReplayFeedbackBundle.Feedback).toBe(Feedback); + expect(TracingReplayFeedbackBundle.feedbackIntegration).toBe(feedbackIntegration); }); }); diff --git a/packages/browser/test/unit/index.bundle.tracing.replay.test.ts b/packages/browser/test/unit/index.bundle.tracing.replay.test.ts index 4c8af2130a3b..4db32003607e 100644 --- a/packages/browser/test/unit/index.bundle.tracing.replay.test.ts +++ b/packages/browser/test/unit/index.bundle.tracing.replay.test.ts @@ -1,6 +1,10 @@ -import { Feedback as FeedbackShim } from '@sentry-internal/integration-shims'; +/* eslint-disable deprecation/deprecation */ +import { + Feedback as FeedbackShim, + feedbackIntegration as feedbackIntegrationShim, +} from '@sentry-internal/integration-shims'; import { BrowserTracing } from '@sentry-internal/tracing'; -import { Replay } from '@sentry/browser'; +import { Replay, replayIntegration } from '@sentry/browser'; import * as TracingReplayBundle from '../../src/index.bundle.tracing.replay'; @@ -17,10 +21,12 @@ describe('index.bundle.tracing.replay', () => { expect(TracingReplayBundle.Integrations.Replay).toBe(Replay); expect(TracingReplayBundle.Replay).toBe(Replay); + expect(TracingReplayBundle.replayIntegration).toBe(replayIntegration); expect(TracingReplayBundle.Integrations.BrowserTracing).toBe(BrowserTracing); expect(TracingReplayBundle.BrowserTracing).toBe(BrowserTracing); expect(TracingReplayBundle.Feedback).toBe(FeedbackShim); + expect(TracingReplayBundle.feedbackIntegration).toBe(feedbackIntegrationShim); }); }); diff --git a/packages/browser/test/unit/index.bundle.tracing.test.ts b/packages/browser/test/unit/index.bundle.tracing.test.ts index b439dece8c6f..cf03a26f7054 100644 --- a/packages/browser/test/unit/index.bundle.tracing.test.ts +++ b/packages/browser/test/unit/index.bundle.tracing.test.ts @@ -1,4 +1,10 @@ -import { Feedback as FeedbackShim, Replay as ReplayShim } from '@sentry-internal/integration-shims'; +/* eslint-disable deprecation/deprecation */ +import { + Feedback as FeedbackShim, + Replay as ReplayShim, + feedbackIntegration as feedbackIntegrationShim, + replayIntegration as replayIntegrationShim, +} from '@sentry-internal/integration-shims'; import { BrowserTracing } from '@sentry-internal/tracing'; import * as TracingBundle from '../../src/index.bundle.tracing'; @@ -16,10 +22,12 @@ describe('index.bundle.tracing', () => { expect(TracingBundle.Integrations.Replay).toBe(ReplayShim); expect(TracingBundle.Replay).toBe(ReplayShim); + expect(TracingBundle.replayIntegration).toBe(replayIntegrationShim); expect(TracingBundle.Integrations.BrowserTracing).toBe(BrowserTracing); expect(TracingBundle.BrowserTracing).toBe(BrowserTracing); expect(TracingBundle.Feedback).toBe(FeedbackShim); + expect(TracingBundle.feedbackIntegration).toBe(feedbackIntegrationShim); }); }); diff --git a/packages/ember/addon/index.ts b/packages/ember/addon/index.ts index 07b13697676e..4f1f399654e4 100644 --- a/packages/ember/addon/index.ts +++ b/packages/ember/addon/index.ts @@ -18,7 +18,10 @@ function _getSentryInitConfig(): EmberSentryConfig['sentry'] { return _global.__sentryEmberConfig; } -export function InitSentryForEmber(_runtimeConfig?: BrowserOptions): void { +/** + * Initialize the Sentry SDK for Ember. + */ +export function init(_runtimeConfig?: BrowserOptions): void { const environmentConfig = getOwnConfig().sentryConfig; assert('Missing configuration.', environmentConfig); @@ -125,5 +128,7 @@ export const instrumentRoutePerformance = (BaseRoute export * from '@sentry/browser'; -// init is now the preferred way to call initialization for this addon. -export const init = InitSentryForEmber; +/** + * @deprecated Use `Sentry.init()` instead. + */ +export const InitSentryForEmber = init; diff --git a/packages/ember/tests/acceptance/sentry-replay-test.ts b/packages/ember/tests/acceptance/sentry-replay-test.ts index 4e541c00a1df..c807233cea25 100644 --- a/packages/ember/tests/acceptance/sentry-replay-test.ts +++ b/packages/ember/tests/acceptance/sentry-replay-test.ts @@ -1,6 +1,6 @@ import { visit } from '@ember/test-helpers'; import * as Sentry from '@sentry/ember'; -import type { BrowserClient, Replay } from '@sentry/ember'; +import type { BrowserClient, replayIntegration } from '@sentry/ember'; import { setupApplicationTest } from 'ember-qunit'; import { module, test } from 'qunit'; @@ -13,10 +13,11 @@ module('Acceptance | Sentry Session Replay', function (hooks) { test('Test replay', async function (assert) { await visit('/replay'); - const integration = Sentry.getClient()?.getIntegrationByName('Replay'); + const integration = + Sentry.getClient()?.getIntegrationByName>('Replay'); assert.ok(integration); - const replay = (integration as Sentry.Replay)['_replay'] as Replay['_replay']; + const replay = integration!['_replay'] as ReturnType['_replay']; assert.true(replay.isEnabled()); assert.false(replay.isPaused()); diff --git a/packages/ember/tests/dummy/app/routes/replay.ts b/packages/ember/tests/dummy/app/routes/replay.ts index ce9ebe2fa813..20e5200760b3 100644 --- a/packages/ember/tests/dummy/app/routes/replay.ts +++ b/packages/ember/tests/dummy/app/routes/replay.ts @@ -4,10 +4,10 @@ import * as Sentry from '@sentry/ember'; export default class ReplayRoute extends Route { public async beforeModel(): Promise { - const { Replay } = Sentry; + const { replayIntegration } = Sentry; const client = Sentry.getClient(); if (client && !client.getIntegrationByName('Replay')) { - client.addIntegration(new Replay()); + client.addIntegration(replayIntegration()); } } } diff --git a/packages/feedback/src/index.ts b/packages/feedback/src/index.ts index 57b6075dfca5..8fcba29c5d0f 100644 --- a/packages/feedback/src/index.ts +++ b/packages/feedback/src/index.ts @@ -1,2 +1,6 @@ export { sendFeedback } from './sendFeedback'; -export { Feedback } from './integration'; +export { + // eslint-disable-next-line deprecation/deprecation + Feedback, + feedbackIntegration, +} from './integration'; diff --git a/packages/feedback/src/integration.ts b/packages/feedback/src/integration.ts index 639540e6eaa3..501f844abeaa 100644 --- a/packages/feedback/src/integration.ts +++ b/packages/feedback/src/integration.ts @@ -1,4 +1,4 @@ -import type { Integration } from '@sentry/types'; +import type { Integration, IntegrationFn } from '@sentry/types'; import { isBrowser, logger } from '@sentry/utils'; import { @@ -25,10 +25,17 @@ import { createWidget } from './widget/createWidget'; const doc = WINDOW.document; +export const feedbackIntegration = ((options?: OptionalFeedbackConfiguration) => { + // eslint-disable-next-line deprecation/deprecation + return new Feedback(options); +}) satisfies IntegrationFn; + /** * Feedback integration. When added as an integration to the SDK, it will * inject a button in the bottom-right corner of the window that opens a * feedback modal when clicked. + * + * @deprecated Use `feedbackIntegration()` instead. */ export class Feedback implements Integration { /** @@ -105,7 +112,7 @@ export class Feedback implements Integration { onSubmitError, onSubmitSuccess, }: OptionalFeedbackConfiguration = {}) { - // Initializations + // eslint-disable-next-line deprecation/deprecation this.name = Feedback.id; // tsc fails if these are not initialized explicitly constructor, e.g. can't call `_initialize()` diff --git a/packages/feedback/test/integration.test.ts b/packages/feedback/test/integration.test.ts index fd5ef67bed1f..4a651a8ef7be 100644 --- a/packages/feedback/test/integration.test.ts +++ b/packages/feedback/test/integration.test.ts @@ -1,7 +1,7 @@ import * as SentryUtils from '@sentry/utils'; import { ACTOR_LABEL } from '../src/constants'; -import { Feedback } from '../src/integration'; +import { feedbackIntegration } from '../src/integration'; jest.spyOn(SentryUtils, 'isBrowser').mockImplementation(() => true); @@ -14,7 +14,7 @@ jest.mock('../src/util/sendFeedbackRequest', () => { }); describe('Feedback integration', () => { - let feedback: Feedback; + let feedback: ReturnType; beforeEach(() => { jest.clearAllMocks(); @@ -27,7 +27,7 @@ describe('Feedback integration', () => { }); it('autoinjects widget with actor', () => { - feedback = new Feedback(); + feedback = feedbackIntegration(); feedback.setupOnce(); const widget = feedback.getWidget(); expect(widget?.actor?.el).toBeInstanceOf(HTMLButtonElement); @@ -40,7 +40,7 @@ describe('Feedback integration', () => { }); it('does not create a widget with `autoInject: false`', () => { - feedback = new Feedback({ autoInject: false }); + feedback = feedbackIntegration({ autoInject: false }); feedback.setupOnce(); const widget = feedback.getWidget(); expect(widget?.actor?.el).toBeUndefined(); @@ -49,7 +49,7 @@ describe('Feedback integration', () => { }); it('opens (and closes) dialog when calling `openDialog` without injecting an actor', () => { - feedback = new Feedback({ autoInject: false }); + feedback = feedbackIntegration({ autoInject: false }); feedback.setupOnce(); let widget = feedback.getWidget(); @@ -75,7 +75,7 @@ describe('Feedback integration', () => { const myActor = document.createElement('div'); myActor.textContent = 'my button'; - feedback = new Feedback({ autoInject: false }); + feedback = feedbackIntegration({ autoInject: false }); let widget = feedback.getWidget(); expect(widget).toBe(null); @@ -92,7 +92,7 @@ describe('Feedback integration', () => { }); it('creates multiple widgets and removes them', () => { - feedback = new Feedback({ autoInject: false }); + feedback = feedbackIntegration({ autoInject: false }); feedback.createWidget(); expect(feedback.getWidget()?.actor?.el).toBeInstanceOf(HTMLButtonElement); diff --git a/packages/integration-shims/src/Feedback.ts b/packages/integration-shims/src/Feedback.ts index 3a1efed9a89a..232e2be0830b 100644 --- a/packages/integration-shims/src/Feedback.ts +++ b/packages/integration-shims/src/Feedback.ts @@ -5,6 +5,8 @@ import { consoleSandbox } from '@sentry/utils'; * This is a shim for the Feedback integration. * It is needed in order for the CDN bundles to continue working when users add/remove feedback * from it, without changing their config. This is necessary for the loader mechanism. + * + * @deprecated Use `feedbackIntergation()` instead. */ class FeedbackShim implements Integration { /** @@ -19,6 +21,7 @@ class FeedbackShim implements Integration { // eslint-disable-next-line @typescript-eslint/no-explicit-any public constructor(_options: any) { + // eslint-disable-next-line deprecation/deprecation this.name = FeedbackShim.id; consoleSandbox(() => { @@ -67,4 +70,15 @@ class FeedbackShim implements Integration { } } +/** + * This is a shim for the Feedback integration. + * It is needed in order for the CDN bundles to continue working when users add/remove feedback + * from it, without changing their config. This is necessary for the loader mechanism. + */ +export function feedbackIntegration(_options: unknown): Integration { + // eslint-disable-next-line deprecation/deprecation + return new FeedbackShim({}); +} + +// eslint-disable-next-line deprecation/deprecation export { FeedbackShim as Feedback }; diff --git a/packages/integration-shims/src/Replay.ts b/packages/integration-shims/src/Replay.ts index 16c905946219..3bcc6c3e6563 100644 --- a/packages/integration-shims/src/Replay.ts +++ b/packages/integration-shims/src/Replay.ts @@ -5,6 +5,8 @@ import { consoleSandbox } from '@sentry/utils'; * This is a shim for the Replay integration. * It is needed in order for the CDN bundles to continue working when users add/remove replay * from it, without changing their config. This is necessary for the loader mechanism. + * + * @deprecated Use `replayIntegration()` instead. */ class ReplayShim implements Integration { /** @@ -19,6 +21,7 @@ class ReplayShim implements Integration { // eslint-disable-next-line @typescript-eslint/no-explicit-any public constructor(_options: any) { + // eslint-disable-next-line deprecation/deprecation this.name = ReplayShim.id; consoleSandbox(() => { @@ -48,4 +51,15 @@ class ReplayShim implements Integration { } } +/** + * This is a shim for the Replay integration. + * It is needed in order for the CDN bundles to continue working when users add/remove replay + * from it, without changing their config. This is necessary for the loader mechanism. + */ +export function replayIntegration(_options: unknown): Integration { + // eslint-disable-next-line deprecation/deprecation + return new ReplayShim({}); +} + +// eslint-disable-next-line deprecation/deprecation export { ReplayShim as Replay }; diff --git a/packages/integration-shims/src/index.ts b/packages/integration-shims/src/index.ts index 410a4a31d9c8..43243f69a194 100644 --- a/packages/integration-shims/src/index.ts +++ b/packages/integration-shims/src/index.ts @@ -1,3 +1,11 @@ -export { Feedback } from './Feedback'; -export { Replay } from './Replay'; +export { + // eslint-disable-next-line deprecation/deprecation + Feedback, + feedbackIntegration, +} from './Feedback'; +export { + // eslint-disable-next-line deprecation/deprecation + Replay, + replayIntegration, +} from './Replay'; export { BrowserTracing, addTracingExtensions } from './BrowserTracing'; diff --git a/packages/replay-canvas/src/canvas.ts b/packages/replay-canvas/src/canvas.ts index ba3ec85ebcfb..04f91af7d578 100644 --- a/packages/replay-canvas/src/canvas.ts +++ b/packages/replay-canvas/src/canvas.ts @@ -1,5 +1,5 @@ import { CanvasManager } from '@sentry-internal/rrweb'; -import { convertIntegrationFnToClass } from '@sentry/core'; +import { convertIntegrationFnToClass, defineIntegration } from '@sentry/core'; import type { CanvasManagerInterface, CanvasManagerOptions } from '@sentry/replay'; import type { Integration, IntegrationClass, IntegrationFn } from '@sentry/types'; @@ -54,10 +54,8 @@ const CANVAS_QUALITY = { const INTEGRATION_NAME = 'ReplayCanvas'; -/** - * An integration to add canvas recording to replay. - */ -const replayCanvasIntegration = ((options: Partial = {}) => { +/** Exported only for type safe tests. */ +export const _replayCanvasIntegration = ((options: Partial = {}) => { const _canvasOptions = { quality: options.quality || 'medium', enableManualSnapshot: options.enableManualSnapshot, @@ -91,7 +89,14 @@ const replayCanvasIntegration = ((options: Partial = {}) => }; }) satisfies IntegrationFn; -// TODO(v8) +/** + * Add this in addition to `replayIntegration()` to enable canvas recording. + */ +export const replayCanvasIntegration = defineIntegration(_replayCanvasIntegration); + +/** + * @deprecated Use `replayCanvasIntegration()` instead + */ // eslint-disable-next-line deprecation/deprecation export const ReplayCanvas = convertIntegrationFnToClass(INTEGRATION_NAME, replayCanvasIntegration) as IntegrationClass< Integration & { diff --git a/packages/replay-canvas/src/index.ts b/packages/replay-canvas/src/index.ts index 97e5bab007a2..b9dc20a69f53 100644 --- a/packages/replay-canvas/src/index.ts +++ b/packages/replay-canvas/src/index.ts @@ -1,2 +1,6 @@ -export { ReplayCanvas } from './canvas'; +export { + // eslint-disable-next-line deprecation/deprecation + ReplayCanvas, + replayCanvasIntegration, +} from './canvas'; export type { ReplayCanvasIntegrationOptions } from './canvas'; diff --git a/packages/replay-canvas/test/canvas.test.ts b/packages/replay-canvas/test/canvas.test.ts index 346df7b4a89d..534c28261498 100644 --- a/packages/replay-canvas/test/canvas.test.ts +++ b/packages/replay-canvas/test/canvas.test.ts @@ -1,7 +1,7 @@ -import { ReplayCanvas } from '../src/canvas'; +import { _replayCanvasIntegration } from '../src/canvas'; it('initializes with default options', () => { - const rc = new ReplayCanvas(); + const rc = _replayCanvasIntegration(); expect(rc.getOptions()).toEqual({ recordCanvas: true, @@ -17,7 +17,7 @@ it('initializes with default options', () => { }); it('initializes with quality option and manual snapshot', () => { - const rc = new ReplayCanvas({ enableManualSnapshot: true, quality: 'low' }); + const rc = _replayCanvasIntegration({ enableManualSnapshot: true, quality: 'low' }); expect(rc.getOptions()).toEqual({ enableManualSnapshot: true, diff --git a/packages/replay/src/index.ts b/packages/replay/src/index.ts index 8e8a5d55579a..6471ff9e87ce 100644 --- a/packages/replay/src/index.ts +++ b/packages/replay/src/index.ts @@ -1,4 +1,8 @@ -export { Replay } from './integration'; +export { + // eslint-disable-next-line deprecation/deprecation + Replay, + replayIntegration, +} from './integration'; export type { ReplayConfiguration, diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index c73da03c1b85..9f4d03e7f5fa 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -1,5 +1,5 @@ import { getClient } from '@sentry/core'; -import type { BrowserClientReplayOptions, Integration } from '@sentry/types'; +import type { BrowserClientReplayOptions, Integration, IntegrationFn } from '@sentry/types'; import { consoleSandbox, dropUndefinedKeys, isBrowser } from '@sentry/utils'; import { @@ -30,8 +30,14 @@ let _initialized = false; type InitialReplayPluginOptions = Omit & Partial>; +export const replayIntegration = ((options?: InitialReplayPluginOptions) => { + // eslint-disable-next-line deprecation/deprecation + return new Replay(options); +}) satisfies IntegrationFn; + /** * The main replay integration class, to be passed to `init({ integrations: [] })`. + * @deprecated Use `replayIntegration()` instead. */ export class Replay implements Integration { /** @@ -111,6 +117,7 @@ export class Replay implements Integration { // eslint-disable-next-line deprecation/deprecation ignoreClass, }: ReplayConfiguration = {}) { + // eslint-disable-next-line deprecation/deprecation this.name = Replay.id; const privacyOptions = getPrivacyOptions({ diff --git a/packages/replay/test/integration/beforeAddRecordingEvent.test.ts b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts index 68467f7bcafc..e8076875386b 100644 --- a/packages/replay/test/integration/beforeAddRecordingEvent.test.ts +++ b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts @@ -23,6 +23,7 @@ type MockTransportSend = jest.MockedFunction; describe('Integration | beforeAddRecordingEvent', () => { let replay: ReplayContainer; + // eslint-disable-next-line deprecation/deprecation let integration: Replay; let mockTransportSend: MockTransportSend; let mockSendReplayRequest: jest.SpyInstance; diff --git a/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts b/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts index 7ea8f45755a4..a1ca45a8d76a 100644 --- a/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts +++ b/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts @@ -1,6 +1,5 @@ import type { Event } from '@sentry/types'; -import type { Replay as ReplayIntegration } from '../../../src'; import { REPLAY_EVENT_NAME, SESSION_IDLE_EXPIRE_DURATION } from '../../../src/constants'; import { handleGlobalEventListener } from '../../../src/coreHandlers/handleGlobalEvent'; import type { ReplayContainer } from '../../../src/replay'; @@ -130,8 +129,7 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { }); it('tags errors and transactions with replay id for session samples', async () => { - let integration: ReplayIntegration; - ({ replay, integration } = await resetSdkMock({})); + const { replay, integration } = await resetSdkMock({}); // @ts-expect-error protected but ok to use for testing integration._initialize(); const transaction = Transaction(); diff --git a/packages/replay/test/integration/stop.test.ts b/packages/replay/test/integration/stop.test.ts index 043dd76bcddc..75faef8f0d04 100644 --- a/packages/replay/test/integration/stop.test.ts +++ b/packages/replay/test/integration/stop.test.ts @@ -17,6 +17,7 @@ type MockRunFlush = jest.MockedFunction; describe('Integration | stop', () => { let replay: ReplayContainer; + // eslint-disable-next-line deprecation/deprecation let integration: Replay; const prevLocation = WINDOW.location; diff --git a/packages/replay/test/mocks/mockSdk.ts b/packages/replay/test/mocks/mockSdk.ts index f7e268bb49ba..ccf11dabb6ad 100644 --- a/packages/replay/test/mocks/mockSdk.ts +++ b/packages/replay/test/mocks/mockSdk.ts @@ -51,8 +51,10 @@ class MockTransport implements Transport { export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }: MockSdkParams = {}): Promise<{ replay: ReplayContainer; + // eslint-disable-next-line deprecation/deprecation integration: ReplayIntegration; }> { + // eslint-disable-next-line deprecation/deprecation const { Replay } = await import('../../src'); // Scope this to the test, instead of the module diff --git a/packages/replay/test/mocks/resetSdkMock.ts b/packages/replay/test/mocks/resetSdkMock.ts index 7b3fb7a169b1..8c60f6d1c742 100644 --- a/packages/replay/test/mocks/resetSdkMock.ts +++ b/packages/replay/test/mocks/resetSdkMock.ts @@ -13,6 +13,7 @@ export async function resetSdkMock({ replayOptions, sentryOptions, autoStart }: domHandler: DomHandler; mockRecord: RecordMock; replay: ReplayContainer; + // eslint-disable-next-line deprecation/deprecation integration: ReplayIntegration; }> { let domHandler: DomHandler; diff --git a/packages/replay/test/unit/multipleInstances.test.ts b/packages/replay/test/unit/multipleInstances.test.ts index a271801936ff..fc5bc13e03cf 100644 --- a/packages/replay/test/unit/multipleInstances.test.ts +++ b/packages/replay/test/unit/multipleInstances.test.ts @@ -1,10 +1,10 @@ -import { Replay } from '../../src'; +import { replayIntegration } from '../../src/integration'; describe('Unit | multipleInstances', () => { it('throws on creating multiple instances', function () { expect(() => { - new Replay(); - new Replay(); + replayIntegration(); + replayIntegration(); }).toThrow(); }); }); From 62b0c4d0e552d09cd2ec3f706b735ae9e827281e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 23 Jan 2024 15:23:42 +0100 Subject: [PATCH 12/16] feat(sveltekit): Update default integration handling & deprecate `addOrUpdateIntegration` (#10263) This updates the last usage of `addOrUpdateIntegration` and deprecates it. --- .../src/client/browserTracingIntegration.ts | 14 +++ packages/sveltekit/src/client/sdk.ts | 67 ++++++++++---- .../src/server/rewriteFramesIntegration.ts | 77 ++++++++++++++++ packages/sveltekit/src/server/sdk.ts | 22 ++--- packages/sveltekit/src/server/utils.ts | 58 +----------- packages/sveltekit/test/client/sdk.test.ts | 12 --- .../test/server/rewriteFramesIntegration.ts | 88 ++++++++++++++++++ packages/sveltekit/test/server/sdk.test.ts | 12 ++- packages/sveltekit/test/server/utils.test.ts | 89 +------------------ packages/utils/src/userIntegrations.ts | 2 + packages/utils/test/userIntegrations.test.ts | 2 + 11 files changed, 253 insertions(+), 190 deletions(-) create mode 100644 packages/sveltekit/src/client/browserTracingIntegration.ts create mode 100644 packages/sveltekit/src/server/rewriteFramesIntegration.ts create mode 100644 packages/sveltekit/test/server/rewriteFramesIntegration.ts diff --git a/packages/sveltekit/src/client/browserTracingIntegration.ts b/packages/sveltekit/src/client/browserTracingIntegration.ts new file mode 100644 index 000000000000..9968f8b6de5f --- /dev/null +++ b/packages/sveltekit/src/client/browserTracingIntegration.ts @@ -0,0 +1,14 @@ +import { BrowserTracing as OriginalBrowserTracing } from '@sentry/svelte'; +import { svelteKitRoutingInstrumentation } from './router'; + +/** + * A custom BrowserTracing integration for Sveltekit. + */ +export class BrowserTracing extends OriginalBrowserTracing { + public constructor(options?: ConstructorParameters[0]) { + super({ + routingInstrumentation: svelteKitRoutingInstrumentation, + ...options, + }); + } +} diff --git a/packages/sveltekit/src/client/sdk.ts b/packages/sveltekit/src/client/sdk.ts index daf8cd3ed7d2..7b9c608a862d 100644 --- a/packages/sveltekit/src/client/sdk.ts +++ b/packages/sveltekit/src/client/sdk.ts @@ -1,9 +1,10 @@ import { applySdkMetadata, hasTracingEnabled } from '@sentry/core'; import type { BrowserOptions } from '@sentry/svelte'; -import { BrowserTracing, WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte'; -import { addOrUpdateIntegration } from '@sentry/utils'; +import { getDefaultIntegrations as getDefaultSvelteIntegrations } from '@sentry/svelte'; +import { WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte'; +import type { Integration } from '@sentry/types'; -import { svelteKitRoutingInstrumentation } from './router'; +import { BrowserTracing } from './browserTracingIntegration'; type WindowWithSentryFetchProxy = typeof WINDOW & { _sentryFetchProxy?: typeof fetch; @@ -18,15 +19,20 @@ declare const __SENTRY_TRACING__: boolean; * @param options Configuration options for the SDK. */ export function init(options: BrowserOptions): void { - applySdkMetadata(options, 'sveltekit', ['sveltekit', 'svelte']); + const opts = { + defaultIntegrations: getDefaultIntegrations(options), + ...options, + }; - addClientIntegrations(options); + applySdkMetadata(opts, 'sveltekit', ['sveltekit', 'svelte']); + + fixBrowserTracingIntegration(opts); // 1. Switch window.fetch to our fetch proxy we injected earlier const actualFetch = switchToFetchProxy(); // 2. Initialize the SDK which will instrument our proxy - initSvelteSdk(options); + initSvelteSdk(opts); // 3. Restore the original fetch now that our proxy is instrumented if (actualFetch) { @@ -36,24 +42,49 @@ export function init(options: BrowserOptions): void { getCurrentScope().setTag('runtime', 'browser'); } -function addClientIntegrations(options: BrowserOptions): void { - let integrations = options.integrations || []; +// TODO v8: Remove this again +// We need to handle BrowserTracing passed to `integrations` that comes from `@sentry/tracing`, not `@sentry/sveltekit` :( +function fixBrowserTracingIntegration(options: BrowserOptions): void { + const { integrations } = options; + if (!integrations) { + return; + } + + if (Array.isArray(integrations)) { + options.integrations = maybeUpdateBrowserTracingIntegration(integrations); + } else { + options.integrations = defaultIntegrations => { + const userFinalIntegrations = integrations(defaultIntegrations); + + return maybeUpdateBrowserTracingIntegration(userFinalIntegrations); + }; + } +} + +function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Integration[] { + 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 + if (browserTracing && !(browserTracing instanceof BrowserTracing)) { + 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); + } + + return integrations; +} - // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", - // in which case everything inside will get treeshaken away +function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefined { + // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", in which case everything inside + // will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - const defaultBrowserTracingIntegration = new BrowserTracing({ - routingInstrumentation: svelteKitRoutingInstrumentation, - }); - - integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations, { - 'options.routingInstrumentation': svelteKitRoutingInstrumentation, - }); + return [...getDefaultSvelteIntegrations(options), new BrowserTracing()]; } } - options.integrations = integrations; + return undefined; } /** diff --git a/packages/sveltekit/src/server/rewriteFramesIntegration.ts b/packages/sveltekit/src/server/rewriteFramesIntegration.ts new file mode 100644 index 000000000000..9187630e96ec --- /dev/null +++ b/packages/sveltekit/src/server/rewriteFramesIntegration.ts @@ -0,0 +1,77 @@ +import { defineIntegration } from '@sentry/core'; +import { rewriteFramesIntegration as originalRewriteFramesIntegration } from '@sentry/integrations'; +import type { IntegrationFn, StackFrame } from '@sentry/types'; +import { GLOBAL_OBJ, basename, escapeStringForRegex, join } from '@sentry/utils'; +import { WRAPPED_MODULE_SUFFIX } from '../vite/autoInstrument'; +import type { GlobalWithSentryValues } from '../vite/injectGlobalValues'; + +type StackFrameIteratee = (frame: StackFrame) => StackFrame; +interface RewriteFramesOptions { + root?: string; + prefix?: string; + iteratee?: StackFrameIteratee; +} + +export const customRewriteFramesIntegration = ((options?: RewriteFramesOptions) => { + return originalRewriteFramesIntegration({ + iteratee: rewriteFramesIteratee, + ...options, + }); +}) satisfies IntegrationFn; + +export const rewriteFramesIntegration = defineIntegration(customRewriteFramesIntegration); + +/** + * A custom iteratee function for the `RewriteFrames` integration. + * + * Does the same as the default iteratee, but also removes the `module` property from the + * frame to improve issue grouping. + * + * For some reason, our stack trace processing pipeline isn't able to resolve the bundled + * module name to the original file name correctly, leading to individual error groups for + * each module. Removing the `module` field makes the grouping algorithm fall back to the + * `filename` field, which is correctly resolved and hence grouping works as expected. + * + * Exported for tests only. + */ +export function rewriteFramesIteratee(frame: StackFrame): StackFrame { + if (!frame.filename) { + return frame; + } + const globalWithSentryValues: GlobalWithSentryValues = GLOBAL_OBJ; + const svelteKitBuildOutDir = globalWithSentryValues.__sentry_sveltekit_output_dir; + const prefix = 'app:///'; + + // Check if the frame filename begins with `/` or a Windows-style prefix such as `C:\` + const isWindowsFrame = /^[a-zA-Z]:\\/.test(frame.filename); + const startsWithSlash = /^\//.test(frame.filename); + if (isWindowsFrame || startsWithSlash) { + const filename = isWindowsFrame + ? frame.filename + .replace(/^[a-zA-Z]:/, '') // remove Windows-style prefix + .replace(/\\/g, '/') // replace all `\\` instances with `/` + : frame.filename; + + let strippedFilename; + if (svelteKitBuildOutDir) { + strippedFilename = filename.replace( + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input + escaped anyway + new RegExp(`^.*${escapeStringForRegex(join(svelteKitBuildOutDir, 'server'))}/`), + '', + ); + } else { + strippedFilename = basename(filename); + } + frame.filename = `${prefix}${strippedFilename}`; + } + + delete frame.module; + + // In dev-mode, the WRAPPED_MODULE_SUFFIX is still present in the frame's file name. + // We need to remove it to make sure that the frame's filename matches the actual file + if (frame.filename.endsWith(WRAPPED_MODULE_SUFFIX)) { + frame.filename = frame.filename.slice(0, -WRAPPED_MODULE_SUFFIX.length); + } + + return frame; +} diff --git a/packages/sveltekit/src/server/sdk.ts b/packages/sveltekit/src/server/sdk.ts index 1001e922a031..c6b3bd26dffe 100644 --- a/packages/sveltekit/src/server/sdk.ts +++ b/packages/sveltekit/src/server/sdk.ts @@ -1,29 +1,23 @@ import { applySdkMetadata, getCurrentScope } from '@sentry/core'; -import { RewriteFrames } from '@sentry/integrations'; import type { NodeOptions } from '@sentry/node'; +import { getDefaultIntegrations as getDefaultNodeIntegrations } from '@sentry/node'; import { init as initNodeSdk } from '@sentry/node'; -import { addOrUpdateIntegration } from '@sentry/utils'; -import { rewriteFramesIteratee } from './utils'; +import { rewriteFramesIntegration } from './rewriteFramesIntegration'; /** * * @param options */ export function init(options: NodeOptions): void { - applySdkMetadata(options, 'sveltekit', ['sveltekit', 'node']); + const opts = { + defaultIntegrations: [...getDefaultNodeIntegrations(options), rewriteFramesIntegration()], + ...options, + }; - addServerIntegrations(options); + applySdkMetadata(opts, 'sveltekit', ['sveltekit', 'node']); - initNodeSdk(options); + initNodeSdk(opts); getCurrentScope().setTag('runtime', 'node'); } - -function addServerIntegrations(options: NodeOptions): void { - options.integrations = addOrUpdateIntegration( - // eslint-disable-next-line deprecation/deprecation - new RewriteFrames({ iteratee: rewriteFramesIteratee }), - options.integrations || [], - ); -} diff --git a/packages/sveltekit/src/server/utils.ts b/packages/sveltekit/src/server/utils.ts index de4444f28992..4106f7f4a09c 100644 --- a/packages/sveltekit/src/server/utils.ts +++ b/packages/sveltekit/src/server/utils.ts @@ -1,11 +1,8 @@ import { flush } from '@sentry/node'; -import type { StackFrame } from '@sentry/types'; -import { GLOBAL_OBJ, basename, escapeStringForRegex, join, logger, tracingContextFromHeaders } from '@sentry/utils'; +import { logger, tracingContextFromHeaders } from '@sentry/utils'; import type { RequestEvent } from '@sveltejs/kit'; import { DEBUG_BUILD } from '../common/debug-build'; -import { WRAPPED_MODULE_SUFFIX } from '../vite/autoInstrument'; -import type { GlobalWithSentryValues } from '../vite/injectGlobalValues'; /** * Takes a request event and extracts traceparent and DSC data @@ -19,59 +16,6 @@ export function getTracePropagationData(event: RequestEvent): ReturnType { const platformSupportsStreaming = !process.env.LAMBDA_TASK_ROOT && !process.env.VERCEL; diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts index 28f053823e3b..10292658bc54 100644 --- a/packages/sveltekit/test/client/sdk.test.ts +++ b/packages/sveltekit/test/client/sdk.test.ts @@ -61,10 +61,7 @@ describe('Sentry client SDK', () => { ...tracingOptions, }); - const integrationsToInit = svelteInit.mock.calls[0][0].integrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); - - expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); expect(browserTracing).toBeDefined(); }); @@ -77,10 +74,7 @@ describe('Sentry client SDK', () => { ...tracingOptions, }); - const integrationsToInit = svelteInit.mock.calls[0][0].integrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); - - expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); expect(browserTracing).toBeUndefined(); }); @@ -96,10 +90,7 @@ describe('Sentry client SDK', () => { enableTracing: true, }); - const integrationsToInit = svelteInit.mock.calls[0][0].integrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); - - expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); expect(browserTracing).toBeUndefined(); // @ts-expect-error this is fine in the test @@ -113,12 +104,9 @@ describe('Sentry client SDK', () => { enableTracing: true, }); - const integrationsToInit = svelteInit.mock.calls[0][0].integrations; - const browserTracing = getClient()?.getIntegrationByName('BrowserTracing') as BrowserTracing; const options = browserTracing.options; - expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); expect(browserTracing).toBeDefined(); // This shows that the user-configured options are still here diff --git a/packages/sveltekit/test/server/rewriteFramesIntegration.ts b/packages/sveltekit/test/server/rewriteFramesIntegration.ts new file mode 100644 index 000000000000..b2acdcf9ccaf --- /dev/null +++ b/packages/sveltekit/test/server/rewriteFramesIntegration.ts @@ -0,0 +1,88 @@ +import { RewriteFrames } from '@sentry/integrations'; +import type { Event, StackFrame } from '@sentry/types'; +import { basename } from '@sentry/utils'; + +import { rewriteFramesIteratee } from '../../src/server/rewriteFramesIntegration'; +import type { GlobalWithSentryValues } from '../../src/vite/injectGlobalValues'; + +describe('rewriteFramesIteratee', () => { + it('removes the module property from the frame', () => { + const frame: StackFrame = { + filename: '/some/path/to/server/chunks/3-ab34d22f.js', + module: '3-ab34d22f.js', + }; + + const result = rewriteFramesIteratee(frame); + + expect(result).not.toHaveProperty('module'); + }); + + it('does the same filename modification as the default RewriteFrames iteratee if no output dir is available', () => { + const frame: StackFrame = { + filename: '/some/path/to/server/chunks/3-ab34d22f.js', + lineno: 1, + colno: 1, + module: '3-ab34d22f.js', + }; + + // eslint-disable-next-line deprecation/deprecation + const originalRewriteFrames = new RewriteFrames(); + // eslint-disable-next-line deprecation/deprecation + const rewriteFrames = new RewriteFrames({ iteratee: rewriteFramesIteratee }); + + const event: Event = { + exception: { + values: [ + { + stacktrace: { + frames: [frame], + }, + }, + ], + }, + }; + + const originalResult = originalRewriteFrames.processEvent(event); + const result = rewriteFrames.processEvent(event); + + expect(result.exception?.values?.[0]?.stacktrace?.frames?.[0]).toEqual({ + filename: 'app:///3-ab34d22f.js', + lineno: 1, + colno: 1, + }); + + expect(result).toStrictEqual(originalResult); + }); + + it.each([ + ['adapter-node', 'build', '/absolute/path/to/build/server/chunks/3-ab34d22f.js', 'app:///chunks/3-ab34d22f.js'], + [ + 'adapter-auto', + '.svelte-kit/output', + '/absolute/path/to/.svelte-kit/output/server/entries/pages/page.ts.js', + 'app:///entries/pages/page.ts.js', + ], + ])( + 'removes the absolut path to the server output dir, if the output dir is available (%s)', + (_, outputDir, frameFilename, modifiedFilename) => { + (globalThis as unknown as GlobalWithSentryValues).__sentry_sveltekit_output_dir = outputDir; + + const frame: StackFrame = { + filename: frameFilename, + lineno: 1, + colno: 1, + module: basename(frameFilename), + }; + + const result = rewriteFramesIteratee({ ...frame }); + + expect(result).toStrictEqual({ + filename: modifiedFilename, + lineno: 1, + colno: 1, + }); + + delete (globalThis as unknown as GlobalWithSentryValues).__sentry_sveltekit_output_dir; + }, + ); +}); diff --git a/packages/sveltekit/test/server/sdk.test.ts b/packages/sveltekit/test/server/sdk.test.ts index b4ecf75a744a..99dcb37516e1 100644 --- a/packages/sveltekit/test/server/sdk.test.ts +++ b/packages/sveltekit/test/server/sdk.test.ts @@ -1,5 +1,6 @@ import * as SentryNode from '@sentry/node'; -import { SDK_VERSION } from '@sentry/node'; +import type { NodeClient } from '@sentry/node'; +import { SDK_VERSION, getClient } from '@sentry/node'; import { GLOBAL_OBJ } from '@sentry/utils'; import { init } from '../../src/server/sdk'; @@ -46,5 +47,14 @@ describe('Sentry server SDK', () => { // @ts-expect-error need access to protected _tags attribute expect(currentScope._tags).toEqual({ runtime: 'node' }); }); + + it('adds rewriteFramesIntegration by default', () => { + init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + }); + + const rewriteFramesIntegration = getClient()?.getIntegrationByName('RewriteFrames'); + expect(rewriteFramesIntegration).toBeDefined(); + }); }); }); diff --git a/packages/sveltekit/test/server/utils.test.ts b/packages/sveltekit/test/server/utils.test.ts index aa2f9be62f94..8e5c064c338c 100644 --- a/packages/sveltekit/test/server/utils.test.ts +++ b/packages/sveltekit/test/server/utils.test.ts @@ -1,9 +1,4 @@ -import { RewriteFrames } from '@sentry/integrations'; -import type { Event, StackFrame } from '@sentry/types'; -import { basename } from '@sentry/utils'; - -import type { GlobalWithSentryValues } from '../../src/server/utils'; -import { getTracePropagationData, rewriteFramesIteratee } from '../../src/server/utils'; +import { getTracePropagationData } from '../../src/server/utils'; const MOCK_REQUEST_EVENT: any = { request: { @@ -58,85 +53,3 @@ describe('getTracePropagationData', () => { expect(dynamicSamplingContext).toBeUndefined(); }); }); - -describe('rewriteFramesIteratee', () => { - it('removes the module property from the frame', () => { - const frame: StackFrame = { - filename: '/some/path/to/server/chunks/3-ab34d22f.js', - module: '3-ab34d22f.js', - }; - - const result = rewriteFramesIteratee(frame); - - expect(result).not.toHaveProperty('module'); - }); - - it('does the same filename modification as the default RewriteFrames iteratee if no output dir is available', () => { - const frame: StackFrame = { - filename: '/some/path/to/server/chunks/3-ab34d22f.js', - lineno: 1, - colno: 1, - module: '3-ab34d22f.js', - }; - - // eslint-disable-next-line deprecation/deprecation - const originalRewriteFrames = new RewriteFrames(); - // eslint-disable-next-line deprecation/deprecation - const rewriteFrames = new RewriteFrames({ iteratee: rewriteFramesIteratee }); - - const event: Event = { - exception: { - values: [ - { - stacktrace: { - frames: [frame], - }, - }, - ], - }, - }; - - const originalResult = originalRewriteFrames.processEvent(event); - const result = rewriteFrames.processEvent(event); - - expect(result.exception?.values?.[0]?.stacktrace?.frames?.[0]).toEqual({ - filename: 'app:///3-ab34d22f.js', - lineno: 1, - colno: 1, - }); - - expect(result).toStrictEqual(originalResult); - }); - - it.each([ - ['adapter-node', 'build', '/absolute/path/to/build/server/chunks/3-ab34d22f.js', 'app:///chunks/3-ab34d22f.js'], - [ - 'adapter-auto', - '.svelte-kit/output', - '/absolute/path/to/.svelte-kit/output/server/entries/pages/page.ts.js', - 'app:///entries/pages/page.ts.js', - ], - ])( - 'removes the absolut path to the server output dir, if the output dir is available (%s)', - (_, outputDir, frameFilename, modifiedFilename) => { - (globalThis as GlobalWithSentryValues).__sentry_sveltekit_output_dir = outputDir; - - const frame: StackFrame = { - filename: frameFilename, - lineno: 1, - colno: 1, - module: basename(frameFilename), - }; - - const result = rewriteFramesIteratee({ ...frame }); - - expect(result).toStrictEqual({ - filename: modifiedFilename, - lineno: 1, - colno: 1, - }); - - delete (globalThis as GlobalWithSentryValues).__sentry_sveltekit_output_dir; - }, - ); -}); diff --git a/packages/utils/src/userIntegrations.ts b/packages/utils/src/userIntegrations.ts index d897284d4c0d..345c43ef04d7 100644 --- a/packages/utils/src/userIntegrations.ts +++ b/packages/utils/src/userIntegrations.ts @@ -50,6 +50,8 @@ function setNestedKey(obj: Record, keyPath: string, value: unknown) * @param userIntegrations Integrations defined by the user. * @param forcedOptions Options with which to patch an existing user-derived instance on the integration. * @returns A final integrations array. + * + * @deprecated This will be removed in v8. */ export function addOrUpdateIntegration( defaultIntegrationInstance: Integration, diff --git a/packages/utils/test/userIntegrations.test.ts b/packages/utils/test/userIntegrations.test.ts index a009378064f1..96782fc414d6 100644 --- a/packages/utils/test/userIntegrations.test.ts +++ b/packages/utils/test/userIntegrations.test.ts @@ -76,12 +76,14 @@ function runTest(testOptions: { let integrations; if (typeof userIntegrations === 'function') { + // eslint-disable-next-line deprecation/deprecation const wrappedUserIntegrationsFunction = addOrUpdateIntegration(forcedDogIntegrationInstance, userIntegrations, { dogName: 'Charlie', descriptor: 'goofy', }); integrations = wrappedUserIntegrationsFunction(underlyingDefaultIntegrations); } else { + // eslint-disable-next-line deprecation/deprecation integrations = addOrUpdateIntegration( forcedDogIntegrationInstance, userIntegrations, From a0b987abb9ba886f0b0820a98096522a1d1f34fe Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Tue, 23 Jan 2024 10:43:35 -0500 Subject: [PATCH 13/16] feat(feedback): Configure feedback border radius (#10289) Adds ability to configure all border radiuses on feedback widget Closes https://github.com/getsentry/sentry-javascript/issues/10256 --- packages/feedback/src/constants.ts | 4 ++++ packages/feedback/src/types/index.ts | 12 ++++++++++++ packages/feedback/src/widget/Actor.css.ts | 2 +- packages/feedback/src/widget/Dialog.css.ts | 8 ++++---- packages/feedback/src/widget/Main.css.ts | 4 ++++ 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/feedback/src/constants.ts b/packages/feedback/src/constants.ts index 48f699408762..6e3e9055b511 100644 --- a/packages/feedback/src/constants.ts +++ b/packages/feedback/src/constants.ts @@ -16,6 +16,7 @@ const LIGHT_THEME = { backgroundHover: '#f6f6f7', foreground: '#2b2233', border: '1.5px solid rgba(41, 35, 47, 0.13)', + borderRadius: '12px', boxShadow: '0px 4px 24px 0px rgba(43, 34, 51, 0.12)', success: '#268d75', @@ -39,6 +40,9 @@ const LIGHT_THEME = { inputForeground: INHERIT, inputBorder: 'var(--border)', inputOutlineFocus: SUBMIT_COLOR, + + formBorderRadius: '20px', + formContentBorderRadius: '6px', }; export const DEFAULT_THEME = { diff --git a/packages/feedback/src/types/index.ts b/packages/feedback/src/types/index.ts index a86fd4ee4107..c2642b54d09c 100644 --- a/packages/feedback/src/types/index.ts +++ b/packages/feedback/src/types/index.ts @@ -215,6 +215,10 @@ export interface FeedbackTheme { * Border styling for actor and dialog */ border: string; + /** + * Border radius styling for actor + */ + borderRadius: string; /** * Box shadow for actor and dialog */ @@ -299,6 +303,14 @@ export interface FeedbackTheme { * Border styles for form inputs when focused */ inputOutlineFocus: string; + /** + * Border radius for dialog + */ + formBorderRadius: string; + /** + * Border radius for form inputs + */ + formContentBorderRadius: string; } export interface FeedbackThemes { diff --git a/packages/feedback/src/widget/Actor.css.ts b/packages/feedback/src/widget/Actor.css.ts index c0fd8d3bd68f..44bd60a3418e 100644 --- a/packages/feedback/src/widget/Actor.css.ts +++ b/packages/feedback/src/widget/Actor.css.ts @@ -11,7 +11,7 @@ export function createActorStyles(d: Document): HTMLStyleElement { align-items: center; gap: 8px; - border-radius: 12px; + border-radius: var(--border-radius); cursor: pointer; font-size: 14px; font-weight: 600; diff --git a/packages/feedback/src/widget/Dialog.css.ts b/packages/feedback/src/widget/Dialog.css.ts index b9cd5a499c13..b3924f00524a 100644 --- a/packages/feedback/src/widget/Dialog.css.ts +++ b/packages/feedback/src/widget/Dialog.css.ts @@ -38,7 +38,7 @@ export function createDialogStyles(d: Document): HTMLStyleElement { top: var(--top); border: var(--border); - border-radius: 20px; + border-radius: var(--form-border-radius); background-color: var(--background); color: var(--foreground); @@ -113,7 +113,7 @@ export function createDialogStyles(d: Document): HTMLStyleElement { background-color: var(--input-background); box-sizing: border-box; border: var(--input-border); - border-radius: 6px; + border-radius: var(--form-content-border-radius); color: var(--input-foreground); font-size: 14px; font-weight: 500; @@ -138,7 +138,7 @@ export function createDialogStyles(d: Document): HTMLStyleElement { .btn { line-height: inherit; border: var(--cancel-border); - border-radius: 6px; + border-radius: var(--form-content-border-radius); cursor: pointer; font-size: 14px; font-weight: 600; @@ -178,7 +178,7 @@ export function createDialogStyles(d: Document): HTMLStyleElement { .success-message { background-color: var(--background); border: var(--border); - border-radius: 12px; + border-radius: var(--border-radius); box-shadow: var(--box-shadow); font-weight: 600; color: var(--success); diff --git a/packages/feedback/src/widget/Main.css.ts b/packages/feedback/src/widget/Main.css.ts index 0c74fedf6832..32bb8048e678 100644 --- a/packages/feedback/src/widget/Main.css.ts +++ b/packages/feedback/src/widget/Main.css.ts @@ -8,6 +8,7 @@ function getThemedCssVariables(theme: FeedbackTheme): string { --error: ${theme.error}; --success: ${theme.success}; --border: ${theme.border}; + --border-radius: ${theme.borderRadius}; --box-shadow: ${theme.boxShadow}; --submit-background: ${theme.submitBackground}; @@ -28,6 +29,9 @@ function getThemedCssVariables(theme: FeedbackTheme): string { --input-foreground: ${theme.inputForeground}; --input-border: ${theme.inputBorder}; --input-outline-focus: ${theme.inputOutlineFocus}; + + --form-border-radius: ${theme.formBorderRadius}; + --form-content-border-radius: ${theme.formContentBorderRadius}; `; } From 675309d9bbe47959e3caf2c79764274ee9fe4404 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 23 Jan 2024 10:48:10 -0500 Subject: [PATCH 14/16] feat(core): Expose `isInitialized()` to replace checking via `getClient` (#10296) Currently, you can use `Sentry.getClient() !== undefined` to check if Sentry was initialized. In v8, we want to change this so that this _always_ returns a client (possibly a Noop client), so this check will not work anymore there. Instead, we can provide a new util that does this explicitly, where we can control what it checks under the hood. --- MIGRATION.md | 6 ++++++ packages/astro/src/index.server.ts | 1 + packages/browser/src/exports.ts | 1 + packages/bun/src/index.ts | 1 + packages/core/src/exports.ts | 7 +++++++ packages/core/src/index.ts | 1 + packages/core/test/lib/exports.test.ts | 18 ++++++++++++++++++ packages/deno/src/index.ts | 1 + packages/node-experimental/src/index.ts | 1 + packages/node-experimental/src/sdk/api.ts | 4 ++-- packages/node-experimental/src/sdk/scope.ts | 5 +++++ packages/node/src/index.ts | 1 + packages/serverless/src/index.ts | 1 + packages/sveltekit/src/server/index.ts | 1 + packages/vercel-edge/src/index.ts | 1 + 15 files changed, 48 insertions(+), 2 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 88b27b25bc16..0253907e3278 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -10,6 +10,12 @@ 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 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 +to check if Sentry was actually initialized, using `getClient()` will thus not work anymore. Instead, you should use the +new `Sentry.isInitialized()` utility to check this. + ## Deprecate `getCurrentHub()` In v8, you will no longer have a Hub, only Scopes as a concept. This also means that `getCurrentHub()` will eventually diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 3074a061a2d8..2197c47381da 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -28,6 +28,7 @@ export { // eslint-disable-next-line deprecation/deprecation getCurrentHub, getClient, + isInitialized, getCurrentScope, getGlobalScope, getIsolationScope, diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 50a282cd1406..5d012bdea2b7 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -40,6 +40,7 @@ export { // eslint-disable-next-line deprecation/deprecation getCurrentHub, getClient, + isInitialized, getCurrentScope, Hub, // eslint-disable-next-line deprecation/deprecation diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index 5d5abce58f48..393e534e12ee 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -45,6 +45,7 @@ export { // eslint-disable-next-line deprecation/deprecation getCurrentHub, getClient, + isInitialized, getCurrentScope, getGlobalScope, getIsolationScope, diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index fa011fc672ac..e1fc9a6102ac 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -394,6 +394,13 @@ export function getClient(): C | undefined { return getCurrentHub().getClient(); } +/** + * Returns true if Sentry has been properly initialized. + */ +export function isInitialized(): boolean { + return !!getClient(); +} + /** * Get the currently active scope. */ diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index f2f33a884123..b9b4b045497e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -31,6 +31,7 @@ export { withScope, withIsolationScope, getClient, + isInitialized, getCurrentScope, startSession, endSession, diff --git a/packages/core/test/lib/exports.test.ts b/packages/core/test/lib/exports.test.ts index f0975065f12e..cb4c9fbdd16d 100644 --- a/packages/core/test/lib/exports.test.ts +++ b/packages/core/test/lib/exports.test.ts @@ -1,3 +1,4 @@ +import { GLOBAL_OBJ } from '@sentry/utils'; import { Hub, Scope, @@ -16,6 +17,7 @@ import { withIsolationScope, withScope, } from '../../src'; +import { isInitialized } from '../../src/exports'; import { TestClient, getDefaultTestClientOptions } from '../mocks/client'; function getTestClient(): TestClient { @@ -315,3 +317,19 @@ describe('session APIs', () => { }); }); }); + +describe('isInitialized', () => { + it('returns false if no client is setup', () => { + GLOBAL_OBJ.__SENTRY__.hub = undefined; + expect(isInitialized()).toBe(false); + }); + + it('returns true if client is setup', () => { + const client = getTestClient(); + const hub = new Hub(client); + // eslint-disable-next-line deprecation/deprecation + makeMain(hub); + + expect(isInitialized()).toBe(true); + }); +}); diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index d4759e71ce78..7adfcae9a894 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -44,6 +44,7 @@ export { // eslint-disable-next-line deprecation/deprecation getCurrentHub, getClient, + isInitialized, getCurrentScope, getGlobalScope, getIsolationScope, diff --git a/packages/node-experimental/src/index.ts b/packages/node-experimental/src/index.ts index 8a6a60dfcd48..b19dad4adfd7 100644 --- a/packages/node-experimental/src/index.ts +++ b/packages/node-experimental/src/index.ts @@ -17,6 +17,7 @@ export type { Span } from './types'; export { startSpan, startSpanManual, startInactiveSpan, getActiveSpan } from '@sentry/opentelemetry'; export { getClient, + isInitialized, addBreadcrumb, captureException, captureEvent, diff --git a/packages/node-experimental/src/sdk/api.ts b/packages/node-experimental/src/sdk/api.ts index a163205ce1ec..ae8450d58a7c 100644 --- a/packages/node-experimental/src/sdk/api.ts +++ b/packages/node-experimental/src/sdk/api.ts @@ -22,9 +22,9 @@ import { getContextFromScope, getScopesFromContext, setScopesOnContext } from '. import type { ExclusiveEventHintOrCaptureContext } from '../utils/prepareEvent'; import { parseEventHintOrCaptureContext } from '../utils/prepareEvent'; import type { Scope } from './scope'; -import { getClient, getCurrentScope, getGlobalScope, getIsolationScope } from './scope'; +import { getClient, getCurrentScope, getGlobalScope, getIsolationScope, isInitialized } from './scope'; -export { getCurrentScope, getGlobalScope, getIsolationScope, getClient }; +export { getCurrentScope, getGlobalScope, getIsolationScope, getClient, isInitialized }; export { setCurrentScope, setIsolationScope } from './scope'; /** diff --git a/packages/node-experimental/src/sdk/scope.ts b/packages/node-experimental/src/sdk/scope.ts index 5b9d56dc4b84..e55667992839 100644 --- a/packages/node-experimental/src/sdk/scope.ts +++ b/packages/node-experimental/src/sdk/scope.ts @@ -65,6 +65,11 @@ export function getClient(): C { return {} as C; } +/** If the SDK was initialized. */ +export function isInitialized(): boolean { + return !!getClient().getDsn(); +} + /** A fork of the classic scope with some otel specific stuff. */ export class Scope extends OpenTelemetryScope implements ScopeInterface { // Overwrite this if you want to use a specific isolation scope here diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 86ee3e78b961..4f563316338b 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -44,6 +44,7 @@ export { // eslint-disable-next-line deprecation/deprecation getCurrentHub, getClient, + isInitialized, getCurrentScope, getGlobalScope, getIsolationScope, diff --git a/packages/serverless/src/index.ts b/packages/serverless/src/index.ts index 3fe65ee74ea1..3fef9fb94283 100644 --- a/packages/serverless/src/index.ts +++ b/packages/serverless/src/index.ts @@ -32,6 +32,7 @@ export { // eslint-disable-next-line deprecation/deprecation getCurrentHub, getClient, + isInitialized, getCurrentScope, getGlobalScope, getIsolationScope, diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 91fb9e2d7771..7d523ee55fd9 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -25,6 +25,7 @@ export { // eslint-disable-next-line deprecation/deprecation getCurrentHub, getClient, + isInitialized, getCurrentScope, getGlobalScope, getIsolationScope, diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index b9c18cb6dc1b..abeb6d54b870 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -44,6 +44,7 @@ export { // eslint-disable-next-line deprecation/deprecation getCurrentHub, getClient, + isInitialized, getCurrentScope, getGlobalScope, getIsolationScope, From c017181e307d5c725a9bc5ea110fece9622de000 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 23 Jan 2024 11:04:21 -0500 Subject: [PATCH 15/16] fix(tracing): Don't send negative ttfb (#10286) As per https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStart, `responseStart` can be 0 if the request is coming straight from the cache. This might lead us to calculate a negative ttfb. To account for these scenarios, use `Math.max` to make sure we always set to 0 in the case of a negative value. --- .../src/browser/metrics/index.ts | 57 +++++++++++++------ .../test/browser/metrics/index.test.ts | 29 ++++++++++ 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/packages/tracing-internal/src/browser/metrics/index.ts b/packages/tracing-internal/src/browser/metrics/index.ts index f5652ff2a052..d0eb73b945d2 100644 --- a/packages/tracing-internal/src/browser/metrics/index.ts +++ b/packages/tracing-internal/src/browser/metrics/index.ts @@ -240,24 +240,7 @@ export function addPerformanceEntries(transaction: Transaction): void { // Measurements are only available for pageload transactions if (op === 'pageload') { - // Generate TTFB (Time to First Byte), which measured as the time between the beginning of the transaction and the - // start of the response in milliseconds - if (typeof responseStartTimestamp === 'number' && transactionStartTime) { - DEBUG_BUILD && logger.log('[Measurements] Adding TTFB'); - _measurements['ttfb'] = { - value: (responseStartTimestamp - transactionStartTime) * 1000, - unit: 'millisecond', - }; - - if (typeof requestStartTimestamp === 'number' && requestStartTimestamp <= responseStartTimestamp) { - // Capture the time spent making the request and receiving the first byte of the response. - // This is the time between the start of the request and the start of the response in milliseconds. - _measurements['ttfb.requestTime'] = { - value: (responseStartTimestamp - requestStartTimestamp) * 1000, - unit: 'millisecond', - }; - } - } + _addTtfbToMeasurements(_measurements, responseStartTimestamp, requestStartTimestamp, transactionStartTime); ['fcp', 'fp', 'lcp'].forEach(name => { if (!_measurements[name] || !transactionStartTime || timeOrigin >= transactionStartTime) { @@ -547,3 +530,41 @@ function setResourceEntrySizeData( data[dataKey] = entryVal; } } + +/** + * Add ttfb information to measurements + * + * Exported for tests + */ +export function _addTtfbToMeasurements( + _measurements: Measurements, + responseStartTimestamp: number | undefined, + requestStartTimestamp: number | undefined, + transactionStartTime: number | undefined, +): void { + // Generate TTFB (Time to First Byte), which measured as the time between the beginning of the transaction and the + // start of the response in milliseconds + if (typeof responseStartTimestamp === 'number' && transactionStartTime) { + DEBUG_BUILD && logger.log('[Measurements] Adding TTFB'); + _measurements['ttfb'] = { + // As per https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStart, + // responseStart can be 0 if the request is coming straight from the cache. + // This might lead us to calculate a negative ttfb if we don't use Math.max here. + // + // This logic is the same as what is in the web-vitals library to calculate ttfb + // https://github.com/GoogleChrome/web-vitals/blob/2301de5015e82b09925238a228a0893635854587/src/onTTFB.ts#L92 + // TODO(abhi): We should use the web-vitals library instead of this custom calculation. + value: Math.max(responseStartTimestamp - transactionStartTime, 0) * 1000, + unit: 'millisecond', + }; + + if (typeof requestStartTimestamp === 'number' && requestStartTimestamp <= responseStartTimestamp) { + // Capture the time spent making the request and receiving the first byte of the response. + // This is the time between the start of the request and the start of the response in milliseconds. + _measurements['ttfb.requestTime'] = { + value: (responseStartTimestamp - requestStartTimestamp) * 1000, + unit: 'millisecond', + }; + } + } +} diff --git a/packages/tracing-internal/test/browser/metrics/index.test.ts b/packages/tracing-internal/test/browser/metrics/index.test.ts index ce6060497ef7..e405760fbcc7 100644 --- a/packages/tracing-internal/test/browser/metrics/index.test.ts +++ b/packages/tracing-internal/test/browser/metrics/index.test.ts @@ -1,5 +1,6 @@ import { Transaction } from '../../../src'; import type { ResourceEntry } from '../../../src/browser/metrics'; +import { _addTtfbToMeasurements } from '../../../src/browser/metrics'; import { _addMeasureSpans, _addResourceSpans } from '../../../src/browser/metrics'; import { WINDOW } from '../../../src/browser/types'; @@ -261,6 +262,34 @@ describe('_addResourceSpans', () => { }); }); +describe('_addTtfbToMeasurements', () => { + it('adds ttfb to measurements', () => { + const measurements = {}; + _addTtfbToMeasurements(measurements, 300, 200, 100); + expect(measurements).toEqual({ + ttfb: { + unit: 'millisecond', + value: 200000, + }, + 'ttfb.requestTime': { + unit: 'millisecond', + value: 100000, + }, + }); + }); + + it('does not add negative ttfb', () => { + const measurements = {}; + _addTtfbToMeasurements(measurements, 100, 200, 300); + expect(measurements).toEqual({ + ttfb: { + unit: 'millisecond', + value: 0, + }, + }); + }); +}); + const setGlobalLocation = (location: Location) => { // @ts-expect-error need to delete this in order to set to new value delete WINDOW.location; From d98bd740ed71f62409ecaf093bf6eaa34a662ab9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 23 Jan 2024 11:17:56 -0500 Subject: [PATCH 16/16] meta(changelog): Update changelog for v7.95.0 --- CHANGELOG.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1ff0e5dfd42..28161bfb0ecc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,47 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.95.0 + +### Important Changes + +#### Deprecations + +This release includes some deprecations in preparation for v8. + +Most notably, it deprecates the `Replay` & `Feedback` classes in favor of a functional replacement: + +```js +import * as Sentry from '@sentry/browser'; + +Sentry.init({ + integrations: [ + // Instead of + new Sentry.Replay(), + new Sentry.Feedback(), + // Use the functional replacement: + Sentry.replayIntegration(), + Sentry.feedbackIntegration(), + ], +}); +``` + +- feat(core): Deprecate `Span.origin` in favor of `sentry.origin` attribute (#10260) +- feat(core): Deprecate `Span.parentSpanId` (#10244) +- feat(core): Expose `isInitialized()` to replace checking via `getClient` (#10296) +- feat(replay): Deprecate `Replay`, `ReplayCanvas`, `Feedback` classes (#10270) +- feat(wasm): Deprecate `Wasm` integration class (#10230) + +### Other Changes + +- feat: Make `parameterize` function available through browser and node API (#10085) +- feat(feedback): Configure feedback border radius (#10289) +- feat(sveltekit): Update default integration handling & deprecate `addOrUpdateIntegration` (#10263) +- fix(replay-canvas): Add missing dependency on @sentry/utils (#10279) +- fix(tracing): Don't send negative ttfb (#10286) + +Work in this release contributed by @AleshaOleg. Thank you for your contribution! + ## 7.94.1 This release fixes a publishing issue.