From ab23658916a68b75f2a5416239af5065652d9fca Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 Apr 2024 12:00:49 +0200 Subject: [PATCH 01/64] test(e2e): Disable faulty Next.js assertion (#11429) --- .../tests/request-instrumentation.test.ts | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts index c0a24f747d56..7dbe20e2efb6 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts @@ -19,14 +19,16 @@ test('Should send a transaction with a fetch span', async ({ page }) => { }), ); - expect((await transactionPromise).spans).toContainEqual( - expect.objectContaining({ - data: expect.objectContaining({ - 'http.method': 'GET', - 'sentry.op': 'http.client', - 'sentry.origin': 'auto.http.otel.http', - }), - description: 'GET http://example.com/', - }), - ); + // TODO: Uncomment the below when fixed. For whatever reason that we now have accepted, spans created with Node.js' http.get() will not attach themselves to transactions. + // More info: https://github.com/getsentry/sentry-javascript/pull/11016/files#diff-10fa195789425786c6e5e769380be18790768f0b76319ee41bbb488d9fe50405R4 + // expect((await transactionPromise).spans).toContainEqual( + // expect.objectContaining({ + // data: expect.objectContaining({ + // 'http.method': 'GET', + // 'sentry.op': 'http.client', + // 'sentry.origin': 'auto.http.otel.http', + // }), + // description: 'GET http://example.com/', + // }), + // ); }); From f2841167fc9fdead93cb5192d94aa0a2609ac13c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 8 Apr 2024 11:37:21 +0100 Subject: [PATCH 02/64] fix(opentelemetry): Avoid weakmap for storing data (#11470) It may happen that we have different modules, different versions etc. so the weakmaps may not be in sync. Just storing a non-enumerable property should be the safer choice here. Partially required for next.js 13.4.19 to work.... --- packages/opentelemetry/src/utils/contextData.ts | 7 ++++--- packages/opentelemetry/src/utils/spanData.ts | 16 ++++------------ 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/opentelemetry/src/utils/contextData.ts b/packages/opentelemetry/src/utils/contextData.ts index e6e4ab0548d3..8cf0833b0150 100644 --- a/packages/opentelemetry/src/utils/contextData.ts +++ b/packages/opentelemetry/src/utils/contextData.ts @@ -1,10 +1,11 @@ import type { Context } from '@opentelemetry/api'; import type { Scope } from '@sentry/types'; +import { addNonEnumerableProperty } from '@sentry/utils'; import { SENTRY_SCOPES_CONTEXT_KEY } from '../constants'; import type { CurrentScopes } from '../types'; -const SCOPE_CONTEXT_MAP = new WeakMap(); +const SCOPE_CONTEXT_FIELD = '_scopeContext'; /** * Try to get the current scopes from the given OTEL context. @@ -27,7 +28,7 @@ export function setScopesOnContext(context: Context, scopes: CurrentScopes): Con * We need this to get the context from the scope in the `trace` functions. */ export function setContextOnScope(scope: Scope, context: Context): void { - SCOPE_CONTEXT_MAP.set(scope, context); + addNonEnumerableProperty(scope, SCOPE_CONTEXT_FIELD, context); } /** @@ -35,5 +36,5 @@ export function setContextOnScope(scope: Scope, context: Context): void { * TODO v8: Use this for the `trace` functions. * */ export function getContextFromScope(scope: Scope): Context | undefined { - return SCOPE_CONTEXT_MAP.get(scope); + return (scope as { [SCOPE_CONTEXT_FIELD]?: Context })[SCOPE_CONTEXT_FIELD]; } diff --git a/packages/opentelemetry/src/utils/spanData.ts b/packages/opentelemetry/src/utils/spanData.ts index 8b0ea37bdcd5..3dc00f043f8a 100644 --- a/packages/opentelemetry/src/utils/spanData.ts +++ b/packages/opentelemetry/src/utils/spanData.ts @@ -1,17 +1,9 @@ import type { Scope } from '@sentry/types'; +import { addNonEnumerableProperty } from '@sentry/utils'; import type { AbstractSpan } from '../types'; -// We store the parent span, scopes & metadata in separate weakmaps, so we can access them for a given span -// This way we can enhance the data that an OTEL Span natively gives us -// and since we are using weakmaps, we do not need to clean up after ourselves -const SpanScopes = new WeakMap< - AbstractSpan, - { - scope: Scope; - isolationScope: Scope; - } ->(); +const SPAN_SCOPES_FIELD = '_spanScopes'; /** * Set the Sentry scope to be used for finishing a given OTEL span. @@ -25,7 +17,7 @@ export function setSpanScopes( isolationScope: Scope; }, ): void { - SpanScopes.set(span, scopes); + addNonEnumerableProperty(span, SPAN_SCOPES_FIELD, scopes); } /** Get the Sentry scopes to use for finishing an OTEL span. */ @@ -35,5 +27,5 @@ export function getSpanScopes(span: AbstractSpan): isolationScope: Scope; } | undefined { - return SpanScopes.get(span); + return (span as { [SPAN_SCOPES_FIELD]?: { scope: Scope; isolationScope: Scope } })[SPAN_SCOPES_FIELD]; } From 9c8f0d343a597fcb4602afd2ce87bb6d1a632b55 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 8 Apr 2024 12:10:53 +0100 Subject: [PATCH 03/64] feat(browser): Add `lazyLoadIntegration` utility (#11339) This can be used to lazy load a pluggable integration. Usage: ```js async function getOrLazyLoadFeedback() { const existing = Sentry.getFeedback(); // check if it has already been installed if (existing) { return existing; } try { const feedbackIntegration = await Sentry.lazyLoadIntegration('feedbackIntegration'); client.addIntegration(feedbackIntegration()); } catch(error) { // this can error, we need to handle this! } } ``` Closes https://github.com/getsentry/sentry-javascript/issues/10905 --- .../lazyLoad/existingIntegration/init.js | 13 +++ .../lazyLoad/existingIntegration/subject.js | 7 ++ .../lazyLoad/existingIntegration/test.ts | 24 ++++++ .../lazyLoad/validIntegration/init.js | 12 +++ .../lazyLoad/validIntegration/subject.js | 7 ++ .../lazyLoad/validIntegration/test.ts | 24 ++++++ packages/browser/src/client.ts | 5 +- packages/browser/src/exports.ts | 2 + .../browser/src/utils/lazyLoadIntegration.ts | 77 +++++++++++++++++ .../unit/utils/lazyLoadIntegration.test.ts | 83 +++++++++++++++++++ 10 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/init.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/init.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/test.ts create mode 100644 packages/browser/src/utils/lazyLoadIntegration.ts create mode 100644 packages/browser/test/unit/utils/lazyLoadIntegration.test.ts diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/init.js b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/init.js new file mode 100644 index 000000000000..85b9d1a0fb2e --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/init.js @@ -0,0 +1,13 @@ +import * as Sentry from '@sentry/browser'; +import { httpClientIntegration } from '@sentry/browser'; + +window.Sentry = { + ...Sentry, + // This would be done by the CDN bundle otherwise + httpClientIntegration: httpClientIntegration, +}; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [], +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/subject.js b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/subject.js new file mode 100644 index 000000000000..6ce3c00ab277 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/subject.js @@ -0,0 +1,7 @@ +window._testLazyLoadIntegration = async function run() { + const integration = await window.Sentry.lazyLoadIntegration('httpClientIntegration'); + + window.Sentry.getClient()?.addIntegration(integration()); + + window._integrationLoaded = true; +}; diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/test.ts b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/test.ts new file mode 100644 index 000000000000..7faba50a3864 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/existingIntegration/test.ts @@ -0,0 +1,24 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; + +sentryTest('it bails if the integration is already loaded', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + + const hasIntegration = await page.evaluate('!!window.Sentry.getClient()?.getIntegrationByName("HttpClient")'); + expect(hasIntegration).toBe(false); + + const scriptTagsBefore = await page.evaluate('document.querySelectorAll("script").length'); + + await page.evaluate('window._testLazyLoadIntegration()'); + await page.waitForFunction('window._integrationLoaded'); + + const scriptTagsAfter = await page.evaluate('document.querySelectorAll("script").length'); + + const hasIntegration2 = await page.evaluate('!!window.Sentry.getClient()?.getIntegrationByName("HttpClient")'); + expect(hasIntegration2).toBe(true); + + expect(scriptTagsAfter).toBe(scriptTagsBefore); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/init.js b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/init.js new file mode 100644 index 000000000000..eb75d30bf760 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/init.js @@ -0,0 +1,12 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = { + ...Sentry, + // Ensure this is _not_ set + httpClientIntegration: undefined, +}; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [], +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/subject.js b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/subject.js new file mode 100644 index 000000000000..6ce3c00ab277 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/subject.js @@ -0,0 +1,7 @@ +window._testLazyLoadIntegration = async function run() { + const integration = await window.Sentry.lazyLoadIntegration('httpClientIntegration'); + + window.Sentry.getClient()?.addIntegration(integration()); + + window._integrationLoaded = true; +}; diff --git a/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/test.ts b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/test.ts new file mode 100644 index 000000000000..d407445c0e84 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/lazyLoad/validIntegration/test.ts @@ -0,0 +1,24 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; + +sentryTest('it allows to lazy load an integration', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + + const hasIntegration = await page.evaluate('!!window.Sentry.getClient()?.getIntegrationByName("HttpClient")'); + expect(hasIntegration).toBe(false); + + const scriptTagsBefore = await page.evaluate('document.querySelectorAll("script").length'); + + await page.evaluate('window._testLazyLoadIntegration()'); + await page.waitForFunction('window._integrationLoaded'); + + const scriptTagsAfter = await page.evaluate('document.querySelectorAll("script").length'); + + const hasIntegration2 = await page.evaluate('!!window.Sentry.getClient()?.getIntegrationByName("HttpClient")'); + expect(hasIntegration2).toBe(true); + + expect(scriptTagsAfter).toBe(scriptTagsBefore + 1); +}); diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 869d28c16c50..882a27cf3881 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -34,7 +34,10 @@ export type BrowserOptions = Options & */ export type BrowserClientOptions = ClientOptions & BrowserClientReplayOptions & - BrowserClientProfilingOptions; + BrowserClientProfilingOptions & { + /** If configured, this URL will be used as base URL for lazy loading integration. */ + cdnBaseUrl?: string; + }; /** * The Sentry Browser SDK Client. diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index a83b1e549eba..2efa7f8ec6d6 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -95,3 +95,5 @@ export { globalHandlersIntegration } from './integrations/globalhandlers'; export { httpContextIntegration } from './integrations/httpcontext'; export { linkedErrorsIntegration } from './integrations/linkederrors'; export { browserApiErrorsIntegration } from './integrations/browserapierrors'; + +export { lazyLoadIntegration } from './utils/lazyLoadIntegration'; diff --git a/packages/browser/src/utils/lazyLoadIntegration.ts b/packages/browser/src/utils/lazyLoadIntegration.ts new file mode 100644 index 000000000000..19cf28f1af52 --- /dev/null +++ b/packages/browser/src/utils/lazyLoadIntegration.ts @@ -0,0 +1,77 @@ +import { SDK_VERSION, getClient } from '@sentry/core'; +import type { IntegrationFn } from '@sentry/types'; +import type { BrowserClient } from '../client'; +import { WINDOW } from '../helpers'; + +// This is a map of integration function method to bundle file name. +const LazyLoadableIntegrations = { + replayIntegration: 'replay', + replayCanvasIntegration: 'replay-canvas', + feedbackIntegration: 'feedback', + captureConsoleIntegration: 'captureconsole', + contextLinesIntegration: 'contextlines', + linkedErrorsIntegration: 'linkederrors', + debugIntegration: 'debug', + dedupeIntegration: 'dedupe', + extraErrorDataIntegration: 'extraerrordata', + httpClientIntegration: 'httpclient', + reportingObserverIntegration: 'reportingobserver', + rewriteFramesIntegration: 'rewriteframes', + sessionTimingIntegration: 'sessiontiming', +} as const; + +const WindowWithMaybeIntegration = WINDOW as { + Sentry?: Partial>; +}; + +/** + * Lazy load an integration from the CDN. + * Rejects if the integration cannot be loaded. + */ +export async function lazyLoadIntegration(name: keyof typeof LazyLoadableIntegrations): Promise { + const bundle = LazyLoadableIntegrations[name]; + + if (!bundle || !WindowWithMaybeIntegration.Sentry) { + throw new Error(`Cannot lazy load integration: ${name}`); + } + + // Bail if the integration already exists + const existing = WindowWithMaybeIntegration.Sentry[name]; + if (typeof existing === 'function') { + return existing; + } + + const url = getScriptURL(bundle); + const script = WINDOW.document.createElement('script'); + script.src = url; + script.crossOrigin = 'anonymous'; + + const waitForLoad = new Promise((resolve, reject) => { + script.addEventListener('load', () => resolve()); + script.addEventListener('error', reject); + }); + + WINDOW.document.body.appendChild(script); + + try { + await waitForLoad; + } catch { + throw new Error(`Error when loading integration: ${name}`); + } + + const integrationFn = WindowWithMaybeIntegration.Sentry[name]; + + if (typeof integrationFn !== 'function') { + throw new Error(`Could not load integration: ${name}`); + } + + return integrationFn; +} + +function getScriptURL(bundle: string): string { + const client = getClient(); + const options = client && client.getOptions(); + const baseURL = (options && options.cdnBaseUrl) || 'https://browser.sentry-cdn.com'; + + return new URL(`/${SDK_VERSION}/${bundle}.min.js`, baseURL).toString(); +} diff --git a/packages/browser/test/unit/utils/lazyLoadIntegration.test.ts b/packages/browser/test/unit/utils/lazyLoadIntegration.test.ts new file mode 100644 index 000000000000..a9a86cbf4374 --- /dev/null +++ b/packages/browser/test/unit/utils/lazyLoadIntegration.test.ts @@ -0,0 +1,83 @@ +import { TextDecoder, TextEncoder } from 'util'; +import { SDK_VERSION, lazyLoadIntegration } from '../../../src'; +import * as Sentry from '../../../src'; +const patchedEncoder = (!global.window.TextEncoder && (global.window.TextEncoder = TextEncoder)) || true; +// @ts-expect-error patch the encoder on the window, else importing JSDOM fails (deleted in afterAll) +const patchedDecoder = (!global.window.TextDecoder && (global.window.TextDecoder = TextDecoder)) || true; + +import { JSDOM } from 'jsdom'; + +const globalDocument = global.document; +const globalWindow = global.window; +const globalLocation = global.location; + +describe('lazyLoadIntegration', () => { + beforeEach(() => { + const dom = new JSDOM('', { + runScripts: 'dangerously', + resources: 'usable', + }); + + global.document = dom.window.document; + // @ts-expect-error need to override global document + global.window = dom.window; + global.location = dom.window.location; + // @ts-expect-error For testing sake + global.Sentry = undefined; + }); + + // Reset back to previous values + afterEach(() => { + global.document = globalDocument; + global.window = globalWindow; + global.location = globalLocation; + }); + + afterAll(() => { + // @ts-expect-error patch the encoder on the window, else importing JSDOM fails + patchedEncoder && delete global.window.TextEncoder; + // @ts-expect-error patch the encoder on the window, else importing JSDOM fails + patchedDecoder && delete global.window.TextDecoder; + }); + + test('it rejects invalid name', async () => { + // @ts-expect-error For testing sake - otherwise this bails out anyhow + global.Sentry = Sentry; + + // @ts-expect-error we want to test this + await expect(() => lazyLoadIntegration('invalid!!!')).rejects.toThrow('Cannot lazy load integration: invalid!!!'); + }); + + test('it rejects without global Sentry variable', async () => { + await expect(() => lazyLoadIntegration('httpClientIntegration')).rejects.toThrow( + 'Cannot lazy load integration: httpClientIntegration', + ); + }); + + test('it does not inject a script tag if integration already exists', async () => { + // @ts-expect-error For testing sake + global.Sentry = Sentry; + + const integration = await lazyLoadIntegration('httpClientIntegration'); + + expect(integration).toBe(Sentry.httpClientIntegration); + expect(global.document.querySelectorAll('script')).toHaveLength(0); + }); + + test('it injects a script tag if integration is not yet loaded xxx', async () => { + // @ts-expect-error For testing sake + global.Sentry = { + ...Sentry, + httpClientIntegration: undefined, + }; + + // We do not await here, as this this does not seem to work with JSDOM :( + // We have browser integration tests to check that this actually works + void lazyLoadIntegration('httpClientIntegration'); + + expect(global.document.querySelectorAll('script')).toHaveLength(1); + expect(global.document.querySelector('script')?.src).toEqual( + `https://browser.sentry-cdn.com/${SDK_VERSION}/httpclient.min.js`, + ); + }); +}); From 83f7ccec7298d77c862b83fce4ef6c1439c31a7e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 8 Apr 2024 13:11:58 +0200 Subject: [PATCH 04/64] feat(node): Update scope `transactionName` in http and express instrumentations (#11434) Concrete changes: 1. `addRequestDataToEvent` no longer applies its transaction name to _non-transaction_ events 2. Http instrumentation sets the isolation scope's `transactionName` in the Otel `requestHook` to the unparameterized request URL path (w/o query params or fragments). 3. Express instrumentation sets the isolation scope's `transactionName` in the Otel `spanName` hook to a parameterized route, once a request handler span (e.g. `app.get(...)`) is created. 4. As a consequence of the above, non-transaction events now get assigned `event.transaction` correctly from the scopes. --- .../express/tracing/withError/server.js | 29 +++++++ .../suites/express/tracing/withError/test.ts | 28 +++++++ packages/node/src/integrations/http.ts | 44 +++++++---- .../node/src/integrations/tracing/express.ts | 9 +++ packages/utils/src/requestdata.ts | 2 +- packages/utils/test/requestdata.test.ts | 77 +++++++++++-------- 6 files changed, 139 insertions(+), 50 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/express/tracing/withError/server.js create mode 100644 dev-packages/node-integration-tests/suites/express/tracing/withError/test.ts diff --git a/dev-packages/node-integration-tests/suites/express/tracing/withError/server.js b/dev-packages/node-integration-tests/suites/express/tracing/withError/server.js new file mode 100644 index 000000000000..3b45591ec4df --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express/tracing/withError/server.js @@ -0,0 +1,29 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + // disable attaching headers to /test/* endpoints + tracePropagationTargets: [/^(?!.*test).*$/], + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const cors = require('cors'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/test/:id1/:id2', (_req, res) => { + Sentry.captureMessage(new Error('error_1')); + res.send('Success'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/withError/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/withError/test.ts new file mode 100644 index 000000000000..f164bdd0caab --- /dev/null +++ b/dev-packages/node-integration-tests/suites/express/tracing/withError/test.ts @@ -0,0 +1,28 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +describe('express tracing experimental', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + describe('CJS', () => { + test('should apply the scope transactionName to error events', done => { + createRunner(__dirname, 'server.js') + .ignore('session', 'sessions', 'transaction') + .expect({ + event: { + exception: { + values: [ + { + value: 'error_1', + }, + ], + }, + transaction: 'GET /test/:id1/:id2', + }, + }) + .start(done) + .makeRequest('get', '/test/123/abc?q=1'); + }); + }); +}); diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index ed93ebacdaa5..c01b6dcf871b 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -4,10 +4,11 @@ import { SpanKind } from '@opentelemetry/api'; import { registerInstrumentations } from '@opentelemetry/instrumentation'; import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; -import { addBreadcrumb, defineIntegration, getIsolationScope, isSentryRequestUrl } from '@sentry/core'; +import { addBreadcrumb, defineIntegration, getIsolationScope, isSentryRequestUrl, spanToJSON } from '@sentry/core'; import { _INTERNAL, getClient, getSpanKind } from '@sentry/opentelemetry'; import type { IntegrationFn } from '@sentry/types'; +import { stripUrlQueryAndFragment } from '@sentry/utils'; import type { NodeClient } from '../sdk/client'; import { setIsolationScope } from '../sdk/scope'; import type { HTTPModuleRequestIncomingMessage } from '../transports/http-module'; @@ -81,19 +82,35 @@ const _httpIntegration = ((options: HttpOptions = {}) => { requireParentforOutgoingSpans: true, requireParentforIncomingSpans: false, requestHook: (span, req) => { - _updateSpan(span); + addOriginToSpan(span, 'auto.http.otel.http'); + + if (getSpanKind(span) !== SpanKind.SERVER) { + return; + } // Update the isolation scope, isolate this request - if (getSpanKind(span) === SpanKind.SERVER) { - const isolationScope = getIsolationScope().clone(); - isolationScope.setSDKProcessingMetadata({ request: req }); - - const client = getClient(); - if (client && client.getOptions().autoSessionTracking) { - isolationScope.setRequestSession({ status: 'ok' }); - } - setIsolationScope(isolationScope); + const isolationScope = getIsolationScope().clone(); + isolationScope.setSDKProcessingMetadata({ request: req }); + + const client = getClient(); + if (client && client.getOptions().autoSessionTracking) { + isolationScope.setRequestSession({ status: 'ok' }); } + setIsolationScope(isolationScope); + + // attempt to update the scope's `transactionName` based on the request URL + // Ideally, framework instrumentations coming after the HttpInstrumentation + // update the transactionName once we get a parameterized route. + const attributes = spanToJSON(span).data; + if (!attributes) { + return; + } + + const httpMethod = String(attributes['http.method']).toUpperCase() || 'GET'; + const httpTarget = stripUrlQueryAndFragment(String(attributes['http.target'])) || '/'; + const bestEffortTransactionName = `${httpMethod} ${httpTarget}`; + + isolationScope.setTransactionName(bestEffortTransactionName); }, responseHook: (span, res) => { if (_breadcrumbs) { @@ -123,11 +140,6 @@ const _httpIntegration = ((options: HttpOptions = {}) => { */ export const httpIntegration = defineIntegration(_httpIntegration); -/** Update the span with data we need. */ -function _updateSpan(span: Span): void { - addOriginToSpan(span, 'auto.http.otel.http'); -} - /** Add a breadcrumb for outgoing requests. */ function _addRequestBreadcrumb(span: Span, response: HTTPModuleRequestIncomingMessage | ServerResponse): void { if (getSpanKind(span) !== SpanKind.CLIENT) { diff --git a/packages/node/src/integrations/tracing/express.ts b/packages/node/src/integrations/tracing/express.ts index 12e44199e16d..57ea1cb75ac0 100644 --- a/packages/node/src/integrations/tracing/express.ts +++ b/packages/node/src/integrations/tracing/express.ts @@ -18,6 +18,15 @@ const _expressIntegration = (() => { requestHook(span) { addOriginToSpan(span, 'auto.http.otel.express'); }, + spanNameHook(info, defaultName) { + if (info.layerType === 'request_handler') { + // type cast b/c Otel unfortunately types info.request as any :( + const req = info.request as { method?: string }; + const method = req.method ? req.method.toUpperCase() : 'GET'; + getIsolationScope().setTransactionName(`${method} ${info.route}`); + } + return defaultName; + }, }), ], }); diff --git a/packages/utils/src/requestdata.ts b/packages/utils/src/requestdata.ts index 807bc0541c2c..85133521ef76 100644 --- a/packages/utils/src/requestdata.ts +++ b/packages/utils/src/requestdata.ts @@ -298,7 +298,7 @@ export function addRequestDataToEvent( } } - if (include.transaction && !event.transaction) { + if (include.transaction && !event.transaction && event.type === 'transaction') { // TODO do we even need this anymore? // TODO make this work for nextjs event.transaction = extractTransaction(req, include.transaction); diff --git a/packages/utils/test/requestdata.test.ts b/packages/utils/test/requestdata.test.ts index 3bd5ac507268..7e44f703c62a 100644 --- a/packages/utils/test/requestdata.test.ts +++ b/packages/utils/test/requestdata.test.ts @@ -149,52 +149,63 @@ describe('addRequestDataToEvent', () => { }); describe('transaction property', () => { - test('extracts method and full route path by default`', () => { - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); + describe('for transaction events', () => { + beforeEach(() => { + mockEvent.type = 'transaction'; + }); - expect(parsedRequest.transaction).toEqual('POST /routerMountPath/subpath/:parameterName'); - }); + test('extracts method and full route path by default`', () => { + const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); - test('extracts method and full path by default when mountpoint is `/`', () => { - mockReq.originalUrl = mockReq.originalUrl.replace('/routerMountpath', ''); - mockReq.baseUrl = ''; + expect(parsedRequest.transaction).toEqual('POST /routerMountPath/subpath/:parameterName'); + }); - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); + test('extracts method and full path by default when mountpoint is `/`', () => { + mockReq.originalUrl = mockReq.originalUrl.replace('/routerMountpath', ''); + mockReq.baseUrl = ''; - // `subpath/` is the full path here, because there's no router mount path - expect(parsedRequest.transaction).toEqual('POST /subpath/:parameterName'); - }); + const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); - test('fallback to method and `originalUrl` if route is missing', () => { - delete mockReq.route; + // `subpath/` is the full path here, because there's no router mount path + expect(parsedRequest.transaction).toEqual('POST /subpath/:parameterName'); + }); - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); + test('fallback to method and `originalUrl` if route is missing', () => { + delete mockReq.route; - expect(parsedRequest.transaction).toEqual('POST /routerMountPath/subpath/specificValue'); - }); + const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); - test('can extract path only instead if configured', () => { - const optionsWithPathTransaction = { - include: { - transaction: 'path', - }, - } as const; + expect(parsedRequest.transaction).toEqual('POST /routerMountPath/subpath/specificValue'); + }); - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq, optionsWithPathTransaction); + test('can extract path only instead if configured', () => { + const optionsWithPathTransaction = { + include: { + transaction: 'path', + }, + } as const; - expect(parsedRequest.transaction).toEqual('/routerMountPath/subpath/:parameterName'); - }); + const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq, optionsWithPathTransaction); - test('can extract handler name instead if configured', () => { - const optionsWithHandlerTransaction = { - include: { - transaction: 'handler', - }, - } as const; + expect(parsedRequest.transaction).toEqual('/routerMountPath/subpath/:parameterName'); + }); - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq, optionsWithHandlerTransaction); + test('can extract handler name instead if configured', () => { + const optionsWithHandlerTransaction = { + include: { + transaction: 'handler', + }, + } as const; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq, optionsWithHandlerTransaction); + + expect(parsedRequest.transaction).toEqual('parameterNameRouteHandler'); + }); + }); + it('transaction is not applied to non-transaction events', () => { + const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); - expect(parsedRequest.transaction).toEqual('parameterNameRouteHandler'); + expect(parsedRequest.transaction).toBeUndefined(); }); }); }); From a1e4efe98af16077123dfde6c28ed2eed7c8f060 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 8 Apr 2024 14:00:21 +0100 Subject: [PATCH 05/64] fix(opentelemetry): Use scope span capturing from core (#11475) To ensure we can use this in an isomorphic way. This means we do not rely on this being a mutable object anymore, but we need to ensure to update it (in http integration) on both the span _and_ for the current context. --- packages/core/src/tracing/index.ts | 1 + packages/node/src/integrations/http.ts | 18 +++++++++-- packages/node/test/integration/scope.test.ts | 6 ++-- packages/opentelemetry/src/index.ts | 1 - packages/opentelemetry/src/spanExporter.ts | 14 ++++++--- packages/opentelemetry/src/spanProcessor.ts | 4 +-- packages/opentelemetry/src/utils/spanData.ts | 31 ------------------- .../test/integration/scope.test.ts | 4 +-- 8 files changed, 33 insertions(+), 46 deletions(-) delete mode 100644 packages/opentelemetry/src/utils/spanData.ts diff --git a/packages/core/src/tracing/index.ts b/packages/core/src/tracing/index.ts index cd9ca5ea6351..7cc86ae76531 100644 --- a/packages/core/src/tracing/index.ts +++ b/packages/core/src/tracing/index.ts @@ -1,3 +1,4 @@ +export { setCapturedScopesOnSpan, getCapturedScopesOnSpan } from './utils'; export { addTracingExtensions } from './hubextensions'; export { startIdleSpan, TRACING_DEFAULTS } from './idleSpan'; export { SentrySpan } from './sentrySpan'; diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index c01b6dcf871b..1c7c9bc8842e 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -4,7 +4,16 @@ import { SpanKind } from '@opentelemetry/api'; import { registerInstrumentations } from '@opentelemetry/instrumentation'; import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; -import { addBreadcrumb, defineIntegration, getIsolationScope, isSentryRequestUrl, spanToJSON } from '@sentry/core'; +import { + addBreadcrumb, + defineIntegration, + getCapturedScopesOnSpan, + getCurrentScope, + getIsolationScope, + isSentryRequestUrl, + setCapturedScopesOnSpan, + spanToJSON, +} from '@sentry/core'; import { _INTERNAL, getClient, getSpanKind } from '@sentry/opentelemetry'; import type { IntegrationFn } from '@sentry/types'; @@ -88,8 +97,12 @@ const _httpIntegration = ((options: HttpOptions = {}) => { return; } + const scopes = getCapturedScopesOnSpan(span); + // Update the isolation scope, isolate this request - const isolationScope = getIsolationScope().clone(); + const isolationScope = (scopes.isolationScope || getIsolationScope()).clone(); + const scope = scopes.scope || getCurrentScope(); + isolationScope.setSDKProcessingMetadata({ request: req }); const client = getClient(); @@ -97,6 +110,7 @@ const _httpIntegration = ((options: HttpOptions = {}) => { isolationScope.setRequestSession({ status: 'ok' }); } setIsolationScope(isolationScope); + setCapturedScopesOnSpan(span, scope, isolationScope); // attempt to update the scope's `transactionName` based on the request URL // Ideally, framework instrumentations coming after the HttpInstrumentation diff --git a/packages/node/test/integration/scope.test.ts b/packages/node/test/integration/scope.test.ts index db3f61d251ba..ed9f20cf2b6c 100644 --- a/packages/node/test/integration/scope.test.ts +++ b/packages/node/test/integration/scope.test.ts @@ -1,5 +1,5 @@ -import { getCurrentScope } from '@sentry/core'; -import { getClient, getSpanScopes } from '@sentry/opentelemetry'; +import { getCapturedScopesOnSpan, getCurrentScope } from '@sentry/core'; +import { getClient } from '@sentry/opentelemetry'; import { clearGlobalScope } from '../../../core/test/lib/clear-global-scope'; import * as Sentry from '../../src/'; @@ -42,7 +42,7 @@ describe('Integration | Scope', () => { scope2.setTag('tag3', 'val3'); Sentry.startSpan({ name: 'outer' }, span => { - expect(getSpanScopes(span)?.scope).toBe(enableTracing ? scope2 : undefined); + expect(getCapturedScopesOnSpan(span)?.scope).toBe(enableTracing ? scope2 : undefined); spanId = span.spanContext().spanId; traceId = span.spanContext().traceId; diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 8cae9b317837..776dd8c50d0c 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -4,7 +4,6 @@ export type { OpenTelemetryClient } from './types'; export { wrapClientClass } from './custom/client'; export { getSpanKind } from './utils/getSpanKind'; -export { getSpanScopes } from './utils/spanData'; export { getScopesFromContext } from './utils/contextData'; diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index a2603fe91718..bdebb8e2f6e0 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -2,7 +2,12 @@ import type { Span } from '@opentelemetry/api'; import { SpanKind } from '@opentelemetry/api'; import type { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; -import { captureEvent, getMetricSummaryJsonForSpan, timedEventsToMeasurements } from '@sentry/core'; +import { + captureEvent, + getCapturedScopesOnSpan, + getMetricSummaryJsonForSpan, + timedEventsToMeasurements, +} from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -23,7 +28,6 @@ import { getLocalParentId } from './utils/groupSpansWithParents'; import { groupSpansWithParents } from './utils/groupSpansWithParents'; import { mapStatus } from './utils/mapStatus'; import { parseSpanDescription } from './utils/parseSpanDescription'; -import { getSpanScopes } from './utils/spanData'; type SpanNodeCompleted = SpanNode & { span: ReadableSpan }; @@ -176,7 +180,7 @@ function parseSpan(span: ReadableSpan): { op?: string; origin?: SpanOrigin; sour function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent { const { op, description, data, origin = 'manual', source } = getSpanData(span); - const capturedSpanScopes = getSpanScopes(span); + const capturedSpanScopes = getCapturedScopesOnSpan(span as unknown as Span); const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined; @@ -220,8 +224,8 @@ function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent { type: 'transaction', sdkProcessingMetadata: { ...dropUndefinedKeys({ - capturedSpanScope: capturedSpanScopes?.scope, - capturedSpanIsolationScope: capturedSpanScopes?.isolationScope, + capturedSpanScope: capturedSpanScopes.scope, + capturedSpanIsolationScope: capturedSpanScopes.isolationScope, sampleRate, dynamicSamplingContext: getDynamicSamplingContextFromSpan(span), }), diff --git a/packages/opentelemetry/src/spanProcessor.ts b/packages/opentelemetry/src/spanProcessor.ts index 740afc2cc6fa..e3f604cab64b 100644 --- a/packages/opentelemetry/src/spanProcessor.ts +++ b/packages/opentelemetry/src/spanProcessor.ts @@ -8,12 +8,12 @@ import { getDefaultIsolationScope, logSpanEnd, logSpanStart, + setCapturedScopesOnSpan, } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_PARENT_IS_REMOTE } from './semanticAttributes'; import { SentrySpanExporter } from './spanExporter'; import { getScopesFromContext } from './utils/contextData'; import { setIsSetup } from './utils/setupCheck'; -import { setSpanScopes } from './utils/spanData'; function onSpanStart(span: Span, parentContext: Context): void { // This is a reliable way to get the parent span - because this is exactly how the parent is identified in the OTEL SDK @@ -42,7 +42,7 @@ function onSpanStart(span: Span, parentContext: Context): void { // We need the scope at time of span creation in order to apply it to the event when the span is finished if (scopes) { - setSpanScopes(span, scopes); + setCapturedScopesOnSpan(span, scopes.scope, scopes.isolationScope); } logSpanStart(span); diff --git a/packages/opentelemetry/src/utils/spanData.ts b/packages/opentelemetry/src/utils/spanData.ts deleted file mode 100644 index 3dc00f043f8a..000000000000 --- a/packages/opentelemetry/src/utils/spanData.ts +++ /dev/null @@ -1,31 +0,0 @@ -import type { Scope } from '@sentry/types'; -import { addNonEnumerableProperty } from '@sentry/utils'; - -import type { AbstractSpan } from '../types'; - -const SPAN_SCOPES_FIELD = '_spanScopes'; - -/** - * Set the Sentry scope to be used for finishing a given OTEL span. - * This is different from `setCapturedScopesOnSpan`, as that works on _sentry_ spans, - * while here we are basically "caching" this on the otel spans. - */ -export function setSpanScopes( - span: AbstractSpan, - scopes: { - scope: Scope; - isolationScope: Scope; - }, -): void { - addNonEnumerableProperty(span, SPAN_SCOPES_FIELD, scopes); -} - -/** Get the Sentry scopes to use for finishing an OTEL span. */ -export function getSpanScopes(span: AbstractSpan): - | { - scope: Scope; - isolationScope: Scope; - } - | undefined { - return (span as { [SPAN_SCOPES_FIELD]?: { scope: Scope; isolationScope: Scope } })[SPAN_SCOPES_FIELD]; -} diff --git a/packages/opentelemetry/test/integration/scope.test.ts b/packages/opentelemetry/test/integration/scope.test.ts index 7742fcf06767..e17516a9d7bf 100644 --- a/packages/opentelemetry/test/integration/scope.test.ts +++ b/packages/opentelemetry/test/integration/scope.test.ts @@ -1,5 +1,6 @@ import { captureException, + getCapturedScopesOnSpan, getClient, getCurrentScope, getIsolationScope, @@ -9,7 +10,6 @@ import { } from '@sentry/core'; import { startSpan } from '../../src/trace'; -import { getSpanScopes } from '../../src/utils/spanData'; import type { TestClientInterface } from '../helpers/TestClient'; import { cleanupOtel, mockSdkInit } from '../helpers/mockSdkInit'; @@ -49,7 +49,7 @@ describe('Integration | Scope', () => { scope2.setTag('tag3', 'val3'); startSpan({ name: 'outer' }, span => { - expect(getSpanScopes(span)?.scope).toBe(enableTracing ? scope2 : undefined); + expect(getCapturedScopesOnSpan(span)?.scope).toBe(enableTracing ? scope2 : undefined); spanId = span.spanContext().spanId; traceId = span.spanContext().traceId; From 8aca02debd9272b5d5f4b6540f990883baf32f11 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 8 Apr 2024 09:26:20 -0400 Subject: [PATCH 06/64] ref(browser): Move utils to @sentry-internal/browser-utils (#11451) ref https://github.com/getsentry/sentry-javascript/issues/9832 Building off https://github.com/getsentry/sentry-javascript/pull/11381, this moves browser related utils into the browser utils package. Next step is to move `browserTracingIntegration` into browser! --- packages/browser-utils/.eslintrc.js | 3 +++ .../src/browser/backgroundtab.ts | 2 +- .../src/browser/browserTracingIntegration.ts | 13 +++------ .../browser-utils/src/browser/instrument.ts | 2 +- .../src/browser/metrics/index.ts | 2 +- packages/browser-utils/src/browser/request.ts | 3 +-- .../src/{common => }/debug-build.ts | 0 packages/browser-utils/src/index.ts | 9 +++++++ .../src/instrument/dom.ts | 15 +++-------- .../src/instrument/history.ts | 17 +++--------- .../src/instrument/xhr.ts | 27 +++++++------------ .../test/browser/request.test.ts | 7 ++--- .../test/instrument/dom.test.ts | 4 +-- .../test/instrument/xhr.test.ts | 5 ++-- .../browser/src/integrations/breadcrumbs.ts | 20 +++++++------- .../browser/src/integrations/httpclient.ts | 3 +-- packages/browser/src/profiling/integration.ts | 3 +-- packages/browser/src/profiling/utils.ts | 13 +++++++-- packages/browser/src/sdk.ts | 9 ++----- packages/deno/src/integrations/breadcrumbs.ts | 11 ++++++-- .../src/coreHandlers/util/xhrUtils.ts | 3 ++- .../src/util/addGlobalListeners.ts | 5 +++- .../beforeAddRecordingEvent.test.ts | 4 +-- .../test/integration/flush.test.ts | 3 ++- .../test/integration/sendReplayEvent.test.ts | 4 +-- .../test/integration/stop.test.ts | 4 +-- .../test/mocks/resetSdkMock.ts | 4 +-- .../handleNetworkBreadcrumbs.test.ts | 2 +- packages/utils/src/instrument/console.ts | 2 +- packages/utils/src/instrument/fetch.ts | 2 +- packages/utils/src/instrument/globalError.ts | 2 +- .../instrument/globalUnhandledRejection.ts | 2 +- .../instrument/{_handlers.ts => handlers.ts} | 0 packages/utils/src/instrument/index.ts | 14 +++------- 34 files changed, 105 insertions(+), 114 deletions(-) rename packages/browser-utils/src/{common => }/debug-build.ts (100%) rename packages/{utils => browser-utils}/src/instrument/dom.ts (94%) rename packages/{utils => browser-utils}/src/instrument/history.ts (78%) rename packages/{utils => browser-utils}/src/instrument/xhr.ts (86%) rename packages/{utils => browser-utils}/test/instrument/dom.test.ts (75%) rename packages/{utils => browser-utils}/test/instrument/xhr.test.ts (75%) rename packages/utils/src/instrument/{_handlers.ts => handlers.ts} (100%) diff --git a/packages/browser-utils/.eslintrc.js b/packages/browser-utils/.eslintrc.js index 50f4342a74c6..7346008450be 100644 --- a/packages/browser-utils/.eslintrc.js +++ b/packages/browser-utils/.eslintrc.js @@ -1,5 +1,8 @@ module.exports = { extends: ['../../.eslintrc.js'], + env: { + browser: true, + }, overrides: [ { files: ['src/**'], diff --git a/packages/browser-utils/src/browser/backgroundtab.ts b/packages/browser-utils/src/browser/backgroundtab.ts index 928503ba6869..a3ed7d88bd3e 100644 --- a/packages/browser-utils/src/browser/backgroundtab.ts +++ b/packages/browser-utils/src/browser/backgroundtab.ts @@ -2,7 +2,7 @@ import { SPAN_STATUS_ERROR, getActiveSpan, getRootSpan } from '@sentry/core'; import { spanToJSON } from '@sentry/core'; import { logger } from '@sentry/utils'; -import { DEBUG_BUILD } from '../common/debug-build'; +import { DEBUG_BUILD } from '../debug-build'; import { WINDOW } from './types'; /** diff --git a/packages/browser-utils/src/browser/browserTracingIntegration.ts b/packages/browser-utils/src/browser/browserTracingIntegration.ts index 5954ca9d4502..02ea72497a3d 100644 --- a/packages/browser-utils/src/browser/browserTracingIntegration.ts +++ b/packages/browser-utils/src/browser/browserTracingIntegration.ts @@ -16,15 +16,10 @@ import { } from '@sentry/core'; import type { Client, IntegrationFn, StartSpanOptions, TransactionSource } from '@sentry/types'; import type { Span } from '@sentry/types'; -import { - addHistoryInstrumentationHandler, - browserPerformanceTimeOrigin, - getDomElement, - logger, - uuid4, -} from '@sentry/utils'; - -import { DEBUG_BUILD } from '../common/debug-build'; +import { browserPerformanceTimeOrigin, getDomElement, logger, uuid4 } from '@sentry/utils'; + +import { DEBUG_BUILD } from '../debug-build'; +import { addHistoryInstrumentationHandler } from '../instrument/history'; import { registerBackgroundTabDetection } from './backgroundtab'; import { addPerformanceEntries, diff --git a/packages/browser-utils/src/browser/instrument.ts b/packages/browser-utils/src/browser/instrument.ts index e4af4805315a..535f584dddb1 100644 --- a/packages/browser-utils/src/browser/instrument.ts +++ b/packages/browser-utils/src/browser/instrument.ts @@ -1,6 +1,6 @@ import { getFunctionName, logger } from '@sentry/utils'; -import { DEBUG_BUILD } from '../common/debug-build'; +import { DEBUG_BUILD } from '../debug-build'; import { onCLS } from './web-vitals/getCLS'; import { onFID } from './web-vitals/getFID'; import { onLCP } from './web-vitals/getLCP'; diff --git a/packages/browser-utils/src/browser/metrics/index.ts b/packages/browser-utils/src/browser/metrics/index.ts index 770ec8354596..ec3de991d593 100644 --- a/packages/browser-utils/src/browser/metrics/index.ts +++ b/packages/browser-utils/src/browser/metrics/index.ts @@ -5,7 +5,7 @@ import type { Measurements, Span, SpanAttributes, StartSpanOptions } from '@sent import { browserPerformanceTimeOrigin, getComponentName, htmlTreeAsString, logger, parseUrl } from '@sentry/utils'; import { spanToJSON } from '@sentry/core'; -import { DEBUG_BUILD } from '../../common/debug-build'; +import { DEBUG_BUILD } from '../../debug-build'; import { addClsInstrumentationHandler, addFidInstrumentationHandler, diff --git a/packages/browser-utils/src/browser/request.ts b/packages/browser-utils/src/browser/request.ts index b808136b0325..73038996d7c0 100644 --- a/packages/browser-utils/src/browser/request.ts +++ b/packages/browser-utils/src/browser/request.ts @@ -16,14 +16,13 @@ import { import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, Span } from '@sentry/types'; import { BAGGAGE_HEADER_NAME, - SENTRY_XHR_DATA_KEY, addFetchInstrumentationHandler, - addXhrInstrumentationHandler, browserPerformanceTimeOrigin, dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, stringMatchesSomePattern, } from '@sentry/utils'; +import { SENTRY_XHR_DATA_KEY, addXhrInstrumentationHandler } from '../instrument/xhr'; import { addPerformanceInstrumentationHandler } from './instrument'; import { WINDOW } from './types'; diff --git a/packages/browser-utils/src/common/debug-build.ts b/packages/browser-utils/src/debug-build.ts similarity index 100% rename from packages/browser-utils/src/common/debug-build.ts rename to packages/browser-utils/src/debug-build.ts diff --git a/packages/browser-utils/src/index.ts b/packages/browser-utils/src/index.ts index 019639e8c0b3..0824c9f50358 100644 --- a/packages/browser-utils/src/index.ts +++ b/packages/browser-utils/src/index.ts @@ -12,4 +12,13 @@ export { addLcpInstrumentationHandler, } from './browser'; +export { addClickKeypressInstrumentationHandler } from './instrument/dom'; + +export { addHistoryInstrumentationHandler } from './instrument/history'; + +export { + addXhrInstrumentationHandler, + SENTRY_XHR_DATA_KEY, +} from './instrument/xhr'; + export type { RequestInstrumentationOptions } from './browser'; diff --git a/packages/utils/src/instrument/dom.ts b/packages/browser-utils/src/instrument/dom.ts similarity index 94% rename from packages/utils/src/instrument/dom.ts rename to packages/browser-utils/src/instrument/dom.ts index aab64c8c149b..4dfa5992aa07 100644 --- a/packages/utils/src/instrument/dom.ts +++ b/packages/browser-utils/src/instrument/dom.ts @@ -1,13 +1,7 @@ -// TODO(v8): Move everything in this file into the browser package. Nothing here is generic and we run risk of leaking browser types into non-browser packages. - -/* eslint-disable @typescript-eslint/no-explicit-any */ -/* eslint-disable @typescript-eslint/ban-types */ import type { HandlerDataDom } from '@sentry/types'; -import { uuid4 } from '../misc'; -import { addNonEnumerableProperty, fill } from '../object'; -import { GLOBAL_OBJ } from '../worldwide'; -import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; +import { addHandler, addNonEnumerableProperty, fill, maybeInstrument, triggerHandlers, uuid4 } from '@sentry/utils'; +import { WINDOW } from '../browser/types'; type SentryWrappedTarget = HTMLElement & { _sentryId?: string }; @@ -25,14 +19,13 @@ type RemoveEventListener = ( type InstrumentedElement = Element & { __sentry_instrumentation_handlers__?: { [key in 'click' | 'keypress']?: { - handler?: Function; + handler?: unknown; /** The number of custom listeners attached to this element */ refCount: number; }; }; }; -const WINDOW = GLOBAL_OBJ as unknown as Window; const DEBOUNCE_DURATION = 1000; let debounceTimerID: number | undefined; @@ -71,7 +64,7 @@ export function instrumentDOM(): void { // could potentially prevent the event from bubbling up to our global listeners. This way, our handler are still // guaranteed to fire at least once.) ['EventTarget', 'Node'].forEach((target: string) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any const proto = (WINDOW as any)[target] && (WINDOW as any)[target].prototype; // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, no-prototype-builtins if (!proto || !proto.hasOwnProperty || !proto.hasOwnProperty('addEventListener')) { diff --git a/packages/utils/src/instrument/history.ts b/packages/browser-utils/src/instrument/history.ts similarity index 78% rename from packages/utils/src/instrument/history.ts rename to packages/browser-utils/src/instrument/history.ts index dc144c0e5818..c918cd4ce2dc 100644 --- a/packages/utils/src/instrument/history.ts +++ b/packages/browser-utils/src/instrument/history.ts @@ -1,15 +1,6 @@ -// TODO(v8): Move everything in this file into the browser package. Nothing here is generic and we run risk of leaking browser types into non-browser packages. - -/* eslint-disable @typescript-eslint/no-explicit-any */ -/* eslint-disable @typescript-eslint/ban-types */ import type { HandlerDataHistory } from '@sentry/types'; - -import { fill } from '../object'; -import { supportsHistory } from '../supports'; -import { GLOBAL_OBJ } from '../worldwide'; -import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; - -const WINDOW = GLOBAL_OBJ as unknown as Window; +import { addHandler, fill, maybeInstrument, supportsHistory, triggerHandlers } from '@sentry/utils'; +import { WINDOW } from '../browser/types'; let lastHref: string | undefined; @@ -33,7 +24,7 @@ function instrumentHistory(): void { } const oldOnPopState = WINDOW.onpopstate; - WINDOW.onpopstate = function (this: WindowEventHandlers, ...args: any[]): any { + WINDOW.onpopstate = function (this: WindowEventHandlers, ...args: unknown[]) { const to = WINDOW.location.href; // keep track of the current URL state, as we always receive only the updated state const from = lastHref; @@ -53,7 +44,7 @@ function instrumentHistory(): void { }; function historyReplacementFunction(originalHistoryFunction: () => void): () => void { - return function (this: History, ...args: any[]): void { + return function (this: History, ...args: unknown[]): void { const url = args.length > 2 ? args[2] : undefined; if (url) { // coerce to string (this is what pushState does) diff --git a/packages/utils/src/instrument/xhr.ts b/packages/browser-utils/src/instrument/xhr.ts similarity index 86% rename from packages/utils/src/instrument/xhr.ts rename to packages/browser-utils/src/instrument/xhr.ts index b00300fd553a..05db402872ed 100644 --- a/packages/utils/src/instrument/xhr.ts +++ b/packages/browser-utils/src/instrument/xhr.ts @@ -1,18 +1,12 @@ -// TODO(v8): Move everything in this file into the browser package. Nothing here is generic and we run risk of leaking browser types into non-browser packages. - -/* eslint-disable @typescript-eslint/no-explicit-any */ -/* eslint-disable @typescript-eslint/ban-types */ import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, WrappedFunction } from '@sentry/types'; -import { isString } from '../is'; -import { fill } from '../object'; -import { GLOBAL_OBJ } from '../worldwide'; -import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; - -const WINDOW = GLOBAL_OBJ as unknown as Window; +import { addHandler, fill, isString, maybeInstrument, triggerHandlers } from '@sentry/utils'; +import { WINDOW } from '../browser/types'; export const SENTRY_XHR_DATA_KEY = '__sentry_xhr_v3__'; +type WindowWithXhr = Window & { XMLHttpRequest?: typeof XMLHttpRequest }; + /** * Add an instrumentation handler for when an XHR request happens. * The handler function is called once when the request starts and once when it ends, @@ -29,15 +23,14 @@ export function addXhrInstrumentationHandler(handler: (data: HandlerDataXhr) => /** Exported only for tests. */ export function instrumentXHR(): void { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (!(WINDOW as any).XMLHttpRequest) { + if (!(WINDOW as WindowWithXhr).XMLHttpRequest) { return; } const xhrproto = XMLHttpRequest.prototype; fill(xhrproto, 'open', function (originalOpen: () => void): () => void { - return function (this: XMLHttpRequest & SentryWrappedXMLHttpRequest, ...args: any[]): void { + return function (this: XMLHttpRequest & SentryWrappedXMLHttpRequest, ...args: unknown[]): void { const startTimestamp = Date.now(); // open() should always be called with two or more arguments @@ -87,8 +80,8 @@ export function instrumentXHR(): void { }; if ('onreadystatechange' in this && typeof this.onreadystatechange === 'function') { - fill(this, 'onreadystatechange', function (original: WrappedFunction): Function { - return function (this: SentryWrappedXMLHttpRequest, ...readyStateArgs: any[]): void { + fill(this, 'onreadystatechange', function (original: WrappedFunction) { + return function (this: SentryWrappedXMLHttpRequest, ...readyStateArgs: unknown[]): void { onreadystatechangeHandler(); return original.apply(this, readyStateArgs); }; @@ -100,7 +93,7 @@ export function instrumentXHR(): void { // Intercepting `setRequestHeader` to access the request headers of XHR instance. // This will only work for user/library defined headers, not for the default/browser-assigned headers. // Request cookies are also unavailable for XHR, as `Cookie` header can't be defined by `setRequestHeader`. - fill(this, 'setRequestHeader', function (original: WrappedFunction): Function { + fill(this, 'setRequestHeader', function (original: WrappedFunction) { return function (this: SentryWrappedXMLHttpRequest, ...setRequestHeaderArgs: unknown[]): void { const [header, value] = setRequestHeaderArgs; @@ -119,7 +112,7 @@ export function instrumentXHR(): void { }); fill(xhrproto, 'send', function (originalSend: () => void): () => void { - return function (this: XMLHttpRequest & SentryWrappedXMLHttpRequest, ...args: any[]): void { + return function (this: XMLHttpRequest & SentryWrappedXMLHttpRequest, ...args: unknown[]): void { const sentryXhrData = this[SENTRY_XHR_DATA_KEY]; if (!sentryXhrData) { diff --git a/packages/browser-utils/test/browser/request.test.ts b/packages/browser-utils/test/browser/request.test.ts index 8855203ce136..0671e8d5805c 100644 --- a/packages/browser-utils/test/browser/request.test.ts +++ b/packages/browser-utils/test/browser/request.test.ts @@ -1,6 +1,7 @@ -/* eslint-disable deprecation/deprecation */ import * as utils from '@sentry/utils'; +import * as xhrInstrumentation from '../../src/instrument/xhr'; + import { extractNetworkProtocol, instrumentOutgoingRequests, shouldAttachHeaders } from '../../src/browser/request'; import { WINDOW } from '../../src/browser/types'; @@ -17,7 +18,7 @@ describe('instrumentOutgoingRequests', () => { it('instruments fetch and xhr requests', () => { const addFetchSpy = jest.spyOn(utils, 'addFetchInstrumentationHandler'); - const addXhrSpy = jest.spyOn(utils, 'addXhrInstrumentationHandler'); + const addXhrSpy = jest.spyOn(xhrInstrumentation, 'addXhrInstrumentationHandler'); instrumentOutgoingRequests(); @@ -34,7 +35,7 @@ describe('instrumentOutgoingRequests', () => { }); it('does not instrument xhr requests if traceXHR is false', () => { - const addXhrSpy = jest.spyOn(utils, 'addXhrInstrumentationHandler'); + const addXhrSpy = jest.spyOn(xhrInstrumentation, 'addXhrInstrumentationHandler'); instrumentOutgoingRequests({ traceXHR: false }); diff --git a/packages/utils/test/instrument/dom.test.ts b/packages/browser-utils/test/instrument/dom.test.ts similarity index 75% rename from packages/utils/test/instrument/dom.test.ts rename to packages/browser-utils/test/instrument/dom.test.ts index 28745109e0f8..ad1dc2fd82fa 100644 --- a/packages/utils/test/instrument/dom.test.ts +++ b/packages/browser-utils/test/instrument/dom.test.ts @@ -1,7 +1,7 @@ import { instrumentDOM } from '../../src/instrument/dom'; -jest.mock('../../src/worldwide', () => { - const original = jest.requireActual('../../src/worldwide'); +jest.mock('@sentry/utils', () => { + const original = jest.requireActual('@sentry/utils'); return { ...original, diff --git a/packages/utils/test/instrument/xhr.test.ts b/packages/browser-utils/test/instrument/xhr.test.ts similarity index 75% rename from packages/utils/test/instrument/xhr.test.ts rename to packages/browser-utils/test/instrument/xhr.test.ts index 60485ff1d398..46fa1a477521 100644 --- a/packages/utils/test/instrument/xhr.test.ts +++ b/packages/browser-utils/test/instrument/xhr.test.ts @@ -1,8 +1,7 @@ import { instrumentXHR } from '../../src/instrument/xhr'; -jest.mock('../../src/worldwide', () => { - const original = jest.requireActual('../../src/worldwide'); - +jest.mock('@sentry/utils', () => { + const original = jest.requireActual('@sentry/utils'); return { ...original, GLOBAL_OBJ: { diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts index 474432fedb48..e3c1120fca57 100644 --- a/packages/browser/src/integrations/breadcrumbs.ts +++ b/packages/browser/src/integrations/breadcrumbs.ts @@ -1,28 +1,28 @@ +import { + SENTRY_XHR_DATA_KEY, + addClickKeypressInstrumentationHandler, + addHistoryInstrumentationHandler, + addXhrInstrumentationHandler, +} from '@sentry-internal/browser-utils'; import { addBreadcrumb, defineIntegration, getClient } from '@sentry/core'; import type { + Breadcrumb, Client, Event as SentryEvent, + FetchBreadcrumbData, + FetchBreadcrumbHint, HandlerDataConsole, HandlerDataDom, HandlerDataFetch, HandlerDataHistory, HandlerDataXhr, IntegrationFn, -} from '@sentry/types'; -import type { - Breadcrumb, - FetchBreadcrumbData, - FetchBreadcrumbHint, XhrBreadcrumbData, XhrBreadcrumbHint, -} from '@sentry/types/build/types/breadcrumb'; +} from '@sentry/types'; import { - SENTRY_XHR_DATA_KEY, - addClickKeypressInstrumentationHandler, addConsoleInstrumentationHandler, addFetchInstrumentationHandler, - addHistoryInstrumentationHandler, - addXhrInstrumentationHandler, getComponentName, getEventDescription, htmlTreeAsString, diff --git a/packages/browser/src/integrations/httpclient.ts b/packages/browser/src/integrations/httpclient.ts index e8d5d596d975..b48cc69d09c0 100644 --- a/packages/browser/src/integrations/httpclient.ts +++ b/packages/browser/src/integrations/httpclient.ts @@ -1,11 +1,10 @@ +import { SENTRY_XHR_DATA_KEY, addXhrInstrumentationHandler } from '@sentry-internal/browser-utils'; import { captureEvent, defineIntegration, getClient, isSentryRequestUrl } from '@sentry/core'; import type { Client, Event as SentryEvent, IntegrationFn, SentryWrappedXMLHttpRequest } from '@sentry/types'; import { GLOBAL_OBJ, - SENTRY_XHR_DATA_KEY, addExceptionMechanism, addFetchInstrumentationHandler, - addXhrInstrumentationHandler, logger, supportsNativeFetch, } from '@sentry/utils'; diff --git a/packages/browser/src/profiling/integration.ts b/packages/browser/src/profiling/integration.ts index af88bd8d91e6..585ad28802b3 100644 --- a/packages/browser/src/profiling/integration.ts +++ b/packages/browser/src/profiling/integration.ts @@ -1,6 +1,5 @@ import { defineIntegration, getActiveSpan, getRootSpan } from '@sentry/core'; -import type { EventEnvelope, IntegrationFn, Span } from '@sentry/types'; -import type { Profile } from '@sentry/types/src/profiling'; +import type { EventEnvelope, IntegrationFn, Profile, Span } from '@sentry/types'; import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; diff --git a/packages/browser/src/profiling/utils.ts b/packages/browser/src/profiling/utils.ts index a9dd735a3812..4682633d101c 100644 --- a/packages/browser/src/profiling/utils.ts +++ b/packages/browser/src/profiling/utils.ts @@ -1,8 +1,17 @@ /* eslint-disable max-lines */ import { DEFAULT_ENVIRONMENT, getClient, spanToJSON } from '@sentry/core'; -import type { DebugImage, Envelope, Event, EventEnvelope, Span, StackFrame, StackParser } from '@sentry/types'; -import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling'; +import type { + DebugImage, + Envelope, + Event, + EventEnvelope, + Profile, + Span, + StackFrame, + StackParser, + ThreadCpuProfile, +} from '@sentry/types'; import { GLOBAL_OBJ, browserPerformanceTimeOrigin, forEachEnvelopeItem, logger, uuid4 } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 88411c082106..9c25dc9750eb 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -9,14 +9,9 @@ import { startSession, } from '@sentry/core'; import type { DsnLike, Integration, Options, UserFeedback } from '@sentry/types'; -import { - addHistoryInstrumentationHandler, - consoleSandbox, - logger, - stackParserFromStackParserOptions, - supportsFetch, -} from '@sentry/utils'; +import { consoleSandbox, logger, stackParserFromStackParserOptions, supportsFetch } from '@sentry/utils'; +import { addHistoryInstrumentationHandler } from '@sentry-internal/browser-utils'; import { dedupeIntegration } from '@sentry/core'; import type { BrowserClientOptions, BrowserOptions } from './client'; import { BrowserClient } from './client'; diff --git a/packages/deno/src/integrations/breadcrumbs.ts b/packages/deno/src/integrations/breadcrumbs.ts index 58c75624b90d..47953d4d7ce8 100644 --- a/packages/deno/src/integrations/breadcrumbs.ts +++ b/packages/deno/src/integrations/breadcrumbs.ts @@ -1,6 +1,13 @@ import { addBreadcrumb, defineIntegration, getClient } from '@sentry/core'; -import type { Client, Event as SentryEvent, HandlerDataConsole, HandlerDataFetch, IntegrationFn } from '@sentry/types'; -import type { FetchBreadcrumbData, FetchBreadcrumbHint } from '@sentry/types/build/types/breadcrumb'; +import type { + Client, + Event as SentryEvent, + FetchBreadcrumbData, + FetchBreadcrumbHint, + HandlerDataConsole, + HandlerDataFetch, + IntegrationFn, +} from '@sentry/types'; import { addConsoleInstrumentationHandler, addFetchInstrumentationHandler, diff --git a/packages/replay-internal/src/coreHandlers/util/xhrUtils.ts b/packages/replay-internal/src/coreHandlers/util/xhrUtils.ts index 4ce349f1bb2e..fa504dcdeec2 100644 --- a/packages/replay-internal/src/coreHandlers/util/xhrUtils.ts +++ b/packages/replay-internal/src/coreHandlers/util/xhrUtils.ts @@ -1,5 +1,6 @@ +import { SENTRY_XHR_DATA_KEY } from '@sentry-internal/browser-utils'; import type { Breadcrumb, XhrBreadcrumbData } from '@sentry/types'; -import { SENTRY_XHR_DATA_KEY, logger } from '@sentry/utils'; +import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../../debug-build'; import type { diff --git a/packages/replay-internal/src/util/addGlobalListeners.ts b/packages/replay-internal/src/util/addGlobalListeners.ts index 164b054ba2ac..70344413d3ba 100644 --- a/packages/replay-internal/src/util/addGlobalListeners.ts +++ b/packages/replay-internal/src/util/addGlobalListeners.ts @@ -1,6 +1,9 @@ +import { + addClickKeypressInstrumentationHandler, + addHistoryInstrumentationHandler, +} from '@sentry-internal/browser-utils'; import { addEventProcessor, getClient } from '@sentry/core'; import type { DynamicSamplingContext } from '@sentry/types'; -import { addClickKeypressInstrumentationHandler, addHistoryInstrumentationHandler } from '@sentry/utils'; import { handleAfterSendEvent } from '../coreHandlers/handleAfterSendEvent'; import { handleBeforeSendEvent } from '../coreHandlers/handleBeforeSendEvent'; diff --git a/packages/replay-internal/test/integration/beforeAddRecordingEvent.test.ts b/packages/replay-internal/test/integration/beforeAddRecordingEvent.test.ts index c6b14d1f1bb8..a8331149838b 100644 --- a/packages/replay-internal/test/integration/beforeAddRecordingEvent.test.ts +++ b/packages/replay-internal/test/integration/beforeAddRecordingEvent.test.ts @@ -1,6 +1,6 @@ +import * as SentryBrowserUtils from '@sentry-internal/browser-utils'; import * as SentryCore from '@sentry/core'; import type { Transport } from '@sentry/types'; -import * as SentryUtils from '@sentry/utils'; import type { Replay } from '../../src/integration'; import type { ReplayContainer } from '../../src/replay'; @@ -31,7 +31,7 @@ describe('Integration | beforeAddRecordingEvent', () => { beforeAll(async () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); - jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { + jest.spyOn(SentryBrowserUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { domHandler = handler; }); diff --git a/packages/replay-internal/test/integration/flush.test.ts b/packages/replay-internal/test/integration/flush.test.ts index b40d6c78906d..ee4cf456fbf9 100644 --- a/packages/replay-internal/test/integration/flush.test.ts +++ b/packages/replay-internal/test/integration/flush.test.ts @@ -1,3 +1,4 @@ +import * as SentryBrowserUtils from '@sentry-internal/browser-utils'; import * as SentryUtils from '@sentry/utils'; import { DEFAULT_FLUSH_MIN_DELAY, MAX_REPLAY_DURATION, WINDOW } from '../../src/constants'; @@ -44,7 +45,7 @@ describe('Integration | flush', () => { let mockAddPerformanceEntries: MockAddPerformanceEntries; beforeAll(async () => { - jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { + jest.spyOn(SentryBrowserUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { domHandler = handler; }); diff --git a/packages/replay-internal/test/integration/sendReplayEvent.test.ts b/packages/replay-internal/test/integration/sendReplayEvent.test.ts index ff887c4b629b..58cdecaf1c65 100644 --- a/packages/replay-internal/test/integration/sendReplayEvent.test.ts +++ b/packages/replay-internal/test/integration/sendReplayEvent.test.ts @@ -1,6 +1,6 @@ +import * as SentryBrowserUtils from '@sentry-internal/browser-utils'; import * as SentryCore from '@sentry/core'; import type { Transport } from '@sentry/types'; -import * as SentryUtils from '@sentry/utils'; import { DEFAULT_FLUSH_MIN_DELAY, WINDOW } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; @@ -30,7 +30,7 @@ describe('Integration | sendReplayEvent', () => { beforeAll(async () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); - jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { + jest.spyOn(SentryBrowserUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { domHandler = handler; }); diff --git a/packages/replay-internal/test/integration/stop.test.ts b/packages/replay-internal/test/integration/stop.test.ts index d1af26e74776..3afbe3eeef1a 100644 --- a/packages/replay-internal/test/integration/stop.test.ts +++ b/packages/replay-internal/test/integration/stop.test.ts @@ -1,4 +1,4 @@ -import * as SentryUtils from '@sentry/utils'; +import * as SentryBrowserUtils from '@sentry-internal/browser-utils'; import { WINDOW } from '../../src/constants'; import type { Replay } from '../../src/integration'; @@ -27,7 +27,7 @@ describe('Integration | stop', () => { beforeAll(async () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); - mockAddDomInstrumentationHandler = jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler'); + mockAddDomInstrumentationHandler = jest.spyOn(SentryBrowserUtils, 'addClickKeypressInstrumentationHandler'); ({ replay, integration } = await mockSdk()); diff --git a/packages/replay-internal/test/mocks/resetSdkMock.ts b/packages/replay-internal/test/mocks/resetSdkMock.ts index 3596ad4d29d2..47a1522a4c63 100644 --- a/packages/replay-internal/test/mocks/resetSdkMock.ts +++ b/packages/replay-internal/test/mocks/resetSdkMock.ts @@ -23,8 +23,8 @@ export async function resetSdkMock({ replayOptions, sentryOptions, autoStart }: // Clear all handlers that have been registered resetInstrumentationHandlers(); - const SentryUtils = await import('@sentry/utils'); - jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { + const SentryBrowserUtils = await import('@sentry-internal/browser-utils'); + jest.spyOn(SentryBrowserUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { domHandler = handler; }); const { mockRrweb } = await import('./mockRrweb'); diff --git a/packages/replay-internal/test/unit/coreHandlers/handleNetworkBreadcrumbs.test.ts b/packages/replay-internal/test/unit/coreHandlers/handleNetworkBreadcrumbs.test.ts index de16549d081e..b3522c0ceb6c 100644 --- a/packages/replay-internal/test/unit/coreHandlers/handleNetworkBreadcrumbs.test.ts +++ b/packages/replay-internal/test/unit/coreHandlers/handleNetworkBreadcrumbs.test.ts @@ -1,3 +1,4 @@ +import { SENTRY_XHR_DATA_KEY } from '@sentry-internal/browser-utils'; import type { Breadcrumb, BreadcrumbHint, @@ -5,7 +6,6 @@ import type { SentryWrappedXMLHttpRequest, XhrBreadcrumbHint, } from '@sentry/types'; -import { SENTRY_XHR_DATA_KEY } from '@sentry/utils'; import { BASE_TIMESTAMP } from '../..'; import { NETWORK_BODY_MAX_SIZE } from '../../../src/constants'; diff --git a/packages/utils/src/instrument/console.ts b/packages/utils/src/instrument/console.ts index 7570f58a55dc..f75faa3346c2 100644 --- a/packages/utils/src/instrument/console.ts +++ b/packages/utils/src/instrument/console.ts @@ -5,7 +5,7 @@ import type { ConsoleLevel, HandlerDataConsole } from '@sentry/types'; import { CONSOLE_LEVELS, originalConsoleMethods } from '../logger'; import { fill } from '../object'; import { GLOBAL_OBJ } from '../worldwide'; -import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; +import { addHandler, maybeInstrument, triggerHandlers } from './handlers'; /** * Add an instrumentation handler for when a console.xxx method is called. diff --git a/packages/utils/src/instrument/fetch.ts b/packages/utils/src/instrument/fetch.ts index 472f48283a47..9c663f88baf0 100644 --- a/packages/utils/src/instrument/fetch.ts +++ b/packages/utils/src/instrument/fetch.ts @@ -4,7 +4,7 @@ import type { HandlerDataFetch } from '@sentry/types'; import { fill } from '../object'; import { supportsNativeFetch } from '../supports'; import { GLOBAL_OBJ } from '../worldwide'; -import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; +import { addHandler, maybeInstrument, triggerHandlers } from './handlers'; type FetchResource = string | { toString(): string } | { url: string }; diff --git a/packages/utils/src/instrument/globalError.ts b/packages/utils/src/instrument/globalError.ts index 0bf50c52f2c4..a3ee2149817f 100644 --- a/packages/utils/src/instrument/globalError.ts +++ b/packages/utils/src/instrument/globalError.ts @@ -1,7 +1,7 @@ import type { HandlerDataError } from '@sentry/types'; import { GLOBAL_OBJ } from '../worldwide'; -import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; +import { addHandler, maybeInstrument, triggerHandlers } from './handlers'; let _oldOnErrorHandler: (typeof GLOBAL_OBJ)['onerror'] | null = null; diff --git a/packages/utils/src/instrument/globalUnhandledRejection.ts b/packages/utils/src/instrument/globalUnhandledRejection.ts index c92f4d723b0d..878540500dac 100644 --- a/packages/utils/src/instrument/globalUnhandledRejection.ts +++ b/packages/utils/src/instrument/globalUnhandledRejection.ts @@ -3,7 +3,7 @@ import type { HandlerDataUnhandledRejection } from '@sentry/types'; import { GLOBAL_OBJ } from '../worldwide'; -import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; +import { addHandler, maybeInstrument, triggerHandlers } from './handlers'; let _oldOnUnhandledRejectionHandler: (typeof GLOBAL_OBJ)['onunhandledrejection'] | null = null; diff --git a/packages/utils/src/instrument/_handlers.ts b/packages/utils/src/instrument/handlers.ts similarity index 100% rename from packages/utils/src/instrument/_handlers.ts rename to packages/utils/src/instrument/handlers.ts diff --git a/packages/utils/src/instrument/index.ts b/packages/utils/src/instrument/index.ts index 40f2f0fe917c..ee84f1375522 100644 --- a/packages/utils/src/instrument/index.ts +++ b/packages/utils/src/instrument/index.ts @@ -1,23 +1,17 @@ -// TODO(v8): Consider moving this file (or at least parts of it) into the browser package. The registered handlers are mostly non-generic and we risk leaking runtime specific code into generic packages. - -import { resetInstrumentationHandlers } from './_handlers'; import { addConsoleInstrumentationHandler } from './console'; -import { addClickKeypressInstrumentationHandler } from './dom'; import { addFetchInstrumentationHandler } from './fetch'; import { addGlobalErrorInstrumentationHandler } from './globalError'; import { addGlobalUnhandledRejectionInstrumentationHandler } from './globalUnhandledRejection'; -import { addHistoryInstrumentationHandler } from './history'; -import { SENTRY_XHR_DATA_KEY, addXhrInstrumentationHandler } from './xhr'; +import { addHandler, maybeInstrument, resetInstrumentationHandlers, triggerHandlers } from './handlers'; export { addConsoleInstrumentationHandler, - addClickKeypressInstrumentationHandler, - addXhrInstrumentationHandler, addFetchInstrumentationHandler, - addHistoryInstrumentationHandler, addGlobalErrorInstrumentationHandler, addGlobalUnhandledRejectionInstrumentationHandler, - SENTRY_XHR_DATA_KEY, + addHandler, + maybeInstrument, + triggerHandlers, // Only exported for tests resetInstrumentationHandlers, }; From 0ad046a8a4d273a42f4b6f93a3cbbc8b099146c7 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 8 Apr 2024 09:26:33 -0400 Subject: [PATCH 07/64] build: Reorganize size limit and restructure limits (#11454) Next week I'm going to play around with optimizing feedback/replay packages as per what we saw in https://github.com/getsentry/sentry-javascript/pull/11452. In prep for that work I re-organized size limit and made the limits a little more sensible. The size bot comment will be a little messed up because we renamed the fields, but after we merge should be a lot better. --- .size-limit.js | 217 ++++++++++++++++++++++++++++++------------------- 1 file changed, 132 insertions(+), 85 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index 68e7f534efa6..7fd6db3be736 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -1,32 +1,32 @@ module.exports = [ - // Main browser webpack builds + // Browser SDK (ESM) { - name: '@sentry/browser (incl. Tracing, Replay, Feedback)', + name: '@sentry/browser', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, replayIntegration, browserTracingIntegration, feedbackIntegration }', + import: createImport('init'), gzip: true, - limit: '90 KB', + limit: '24 KB', }, { - name: '@sentry/browser (incl. Tracing, Replay)', + name: '@sentry/browser (incl. Tracing)', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, replayIntegration, browserTracingIntegration }', + import: createImport('init', 'browserTracingIntegration'), gzip: true, - limit: '90 KB', + limit: '34 KB', }, { - name: '@sentry/browser (incl. Tracing, Replay with Canvas)', + name: '@sentry/browser (incl. Tracing, Replay)', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, replayIntegration, browserTracingIntegration, replayCanvasIntegration }', + import: createImport('init', 'browserTracingIntegration', 'replayIntegration'), gzip: true, - limit: '90 KB', + limit: '70 KB', }, { name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, replayIntegration, browserTracingIntegration }', + import: createImport('init', 'browserTracingIntegration', 'replayIntegration'), gzip: true, - limit: '75 KB', + limit: '65 KB', modifyWebpackConfig: function (config) { const webpack = require('webpack'); config.plugins.push( @@ -41,134 +41,181 @@ module.exports = [ }, }, { - name: '@sentry/browser (incl. Tracing)', + name: '@sentry/browser (incl. Tracing, Replay with Canvas)', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, browserTracingIntegration }', + import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'), gzip: true, - limit: '90 KB', + limit: '75 KB', }, { - name: '@sentry/browser (incl. browserTracingIntegration)', + name: '@sentry/browser (incl. Tracing, Replay, Feedback)', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, browserTracingIntegration }', + import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'feedbackIntegration'), gzip: true, - limit: '90 KB', + limit: '80 KB', }, { - name: '@sentry/browser (incl. feedbackIntegration)', + name: '@sentry/browser (incl. Feedback)', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, feedbackIntegration }', + import: createImport('init', 'feedbackIntegration'), gzip: true, - limit: '90 KB', + limit: '35 KB', }, { - name: '@sentry/browser (incl. feedbackModalIntegration)', + name: '@sentry/browser (incl. Feedback, Feedback Modal)', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, feedbackIntegration, feedbackModalIntegration }', + import: createImport('init', 'feedbackIntegration', 'feedbackModalIntegration'), gzip: true, - limit: '90 KB', + limit: '35 KB', }, { - name: '@sentry/browser (incl. feedbackScreenshotIntegration)', + name: '@sentry/browser (incl. Feedback, Feedback Modal, Feedback Screenshot)', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, feedbackIntegration, feedbackModalIntegration, feedbackScreenshotIntegration }', + import: createImport('init', 'feedbackIntegration', 'feedbackModalIntegration', 'feedbackScreenshotIntegration'), gzip: true, - limit: '90 KB', + limit: '35 KB', }, { name: '@sentry/browser (incl. sendFeedback)', path: 'packages/browser/build/npm/esm/index.js', - import: '{ init, sendFeedback }', + import: createImport('init', 'sendFeedback'), gzip: true, - limit: '90 KB', + limit: '30 KB', }, + // React SDK (ESM) { - name: '@sentry/browser', - path: 'packages/browser/build/npm/esm/index.js', - import: '{ init }', + name: '@sentry/react', + path: 'packages/react/build/esm/index.js', + import: createImport('init', 'ErrorBoundary'), gzip: true, - limit: '90 KB', + limit: '27 KB', }, - - // Browser CDN bundles { - name: 'CDN Bundle (incl. Tracing, Replay, Feedback)', - path: 'packages/browser/build/bundles/bundle.tracing.replay.feedback.min.js', + name: '@sentry/react (incl. Tracing)', + path: 'packages/react/build/esm/index.js', + import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'), gzip: true, - limit: '90 KB', + limit: '37 KB', }, + // Vue SDK (ESM) { - name: 'CDN Bundle (incl. Tracing, Replay)', - path: 'packages/browser/build/bundles/bundle.tracing.replay.min.js', + name: '@sentry/vue', + path: 'packages/vue/build/esm/index.js', + import: createImport('init'), gzip: true, - limit: '90 KB', + limit: '28 KB', }, { - name: 'CDN Bundle (incl. Tracing)', - path: 'packages/browser/build/bundles/bundle.tracing.min.js', + name: '@sentry/vue (incl. Tracing)', + path: 'packages/vue/build/esm/index.js', + import: createImport('init', 'browserTracingIntegration'), + gzip: true, + limit: '38 KB', + }, + // Svelte SDK (ESM) + { + name: '@sentry/svelte', + path: 'packages/svelte/build/esm/index.js', + import: createImport('init'), gzip: true, - limit: '40 KB', + limit: '24 KB', }, + // Browser CDN bundles { name: 'CDN Bundle', - path: 'packages/browser/build/bundles/bundle.min.js', + path: createCDNPath('bundle.min.js'), gzip: true, - limit: '30 KB', + limit: '26 KB', + }, + { + name: 'CDN Bundle (incl. Tracing)', + path: createCDNPath('bundle.tracing.min.js'), + gzip: true, + limit: '36 KB', + }, + { + name: 'CDN Bundle (incl. Tracing, Replay)', + path: createCDNPath('bundle.tracing.replay.min.js'), + gzip: true, + limit: '70 KB', + }, + { + name: 'CDN Bundle (incl. Tracing, Replay, Feedback)', + path: createCDNPath('bundle.tracing.replay.feedback.min.js'), + gzip: true, + limit: '75 KB', }, - // browser CDN bundles (non-gzipped) { - name: 'CDN Bundle (incl. Tracing, Replay) - uncompressed', - path: 'packages/browser/build/bundles/bundle.tracing.replay.min.js', + name: 'CDN Bundle - uncompressed', + path: createCDNPath('bundle.min.js'), gzip: false, brotli: false, - limit: '260 KB', + limit: '80 KB', }, { name: 'CDN Bundle (incl. Tracing) - uncompressed', - path: 'packages/browser/build/bundles/bundle.tracing.min.js', + path: createCDNPath('bundle.tracing.min.js'), gzip: false, brotli: false, - limit: '120 KB', + limit: '105 KB', }, { - name: 'CDN Bundle - uncompressed', - path: 'packages/browser/build/bundles/bundle.min.js', + name: 'CDN Bundle (incl. Tracing, Replay) - uncompressed', + path: createCDNPath('bundle.tracing.replay.min.js'), gzip: false, brotli: false, - limit: '80 KB', - }, - - // React - { - name: '@sentry/react (incl. Tracing, Replay)', - path: 'packages/react/build/esm/index.js', - import: '{ init, browserTracingIntegration, replayIntegration }', - gzip: true, - limit: '90 KB', + limit: '220 KB', }, + // Next.js SDK (ESM) { - name: '@sentry/react', - path: 'packages/react/build/esm/index.js', - import: '{ init }', - gzip: true, - limit: '90 KB', - }, - - // Next.js - // TODO: Re-enable these, when we figure out why they break... - /* { - name: '@sentry/nextjs Client (incl. Tracing, Replay)', + name: '@sentry/nextjs (client)', path: 'packages/nextjs/build/esm/client/index.js', - import: '{ init, browserTracingIntegration, replayIntegration }', - gzip: true, - limit: '110 KB', + import: createImport('init'), + ignore: ['next/router', 'next/constants'], + gzip: true, + limit: '37 KB', + }, + // SvelteKit SDK (ESM) + { + name: '@sentry/sveltekit (client)', + path: 'packages/sveltekit/build/esm/client/index.js', + import: createImport('init'), + ignore: ['$app/stores'], + gzip: true, + limit: '37 KB', + }, + // Node SDK (ESM) + { + name: '@sentry/node', + path: 'packages/node/build/esm/index.js', + import: createImport('init'), + ignore: [ + 'node:http', + 'node:https', + 'node:diagnostics_channel', + 'async_hooks', + 'child_process', + 'fs', + 'os', + 'path', + 'inspector', + 'worker_threads', + 'http', + 'stream', + 'zlib', + 'net', + 'tls', + ], + gzip: true, + limit: '150 KB', }, - { - name: '@sentry/nextjs Client', - path: 'packages/nextjs/build/esm/client/index.js', - import: '{ init }', - gzip: true, - limit: '57 KB', - }, */ ]; + +function createImport(...args) { + return `{ ${args.join(', ')} }`; +} + +function createCDNPath(name) { + return `packages/browser/build/bundles/${name}`; +} From e4436943e6f075b2660f6c6482d8edd39a9810a2 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 8 Apr 2024 15:26:59 +0200 Subject: [PATCH 08/64] fix(test): Some ANR tests should test the transport (#11465) All of the ANR tests were verified over stdout. This PR ensures some of the basic tests actually send events over http to ensure the transport is fully tested. --- .../suites/anr/basic-session.js | 3 +-- .../node-integration-tests/suites/anr/basic.js | 3 +-- .../node-integration-tests/suites/anr/basic.mjs | 3 +-- .../suites/anr/isolated.mjs | 3 +-- .../node-integration-tests/suites/anr/test.ts | 16 ++++++++++++---- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/anr/basic-session.js b/dev-packages/node-integration-tests/suites/anr/basic-session.js index 5661e08b850b..c6415b6358da 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic-session.js +++ b/dev-packages/node-integration-tests/suites/anr/basic-session.js @@ -8,9 +8,8 @@ setTimeout(() => { }, 10000); Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', + dsn: process.env.SENTRY_DSN, release: '1.0', - debug: true, integrations: [Sentry.anrIntegration({ captureStackTrace: true, anrThreshold: 100 })], autoSessionTracking: true, }); diff --git a/dev-packages/node-integration-tests/suites/anr/basic.js b/dev-packages/node-integration-tests/suites/anr/basic.js index d98b18216703..b1dddf958d46 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic.js +++ b/dev-packages/node-integration-tests/suites/anr/basic.js @@ -8,9 +8,8 @@ setTimeout(() => { }, 10000); Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', + dsn: process.env.SENTRY_DSN, release: '1.0', - debug: true, autoSessionTracking: false, integrations: [Sentry.anrIntegration({ captureStackTrace: true, anrThreshold: 100 })], }); diff --git a/dev-packages/node-integration-tests/suites/anr/basic.mjs b/dev-packages/node-integration-tests/suites/anr/basic.mjs index 77bb9ae3626d..c3e74222f587 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic.mjs +++ b/dev-packages/node-integration-tests/suites/anr/basic.mjs @@ -8,9 +8,8 @@ setTimeout(() => { }, 10000); Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', + dsn: process.env.SENTRY_DSN, release: '1.0', - debug: true, autoSessionTracking: false, integrations: [Sentry.anrIntegration({ captureStackTrace: true, anrThreshold: 100 })], }); diff --git a/dev-packages/node-integration-tests/suites/anr/isolated.mjs b/dev-packages/node-integration-tests/suites/anr/isolated.mjs index d9b179c63e71..2f36575fbbd2 100644 --- a/dev-packages/node-integration-tests/suites/anr/isolated.mjs +++ b/dev-packages/node-integration-tests/suites/anr/isolated.mjs @@ -8,9 +8,8 @@ setTimeout(() => { }, 10000); Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', + dsn: process.env.SENTRY_DSN, release: '1.0', - debug: true, autoSessionTracking: false, integrations: [Sentry.anrIntegration({ captureStackTrace: true, anrThreshold: 100 })], }); diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index b0299f4a038d..6af5d46bfb50 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -66,15 +66,19 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => }); test('CJS', done => { - createRunner(__dirname, 'basic.js').expect({ event: EXPECTED_ANR_EVENT }).start(done); + createRunner(__dirname, 'basic.js').withMockSentryServer().expect({ event: EXPECTED_ANR_EVENT }).start(done); }); test('ESM', done => { - createRunner(__dirname, 'basic.mjs').expect({ event: EXPECTED_ANR_EVENT }).start(done); + createRunner(__dirname, 'basic.mjs').withMockSentryServer().expect({ event: EXPECTED_ANR_EVENT }).start(done); }); test('With --inspect', done => { - createRunner(__dirname, 'basic.mjs').withFlags('--inspect').expect({ event: EXPECTED_ANR_EVENT }).start(done); + createRunner(__dirname, 'basic.mjs') + .withMockSentryServer() + .withFlags('--inspect') + .expect({ event: EXPECTED_ANR_EVENT }) + .start(done); }); test('should exit', done => { @@ -97,6 +101,7 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => test('With session', done => { createRunner(__dirname, 'basic-session.js') + .withMockSentryServer() .expect({ session: { status: 'abnormal', @@ -142,6 +147,9 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => }; test('fetches correct isolated scope', done => { - createRunner(__dirname, 'isolated.mjs').expect({ event: EXPECTED_ISOLATED_EVENT }).start(done); + createRunner(__dirname, 'isolated.mjs') + .withMockSentryServer() + .expect({ event: EXPECTED_ISOLATED_EVENT }) + .start(done); }); }); From 4239d2f172ba12b918cf7241a6e1cc125c1f61b1 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 8 Apr 2024 15:36:57 +0200 Subject: [PATCH 09/64] fix(core): Don't log error from Inbound Filters integration (#11473) Closes #11462 --- packages/core/src/integrations/inboundfilters.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 9c64476568c7..66eafcf4db40 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -155,10 +155,6 @@ function _getPossibleEventMessages(event: Event): string[] { } } - if (DEBUG_BUILD && possibleMessages.length === 0) { - logger.error(`Could not extract message for event ${getEventDescription(event)}`); - } - return possibleMessages; } From d7c6fe3fefb9fedffb015cefec070e8e86dadf0c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 Apr 2024 15:41:25 +0200 Subject: [PATCH 10/64] feat: Export `spanToJSON` and `spanToTraceHeader` from all relevant packages (#11474) --- packages/aws-serverless/src/index.ts | 1 + packages/browser/src/exports.ts | 1 + packages/bun/src/index.ts | 1 + packages/deno/src/index.ts | 2 ++ packages/google-cloud-serverless/src/index.ts | 1 + packages/node/src/index.ts | 1 + packages/remix/src/index.server.ts | 2 ++ packages/sveltekit/src/server/index.ts | 2 ++ packages/vercel-edge/src/index.ts | 2 ++ 9 files changed, 13 insertions(+) diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 68e0a587fb0c..5cf5a9375462 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -93,6 +93,7 @@ export { spotlightIntegration, initOpenTelemetry, spanToJSON, + spanToTraceHeader, trpcMiddleware, } from '@sentry/node'; diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 2efa7f8ec6d6..fa38e5373110 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -56,6 +56,7 @@ export { captureSession, endSession, spanToJSON, + spanToTraceHeader, } from '@sentry/core'; export { diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index db54dcdd6fb5..a91dbd48b6f9 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -113,6 +113,7 @@ export { spotlightIntegration, initOpenTelemetry, spanToJSON, + spanToTraceHeader, trpcMiddleware, } from '@sentry/node'; diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index a78a2392a429..a554d807b418 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -74,6 +74,8 @@ export { startSession, captureSession, endSession, + spanToJSON, + spanToTraceHeader, } from '@sentry/core'; export { DenoClient } from './client'; diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index d07dab055ae2..2bf45cf22b98 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -93,6 +93,7 @@ export { spotlightIntegration, initOpenTelemetry, spanToJSON, + spanToTraceHeader, trpcMiddleware, } from '@sentry/node'; diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 0960d1a12762..818596d0c4fe 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -102,6 +102,7 @@ export { withActiveSpan, getRootSpan, spanToJSON, + spanToTraceHeader, trpcMiddleware, } from '@sentry/core'; diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index ff3d0eb5c51b..41db9c117147 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -96,6 +96,8 @@ export { spotlightIntegration, setupFastifyErrorHandler, trpcMiddleware, + spanToJSON, + spanToTraceHeader, } from '@sentry/node'; // Keeping the `*` exports for backwards compatibility and types diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 066ede9295aa..4cba1a3e825e 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -70,6 +70,8 @@ export { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, trpcMiddleware, + spanToJSON, + spanToTraceHeader, } from '@sentry/node'; // We can still leave this for the carrier init and type exports diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index 58fdc6bdd9c4..466c122b7ca6 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -69,6 +69,8 @@ export { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, trpcMiddleware, + spanToJSON, + spanToTraceHeader, } from '@sentry/core'; export { VercelEdgeClient } from './client'; From f644fd6d515de71bb72b7be9dacb6cef731d1fd9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 8 Apr 2024 14:44:51 +0100 Subject: [PATCH 11/64] feat(core): Allow to pass scope to `withIsolationScope()` (#11478) Also adds tests for this for core, vercel-edge and opentelemetry. NOTE: In core, this does not do anything, as we cannot actually update the isolation scope in browser. Co-authored-by: Luca Forstner --- packages/core/src/currentScopes.ts | 39 ++++- packages/core/test/lib/scope.test.ts | 144 ++++++++++++++--- .../test/asyncContextStrategy.test.ts | 129 +++++++++++++++ packages/vercel-edge/src/async.ts | 6 +- packages/vercel-edge/test/async.test.ts | 150 ++++++++++++++++++ 5 files changed, 436 insertions(+), 32 deletions(-) create mode 100644 packages/vercel-edge/test/async.test.ts diff --git a/packages/core/src/currentScopes.ts b/packages/core/src/currentScopes.ts index 1e68c9583372..e0b598c1ad39 100644 --- a/packages/core/src/currentScopes.ts +++ b/packages/core/src/currentScopes.ts @@ -74,15 +74,44 @@ export function withScope( * * This function is intended for Sentry SDK and SDK integration development. It is not recommended to be used in "normal" * applications directly because it comes with pitfalls. Use at your own risk! + */ +export function withIsolationScope(callback: (isolationScope: Scope) => T): T; +/** + * Set the provided isolation scope as active in the given callback. If no + * async context strategy is set, the isolation scope and the current scope will not be forked (this is currently the + * case, for example, in the browser). + * + * Usage of this function in environments without async context strategy is discouraged and may lead to unexpected behaviour. * - * @param callback The callback in which the passed isolation scope is active. (Note: In environments without async - * context strategy, the currently active isolation scope may change within execution of the callback.) - * @returns The same value that `callback` returns. + * This function is intended for Sentry SDK and SDK integration development. It is not recommended to be used in "normal" + * applications directly because it comes with pitfalls. Use at your own risk! + * + * If you pass in `undefined` as a scope, it will fork a new isolation scope, the same as if no scope is passed. + */ +export function withIsolationScope(isolationScope: Scope | undefined, callback: (isolationScope: Scope) => T): T; +/** + * Either creates a new active isolation scope, or sets the given isolation scope as active scope in the given callback. */ -export function withIsolationScope(callback: (isolationScope: Scope) => T): T { +export function withIsolationScope( + ...rest: + | [callback: (isolationScope: Scope) => T] + | [isolationScope: Scope | undefined, callback: (isolationScope: Scope) => T] +): T { const carrier = getMainCarrier(); const acs = getAsyncContextStrategy(carrier); - return acs.withIsolationScope(callback); + + // If a scope is defined, we want to make this the active scope instead of the default one + if (rest.length === 2) { + const [isolationScope, callback] = rest; + + if (!isolationScope) { + return acs.withIsolationScope(callback); + } + + return acs.withSetIsolationScope(isolationScope, callback); + } + + return acs.withIsolationScope(rest[0]); } /** diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts index bbd40af742d1..aadc26856c6e 100644 --- a/packages/core/test/lib/scope.test.ts +++ b/packages/core/test/lib/scope.test.ts @@ -5,6 +5,7 @@ import { getGlobalScope, getIsolationScope, withIsolationScope, + withScope, } from '../../src'; import { Scope } from '../../src/scope'; @@ -853,40 +854,135 @@ describe('Scope', () => { }); }); -describe('isolation scope', () => { - describe('withIsolationScope()', () => { - it('will pass an isolation scope without Sentry.init()', done => { - expect.assertions(1); - withIsolationScope(scope => { - expect(scope).toBeDefined(); - done(); - }); +describe('withScope()', () => { + beforeEach(() => { + getIsolationScope().clear(); + getCurrentScope().clear(); + getGlobalScope().clear(); + }); + + it('will make the passed scope the active scope within the callback', done => { + withScope(scope => { + expect(getCurrentScope()).toBe(scope); + done(); + }); + }); + + it('will pass a scope that is different from the current active isolation scope', done => { + withScope(scope => { + expect(getIsolationScope()).not.toBe(scope); + done(); }); + }); - it('will make the passed isolation scope the active isolation scope within the callback', done => { - expect.assertions(1); - withIsolationScope(scope => { - expect(getIsolationScope()).toBe(scope); + it('will always make the inner most passed scope the current isolation scope when nesting calls', done => { + withIsolationScope(_scope1 => { + withIsolationScope(scope2 => { + expect(getIsolationScope()).toBe(scope2); done(); }); }); + }); + + it('forks the scope when not passing any scope', done => { + const initialScope = getCurrentScope(); + initialScope.setTag('aa', 'aa'); - it('will pass an isolation scope that is different from the current active scope', done => { - expect.assertions(1); - withIsolationScope(scope => { - expect(getCurrentScope()).not.toBe(scope); + withScope(scope => { + expect(getCurrentScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('forks the scope when passing undefined', done => { + const initialScope = getCurrentScope(); + initialScope.setTag('aa', 'aa'); + + withScope(undefined, scope => { + expect(getCurrentScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('sets the passed in scope as active scope', done => { + const initialScope = getCurrentScope(); + initialScope.setTag('aa', 'aa'); + + const customScope = new Scope(); + + withScope(customScope, scope => { + expect(getCurrentScope()).toBe(customScope); + expect(scope).toBe(customScope); + done(); + }); + }); +}); + +describe('withIsolationScope()', () => { + beforeEach(() => { + getIsolationScope().clear(); + getCurrentScope().clear(); + getGlobalScope().clear(); + }); + + it('will make the passed isolation scope the active isolation scope within the callback', done => { + withIsolationScope(scope => { + expect(getIsolationScope()).toBe(scope); + done(); + }); + }); + + it('will pass an isolation scope that is different from the current active scope', done => { + withIsolationScope(scope => { + expect(getCurrentScope()).not.toBe(scope); + done(); + }); + }); + + it('will always make the inner most passed scope the current scope when nesting calls', done => { + withIsolationScope(_scope1 => { + withIsolationScope(scope2 => { + expect(getIsolationScope()).toBe(scope2); done(); }); }); + }); + + // Note: This is expected! In browser, we do not actually fork this + it('does not fork isolation scope when not passing any isolation scope', done => { + const isolationScope = getIsolationScope(); + + withIsolationScope(scope => { + expect(getIsolationScope()).toBe(scope); + expect(scope).toBe(isolationScope); + done(); + }); + }); - it('will always make the inner most passed scope the current scope when nesting calls', done => { - expect.assertions(1); - withIsolationScope(_scope1 => { - withIsolationScope(scope2 => { - expect(getIsolationScope()).toBe(scope2); - done(); - }); - }); + it('does not fork isolation scope when passing undefined', done => { + const isolationScope = getIsolationScope(); + + withIsolationScope(undefined, scope => { + expect(getIsolationScope()).toBe(scope); + expect(scope).toBe(isolationScope); + done(); + }); + }); + + it('ignores passed in isolation scope', done => { + const isolationScope = getIsolationScope(); + const customIsolationScope = new Scope(); + + withIsolationScope(customIsolationScope, scope => { + expect(getIsolationScope()).toBe(isolationScope); + expect(scope).toBe(isolationScope); + done(); }); }); }); diff --git a/packages/opentelemetry/test/asyncContextStrategy.test.ts b/packages/opentelemetry/test/asyncContextStrategy.test.ts index 16eea942655e..f3b664fcaf44 100644 --- a/packages/opentelemetry/test/asyncContextStrategy.test.ts +++ b/packages/opentelemetry/test/asyncContextStrategy.test.ts @@ -1,5 +1,6 @@ import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base'; import { + Scope as ScopeClass, getCurrentScope, getIsolationScope, setAsyncContextStrategy, @@ -298,4 +299,132 @@ describe('asyncContextStrategy', () => { }); }); }); + + describe('withScope()', () => { + it('will make the passed scope the active scope within the callback', done => { + withScope(scope => { + expect(getCurrentScope()).toBe(scope); + done(); + }); + }); + + it('will pass a scope that is different from the current active isolation scope', done => { + withScope(scope => { + expect(getIsolationScope()).not.toBe(scope); + done(); + }); + }); + + it('will always make the inner most passed scope the current scope when nesting calls', done => { + withIsolationScope(_scope1 => { + withIsolationScope(scope2 => { + expect(getIsolationScope()).toBe(scope2); + done(); + }); + }); + }); + + it('forks the scope when not passing any scope', done => { + const initialScope = getCurrentScope(); + initialScope.setTag('aa', 'aa'); + + withScope(scope => { + expect(getCurrentScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('forks the scope when passing undefined', done => { + const initialScope = getCurrentScope(); + initialScope.setTag('aa', 'aa'); + + withScope(undefined, scope => { + expect(getCurrentScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('sets the passed in scope as active scope', done => { + const initialScope = getCurrentScope(); + initialScope.setTag('aa', 'aa'); + + const customScope = new ScopeClass(); + + withScope(customScope, scope => { + expect(getCurrentScope()).toBe(customScope); + expect(scope).toBe(customScope); + done(); + }); + }); + }); + + describe('withIsolationScope()', () => { + it('will make the passed isolation scope the active isolation scope within the callback', done => { + withIsolationScope(scope => { + expect(getIsolationScope()).toBe(scope); + done(); + }); + }); + + it('will pass an isolation scope that is different from the current active scope', done => { + withIsolationScope(scope => { + expect(getCurrentScope()).not.toBe(scope); + done(); + }); + }); + + it('will always make the inner most passed scope the current scope when nesting calls', done => { + withIsolationScope(_scope1 => { + withIsolationScope(scope2 => { + expect(getIsolationScope()).toBe(scope2); + done(); + }); + }); + }); + + it('forks the isolation scope when not passing any isolation scope', done => { + const initialScope = getIsolationScope(); + initialScope.setTag('aa', 'aa'); + + withIsolationScope(scope => { + expect(getIsolationScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('forks the isolation scope when passing undefined', done => { + const initialScope = getIsolationScope(); + initialScope.setTag('aa', 'aa'); + + withIsolationScope(undefined, scope => { + expect(getIsolationScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('sets the passed in isolation scope as active isolation scope', done => { + const initialScope = getIsolationScope(); + initialScope.setTag('aa', 'aa'); + + const customScope = new ScopeClass(); + + withIsolationScope(customScope, scope => { + expect(getIsolationScope()).toBe(customScope); + expect(scope).toBe(customScope); + done(); + }); + }); + }); }); diff --git a/packages/vercel-edge/src/async.ts b/packages/vercel-edge/src/async.ts index 11b6b790df1a..50e2d80ad652 100644 --- a/packages/vercel-edge/src/async.ts +++ b/packages/vercel-edge/src/async.ts @@ -11,15 +11,15 @@ interface AsyncLocalStorage { run(store: T, callback: (...args: TArgs) => R, ...args: TArgs): R; } -// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any -const MaybeGlobalAsyncLocalStorage = (GLOBAL_OBJ as any).AsyncLocalStorage; - let asyncStorage: AsyncLocalStorage; /** * Sets the async context strategy to use AsyncLocalStorage which should be available in the edge runtime. */ export function setAsyncLocalStorageAsyncContextStrategy(): void { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any + const MaybeGlobalAsyncLocalStorage = (GLOBAL_OBJ as any).AsyncLocalStorage; + if (!MaybeGlobalAsyncLocalStorage) { DEBUG_BUILD && logger.warn( diff --git a/packages/vercel-edge/test/async.test.ts b/packages/vercel-edge/test/async.test.ts new file mode 100644 index 000000000000..37b357ab4910 --- /dev/null +++ b/packages/vercel-edge/test/async.test.ts @@ -0,0 +1,150 @@ +import { Scope, getCurrentScope, getGlobalScope, getIsolationScope, withIsolationScope, withScope } from '@sentry/core'; +import { GLOBAL_OBJ } from '@sentry/utils'; +import { AsyncLocalStorage } from 'async_hooks'; +import { setAsyncLocalStorageAsyncContextStrategy } from '../src/async'; + +describe('withScope()', () => { + beforeEach(() => { + getIsolationScope().clear(); + getCurrentScope().clear(); + getGlobalScope().clear(); + + (GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage; + setAsyncLocalStorageAsyncContextStrategy(); + }); + + it('will make the passed scope the active scope within the callback', done => { + withScope(scope => { + expect(getCurrentScope()).toBe(scope); + done(); + }); + }); + + it('will pass a scope that is different from the current active isolation scope', done => { + withScope(scope => { + expect(getIsolationScope()).not.toBe(scope); + done(); + }); + }); + + it('will always make the inner most passed scope the current scope when nesting calls', done => { + withIsolationScope(_scope1 => { + withIsolationScope(scope2 => { + expect(getIsolationScope()).toBe(scope2); + done(); + }); + }); + }); + + it('forks the scope when not passing any scope', done => { + const initialScope = getCurrentScope(); + initialScope.setTag('aa', 'aa'); + + withScope(scope => { + expect(getCurrentScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('forks the scope when passing undefined', done => { + const initialScope = getCurrentScope(); + initialScope.setTag('aa', 'aa'); + + withScope(undefined, scope => { + expect(getCurrentScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('sets the passed in scope as active scope', done => { + const initialScope = getCurrentScope(); + initialScope.setTag('aa', 'aa'); + + const customScope = new Scope(); + + withScope(customScope, scope => { + expect(getCurrentScope()).toBe(customScope); + expect(scope).toBe(customScope); + done(); + }); + }); +}); + +describe('withIsolationScope()', () => { + beforeEach(() => { + getIsolationScope().clear(); + getCurrentScope().clear(); + getGlobalScope().clear(); + (GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage; + + setAsyncLocalStorageAsyncContextStrategy(); + }); + + it('will make the passed isolation scope the active isolation scope within the callback', done => { + withIsolationScope(scope => { + expect(getIsolationScope()).toBe(scope); + done(); + }); + }); + + it('will pass an isolation scope that is different from the current active scope', done => { + withIsolationScope(scope => { + expect(getCurrentScope()).not.toBe(scope); + done(); + }); + }); + + it('will always make the inner most passed scope the current scope when nesting calls', done => { + withIsolationScope(_scope1 => { + withIsolationScope(scope2 => { + expect(getIsolationScope()).toBe(scope2); + done(); + }); + }); + }); + + it('forks the isolation scope when not passing any isolation scope', done => { + const initialScope = getIsolationScope(); + initialScope.setTag('aa', 'aa'); + + withIsolationScope(scope => { + expect(getIsolationScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('forks the isolation scope when passing undefined', done => { + const initialScope = getIsolationScope(); + initialScope.setTag('aa', 'aa'); + + withIsolationScope(undefined, scope => { + expect(getIsolationScope()).toBe(scope); + scope.setTag('bb', 'bb'); + expect(scope).not.toBe(initialScope); + expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + done(); + }); + }); + + it('sets the passed in isolation scope as active isolation scope', done => { + const initialScope = getIsolationScope(); + initialScope.setTag('aa', 'aa'); + + const customScope = new Scope(); + + withIsolationScope(customScope, scope => { + expect(getIsolationScope()).toBe(customScope); + expect(scope).toBe(customScope); + done(); + }); + }); +}); From 75d288c1947f1195f3e57c59c9121b2242f20b83 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 8 Apr 2024 15:02:16 +0100 Subject: [PATCH 12/64] feat(next): Handle existing root spans for isolation scope (#11479) This updates handling of next.js instrumentation to re-use an isolation scope from a root span. This should ensure we have consistent isolation scopes, no matter if next.js auto creates spans or not. --- .../withIsolationScopeOrReuseFromRootSpan.ts | 39 ++++++++ .../nextjs/src/common/utils/wrapperUtils.ts | 4 +- .../common/withServerActionInstrumentation.ts | 4 +- .../src/common/wrapApiHandlerWithSentry.ts | 4 +- .../wrapApiHandlerWithSentryVercelCrons.ts | 5 +- .../wrapGenerationFunctionWithSentry.ts | 4 +- .../src/common/wrapPageComponentWithSentry.ts | 7 +- .../src/common/wrapRouteHandlerWithSentry.ts | 4 +- .../common/wrapServerComponentWithSentry.ts | 4 +- ...hIsolationScopeOrReuseFromRootSpan.test.ts | 96 +++++++++++++++++++ ...hIsolationScopeOrReuseFromRootSpan.test.ts | 96 +++++++++++++++++++ 11 files changed, 250 insertions(+), 17 deletions(-) create mode 100644 packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts create mode 100644 packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts create mode 100644 packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts diff --git a/packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts b/packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts new file mode 100644 index 000000000000..82231022b980 --- /dev/null +++ b/packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts @@ -0,0 +1,39 @@ +import { + getActiveSpan, + getCapturedScopesOnSpan, + getDefaultIsolationScope, + getRootSpan, + spanToJSON, + withIsolationScope, +} from '@sentry/core'; +import type { Scope } from '@sentry/types'; + +/** + * Wrap a callback with a new isolation scope. + * However, if we have an active root span that was generated by next, we want to reuse the isolation scope from that span. + */ +export function withIsolationScopeOrReuseFromRootSpan(cb: (isolationScope: Scope) => T): T { + const activeSpan = getActiveSpan(); + + if (!activeSpan) { + return withIsolationScope(cb); + } + + const rootSpan = getRootSpan(activeSpan); + + // Verify this is a next span + if (!spanToJSON(rootSpan).data?.['next.route']) { + return withIsolationScope(cb); + } + + const scopes = getCapturedScopesOnSpan(rootSpan); + + const isolationScope = scopes.isolationScope; + + // If this is the default isolation scope, we still want to fork one + if (isolationScope === getDefaultIsolationScope()) { + return withIsolationScope(cb); + } + + return withIsolationScope(isolationScope, cb); +} diff --git a/packages/nextjs/src/common/utils/wrapperUtils.ts b/packages/nextjs/src/common/utils/wrapperUtils.ts index 311be587c439..ef14e19b02fa 100644 --- a/packages/nextjs/src/common/utils/wrapperUtils.ts +++ b/packages/nextjs/src/common/utils/wrapperUtils.ts @@ -10,13 +10,13 @@ import { startSpan, startSpanManual, withActiveSpan, - withIsolationScope, } from '@sentry/core'; import type { Span } from '@sentry/types'; import { isString } from '@sentry/utils'; import { platformSupportsStreaming } from './platformSupportsStreaming'; import { autoEndSpanOnResponseEnd, flushQueue } from './responseEnd'; +import { withIsolationScopeOrReuseFromRootSpan } from './withIsolationScopeOrReuseFromRootSpan'; declare module 'http' { interface IncomingMessage { @@ -89,7 +89,7 @@ export function withTracedServerSideDataFetcher Pr }, ): (...params: Parameters) => Promise> { return async function (this: unknown, ...args: Parameters): Promise> { - return withIsolationScope(async isolationScope => { + return withIsolationScopeOrReuseFromRootSpan(async isolationScope => { isolationScope.setSDKProcessingMetadata({ request: req, }); diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts index ff089944ac76..a3e95575217a 100644 --- a/packages/nextjs/src/common/withServerActionInstrumentation.ts +++ b/packages/nextjs/src/common/withServerActionInstrumentation.ts @@ -6,7 +6,6 @@ import { getClient, handleCallbackErrors, startSpan, - withIsolationScope, } from '@sentry/core'; import { logger } from '@sentry/utils'; @@ -14,6 +13,7 @@ import { DEBUG_BUILD } from './debug-build'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; +import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; interface Options { formData?: FormData; @@ -58,7 +58,7 @@ async function withServerActionInstrumentationImplementation> { addTracingExtensions(); - return withIsolationScope(isolationScope => { + return withIsolationScopeOrReuseFromRootSpan(isolationScope => { const sendDefaultPii = getClient()?.getOptions().sendDefaultPii; let sentryTraceHeader; diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts index 81b2f88e7888..1519667a7abd 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts @@ -5,7 +5,6 @@ import { continueTrace, setHttpStatus, startSpanManual, - withIsolationScope, } from '@sentry/core'; import { consoleSandbox, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils'; @@ -13,6 +12,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; +import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; /** * Wrap the given API route handler for tracing and error capturing. Thin wrapper around `withSentry`, which only @@ -54,7 +54,7 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz addTracingExtensions(); - return withIsolationScope(isolationScope => { + return withIsolationScopeOrReuseFromRootSpan(isolationScope => { return continueTrace( { // TODO(v8): Make it so that continue trace will allow null as sentryTrace value and remove this fallback here diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts index ec8791f95584..83f5b4aded98 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts @@ -1,7 +1,8 @@ -import { addTracingExtensions, captureCheckIn, withIsolationScope } from '@sentry/core'; +import { addTracingExtensions, captureCheckIn } from '@sentry/core'; import type { NextApiRequest } from 'next'; import type { VercelCronsConfig } from './types'; +import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; type EdgeRequest = { nextUrl: URL; @@ -19,7 +20,7 @@ export function wrapApiHandlerWithSentryVercelCrons { - return withIsolationScope(() => { + return withIsolationScopeOrReuseFromRootSpan(() => { if (!args || !args[0]) { return originalFunction.apply(thisArg, args); } diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index e0b771b7643b..e6dd1072a16b 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -8,7 +8,6 @@ import { getCurrentScope, handleCallbackErrors, startSpanManual, - withIsolationScope, } from '@sentry/core'; import type { WebFetchHeaders } from '@sentry/types'; import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'; @@ -17,6 +16,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import type { GenerationFunctionContext } from '../common/types'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; import { commonObjectToPropagationContext } from './utils/commonObjectTracing'; +import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; /** * Wraps a generation function (e.g. generateMetadata) with Sentry error and performance instrumentation. @@ -47,7 +47,7 @@ export function wrapGenerationFunctionWithSentry a data = { params, searchParams }; } - return withIsolationScope(isolationScope => { + return withIsolationScopeOrReuseFromRootSpan(isolationScope => { isolationScope.setSDKProcessingMetadata({ request: { headers: headers ? winterCGHeadersToDict(headers) : undefined, diff --git a/packages/nextjs/src/common/wrapPageComponentWithSentry.ts b/packages/nextjs/src/common/wrapPageComponentWithSentry.ts index 90d7f739d531..b14b5cff2e07 100644 --- a/packages/nextjs/src/common/wrapPageComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapPageComponentWithSentry.ts @@ -1,5 +1,6 @@ -import { addTracingExtensions, captureException, getCurrentScope, withIsolationScope } from '@sentry/core'; +import { addTracingExtensions, captureException, getCurrentScope } from '@sentry/core'; import { extractTraceparentData } from '@sentry/utils'; +import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; interface FunctionComponent { (...args: unknown[]): unknown; @@ -25,7 +26,7 @@ export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | C if (isReactClassComponent(pageComponent)) { return class SentryWrappedPageComponent extends pageComponent { public render(...args: unknown[]): unknown { - return withIsolationScope(() => { + return withIsolationScopeOrReuseFromRootSpan(() => { const scope = getCurrentScope(); // We extract the sentry trace data that is put in the component props by datafetcher wrappers const sentryTraceData = @@ -60,7 +61,7 @@ export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | C } else if (typeof pageComponent === 'function') { return new Proxy(pageComponent, { apply(target, thisArg, argArray: [{ _sentryTraceData?: string } | undefined]) { - return withIsolationScope(() => { + return withIsolationScopeOrReuseFromRootSpan(() => { const scope = getCurrentScope(); // We extract the sentry trace data that is put in the component props by datafetcher wrappers const sentryTraceData = argArray?.[0]?._sentryTraceData; diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 50300cf33b02..edee1e54cec4 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -8,7 +8,6 @@ import { handleCallbackErrors, setHttpStatus, startSpan, - withIsolationScope, } from '@sentry/core'; import { winterCGHeadersToDict } from '@sentry/utils'; @@ -16,6 +15,7 @@ import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavi import type { RouteHandlerContext } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; +import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; /** * Wraps a Next.js route handler with performance and error instrumentation. @@ -29,7 +29,7 @@ export function wrapRouteHandlerWithSentry any>( const { method, parameterizedRoute, headers } = context; return new Proxy(routeHandler, { apply: (originalFunction, thisArg, args) => { - return withIsolationScope(async isolationScope => { + return withIsolationScopeOrReuseFromRootSpan(async isolationScope => { isolationScope.setSDKProcessingMetadata({ request: { headers: headers ? winterCGHeadersToDict(headers) : undefined, diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index f91a2181b58b..76521b71e0d1 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -7,7 +7,6 @@ import { getCurrentScope, handleCallbackErrors, startSpanManual, - withIsolationScope, } from '@sentry/core'; import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'; @@ -16,6 +15,7 @@ import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/ import type { ServerComponentContext } from '../common/types'; import { commonObjectToPropagationContext } from './utils/commonObjectTracing'; import { flushQueue } from './utils/responseEnd'; +import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; /** * Wraps an `app` directory server component with Sentry error instrumentation. @@ -34,7 +34,7 @@ export function wrapServerComponentWithSentry any> return new Proxy(appDirComponent, { apply: (originalFunction, thisArg, args) => { // TODO: If we ever allow withIsolationScope to take a scope, we should pass a scope here that is shared between all of the server components, similar to what `commonObjectToPropagationContext` does. - return withIsolationScope(isolationScope => { + return withIsolationScopeOrReuseFromRootSpan(isolationScope => { const completeHeadersDict: Record = context.headers ? winterCGHeadersToDict(context.headers) : {}; diff --git a/packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts b/packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts new file mode 100644 index 000000000000..8f42b8a9264b --- /dev/null +++ b/packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts @@ -0,0 +1,96 @@ +import { + Scope, + getCurrentScope, + getGlobalScope, + getIsolationScope, + setCapturedScopesOnSpan, + startSpan, +} from '@sentry/core'; +import { GLOBAL_OBJ } from '@sentry/utils'; +import { init } from '@sentry/vercel-edge'; +import { AsyncLocalStorage } from 'async_hooks'; + +import { withIsolationScopeOrReuseFromRootSpan } from '../../src/common/utils/withIsolationScopeOrReuseFromRootSpan'; + +describe('withIsolationScopeOrReuseFromRootSpan', () => { + beforeEach(() => { + getIsolationScope().clear(); + getCurrentScope().clear(); + getGlobalScope().clear(); + (GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage; + + init({ + enableTracing: true, + }); + }); + + it('works without any span', () => { + const initialIsolationScope = getIsolationScope(); + initialIsolationScope.setTag('aa', 'aa'); + + withIsolationScopeOrReuseFromRootSpan(isolationScope => { + isolationScope.setTag('bb', 'bb'); + expect(isolationScope).not.toBe(initialIsolationScope); + expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + }); + }); + + it('works with a non-next.js span', () => { + const initialIsolationScope = getIsolationScope(); + initialIsolationScope.setTag('aa', 'aa'); + + const customScope = new Scope(); + + startSpan({ name: 'other' }, span => { + setCapturedScopesOnSpan(span, getCurrentScope(), customScope); + + withIsolationScopeOrReuseFromRootSpan(isolationScope => { + isolationScope.setTag('bb', 'bb'); + expect(isolationScope).not.toBe(initialIsolationScope); + expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + }); + }); + }); + + it('works with a next.js span', () => { + const initialIsolationScope = getIsolationScope(); + initialIsolationScope.setTag('aa', 'aa'); + + const customScope = new Scope(); + + startSpan( + { + name: 'other', + attributes: { 'next.route': 'aha' }, + }, + span => { + setCapturedScopesOnSpan(span, getCurrentScope(), customScope); + + withIsolationScopeOrReuseFromRootSpan(isolationScope => { + isolationScope.setTag('bb', 'bb'); + expect(isolationScope).toBe(customScope); + expect(isolationScope.getScopeData().tags).toEqual({ bb: 'bb' }); + }); + }, + ); + }); + + it('works with a next.js span that has default isolation scope', () => { + const initialIsolationScope = getIsolationScope(); + initialIsolationScope.setTag('aa', 'aa'); + + startSpan( + { + name: 'other', + attributes: { 'next.route': 'aha' }, + }, + () => { + withIsolationScopeOrReuseFromRootSpan(isolationScope => { + isolationScope.setTag('bb', 'bb'); + expect(isolationScope).not.toBe(initialIsolationScope); + expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + }); + }, + ); + }); +}); diff --git a/packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts b/packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts new file mode 100644 index 000000000000..0cfc53e0a5b2 --- /dev/null +++ b/packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts @@ -0,0 +1,96 @@ +import { + Scope, + getCurrentScope, + getGlobalScope, + getIsolationScope, + setCapturedScopesOnSpan, + startSpan, +} from '@sentry/core'; +import { init } from '@sentry/node'; +import { GLOBAL_OBJ } from '@sentry/utils'; +import { AsyncLocalStorage } from 'async_hooks'; + +import { withIsolationScopeOrReuseFromRootSpan } from '../../src/common/utils/withIsolationScopeOrReuseFromRootSpan'; + +describe('withIsolationScopeOrReuseFromRootSpan', () => { + beforeEach(() => { + getIsolationScope().clear(); + getCurrentScope().clear(); + getGlobalScope().clear(); + (GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage; + + init({ + enableTracing: true, + }); + }); + + it('works without any span', () => { + const initialIsolationScope = getIsolationScope(); + initialIsolationScope.setTag('aa', 'aa'); + + withIsolationScopeOrReuseFromRootSpan(isolationScope => { + isolationScope.setTag('bb', 'bb'); + expect(isolationScope).not.toBe(initialIsolationScope); + expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + }); + }); + + it('works with a non-next.js span', () => { + const initialIsolationScope = getIsolationScope(); + initialIsolationScope.setTag('aa', 'aa'); + + const customScope = new Scope(); + + startSpan({ name: 'other' }, span => { + setCapturedScopesOnSpan(span, getCurrentScope(), customScope); + + withIsolationScopeOrReuseFromRootSpan(isolationScope => { + isolationScope.setTag('bb', 'bb'); + expect(isolationScope).not.toBe(initialIsolationScope); + expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + }); + }); + }); + + it('works with a next.js span', () => { + const initialIsolationScope = getIsolationScope(); + initialIsolationScope.setTag('aa', 'aa'); + + const customScope = new Scope(); + + startSpan( + { + name: 'other', + attributes: { 'next.route': 'aha' }, + }, + span => { + setCapturedScopesOnSpan(span, getCurrentScope(), customScope); + + withIsolationScopeOrReuseFromRootSpan(isolationScope => { + isolationScope.setTag('bb', 'bb'); + expect(isolationScope).toBe(customScope); + expect(isolationScope.getScopeData().tags).toEqual({ bb: 'bb' }); + }); + }, + ); + }); + + it('works with a next.js span that has default isolation scope', () => { + const initialIsolationScope = getIsolationScope(); + initialIsolationScope.setTag('aa', 'aa'); + + startSpan( + { + name: 'other', + attributes: { 'next.route': 'aha' }, + }, + () => { + withIsolationScopeOrReuseFromRootSpan(isolationScope => { + isolationScope.setTag('bb', 'bb'); + expect(isolationScope).not.toBe(initialIsolationScope); + expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); + }); + }, + ); + }); +}); From 091d23b7a9b31786a12128757b3792d2bac03121 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 8 Apr 2024 10:15:12 -0400 Subject: [PATCH 13/64] test: Port ignoreErrors and denyUrls tests from karma runner (#11449) I want to remove the karma/mocha based tests in the browser package. To accomplish this, I'll be porting 1 test suite a day from the old integration tests to playwright. Today is Day 3: `packages/browser/test/integration/suites/config.js` I was surprised we never had `ignoreErrors` or `denyUrls` tests in playwright, so it's good to get the confidence that everything works here. ref https://github.com/getsentry/sentry-javascript/issues/11084 day 2: https://github.com/getsentry/sentry-javascript/pull/11436 --- .../suites/public-api/denyUrls/init.js | 14 +++++ .../suites/public-api/denyUrls/subject.js | 32 +++++++++++ .../suites/public-api/denyUrls/test.ts | 17 ++++++ .../suites/public-api/ignoreErrors/init.js | 14 +++++ .../suites/public-api/ignoreErrors/subject.js | 3 + .../suites/public-api/ignoreErrors/test.ts | 19 +++++++ .../browser/test/integration/suites/config.js | 55 ------------------- .../browser/test/integration/suites/shell.js | 1 - 8 files changed, 99 insertions(+), 56 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/public-api/denyUrls/init.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/denyUrls/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/denyUrls/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/init.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/test.ts delete mode 100644 packages/browser/test/integration/suites/config.js diff --git a/dev-packages/browser-integration-tests/suites/public-api/denyUrls/init.js b/dev-packages/browser-integration-tests/suites/public-api/denyUrls/init.js new file mode 100644 index 000000000000..c16b31fd1c85 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/denyUrls/init.js @@ -0,0 +1,14 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +window._errorCount = 0; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + denyUrls: ['foo.js'], + beforeSend: event => { + window._errorCount++; + return event; + }, +}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/denyUrls/subject.js b/dev-packages/browser-integration-tests/suites/public-api/denyUrls/subject.js new file mode 100644 index 000000000000..d0189ca3db75 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/denyUrls/subject.js @@ -0,0 +1,32 @@ +/** + * We always filter on the caller, not the cause of the error + * + * > foo.js file called a function in bar.js + * > bar.js file called a function in baz.js + * > baz.js threw an error + * + * foo.js is denied in the `init` call (init.js), thus we filter it + * */ +var urlWithDeniedUrl = new Error('filter'); +urlWithDeniedUrl.stack = + 'Error: bar\n' + + ' at http://localhost:5000/foo.js:7:19\n' + + ' at bar(http://localhost:5000/bar.js:2:3)\n' + + ' at baz(http://localhost:5000/baz.js:2:9)\n'; + +/** + * > foo-pass.js file called a function in bar-pass.js + * > bar-pass.js file called a function in baz-pass.js + * > baz-pass.js threw an error + * + * foo-pass.js is *not* denied in the `init` call (init.js), thus we don't filter it + * */ +var urlWithoutDeniedUrl = new Error('pass'); +urlWithoutDeniedUrl.stack = + 'Error: bar\n' + + ' at http://localhost:5000/foo-pass.js:7:19\n' + + ' at bar(http://localhost:5000/bar-pass.js:2:3)\n' + + ' at baz(http://localhost:5000/baz-pass.js:2:9)\n'; + +Sentry.captureException(urlWithDeniedUrl); +Sentry.captureException(urlWithoutDeniedUrl); diff --git a/dev-packages/browser-integration-tests/suites/public-api/denyUrls/test.ts b/dev-packages/browser-integration-tests/suites/public-api/denyUrls/test.ts new file mode 100644 index 000000000000..c374e8ae766c --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/denyUrls/test.ts @@ -0,0 +1,17 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../utils/helpers'; + +sentryTest('should allow to ignore specific urls', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values?.[0].type).toEqual('Error'); + expect(eventData.exception?.values?.[0].value).toEqual('pass'); + + const count = await page.evaluate('window._errorCount'); + expect(count).toEqual(1); +}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/init.js b/dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/init.js new file mode 100644 index 000000000000..e66214c5c0bf --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/init.js @@ -0,0 +1,14 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +window._errorCount = 0; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + ignoreErrors: ['ignoreErrorTest'], + beforeSend: event => { + window._errorCount++; + return event; + }, +}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/subject.js b/dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/subject.js new file mode 100644 index 000000000000..9f7883c97284 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/subject.js @@ -0,0 +1,3 @@ +Sentry.captureException(new Error('foo')); +Sentry.captureException(new Error('ignoreErrorTest')); +Sentry.captureException(new Error('bar')); diff --git a/dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/test.ts new file mode 100644 index 000000000000..35752ae39232 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/ignoreErrors/test.ts @@ -0,0 +1,19 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../utils/fixtures'; +import { getMultipleSentryEnvelopeRequests } from '../../../utils/helpers'; + +sentryTest('should allow to ignore specific errors', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const events = await getMultipleSentryEnvelopeRequests(page, 2, { url }); + + expect(events[0].exception?.values?.[0].type).toEqual('Error'); + expect(events[0].exception?.values?.[0].value).toEqual('foo'); + expect(events[1].exception?.values?.[0].type).toEqual('Error'); + expect(events[1].exception?.values?.[0].value).toEqual('bar'); + + const count = await page.evaluate('window._errorCount'); + expect(count).toEqual(2); +}); diff --git a/packages/browser/test/integration/suites/config.js b/packages/browser/test/integration/suites/config.js deleted file mode 100644 index 2637ea8c13b7..000000000000 --- a/packages/browser/test/integration/suites/config.js +++ /dev/null @@ -1,55 +0,0 @@ -describe('config', function () { - it('should allow to ignore specific errors', function () { - return runInSandbox(sandbox, function () { - Sentry.captureException(new Error('foo')); - Sentry.captureException(new Error('ignoreErrorTest')); - Sentry.captureException(new Error('bar')); - }).then(function (summary) { - assert.equal(summary.events[0].exception.values[0].type, 'Error'); - assert.equal(summary.events[0].exception.values[0].value, 'foo'); - assert.equal(summary.events[1].exception.values[0].type, 'Error'); - assert.equal(summary.events[1].exception.values[0].value, 'bar'); - }); - }); - - it('should allow to ignore specific urls', function () { - return runInSandbox(sandbox, function () { - /** - * We always filter on the caller, not the cause of the error - * - * > foo.js file called a function in bar.js - * > bar.js file called a function in baz.js - * > baz.js threw an error - * - * foo.js is denied in the `init` call (init.js), thus we filter it - * */ - var urlWithDeniedUrl = new Error('filter'); - urlWithDeniedUrl.stack = - 'Error: bar\n' + - ' at http://localhost:5000/foo.js:7:19\n' + - ' at bar(http://localhost:5000/bar.js:2:3)\n' + - ' at baz(http://localhost:5000/baz.js:2:9)\n'; - - /** - * > foo-pass.js file called a function in bar-pass.js - * > bar-pass.js file called a function in baz-pass.js - * > baz-pass.js threw an error - * - * foo-pass.js is *not* denied in the `init` call (init.js), thus we don't filter it - * */ - var urlWithoutDeniedUrl = new Error('pass'); - urlWithoutDeniedUrl.stack = - 'Error: bar\n' + - ' at http://localhost:5000/foo-pass.js:7:19\n' + - ' at bar(http://localhost:5000/bar-pass.js:2:3)\n' + - ' at baz(http://localhost:5000/baz-pass.js:2:9)\n'; - - Sentry.captureException(urlWithDeniedUrl); - Sentry.captureException(urlWithoutDeniedUrl); - }).then(function (summary) { - assert.lengthOf(summary.events, 1); - assert.equal(summary.events[0].exception.values[0].type, 'Error'); - assert.equal(summary.events[0].exception.values[0].value, 'pass'); - }); - }); -}); diff --git a/packages/browser/test/integration/suites/shell.js b/packages/browser/test/integration/suites/shell.js index e1555623b495..7df81ae2b53e 100644 --- a/packages/browser/test/integration/suites/shell.js +++ b/packages/browser/test/integration/suites/shell.js @@ -22,7 +22,6 @@ function runVariant(variant) { /** * The test runner will replace each of these placeholders with the contents of the corresponding file. */ - {{ suites/config.js }} // biome-ignore format: No trailing commas {{ suites/onerror.js }} // biome-ignore format: No trailing commas {{ suites/onunhandledrejection.js }} // biome-ignore format: No trailing commas {{ suites/builtins.js }} // biome-ignore format: No trailing commas From 022fcb6e56eca1582aeaa0e4b5f7780dd8cfe8a2 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Mon, 8 Apr 2024 10:31:50 -0400 Subject: [PATCH 14/64] ref(feedback): Configure font size (#11437) Font sizes were set instead of using config option --- packages/feedback/src/core/components/Actor.css.ts | 4 ++-- packages/feedback/src/modal/components/Dialog.css.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/feedback/src/core/components/Actor.css.ts b/packages/feedback/src/core/components/Actor.css.ts index 38c542d6e1b7..de295e9fc1f4 100644 --- a/packages/feedback/src/core/components/Actor.css.ts +++ b/packages/feedback/src/core/components/Actor.css.ts @@ -23,9 +23,9 @@ export function createActorStyles(): HTMLStyleElement { border-radius: var(--border-radius); cursor: pointer; - font-size: 14px; - font-weight: 600; font-family: inherit; + font-size: var(--font-size); + font-weight: 600; padding: 12px 16px; text-decoration: none; z-index: 9000; diff --git a/packages/feedback/src/modal/components/Dialog.css.ts b/packages/feedback/src/modal/components/Dialog.css.ts index 72fe0bd59cb4..9a8e3dc8aa68 100644 --- a/packages/feedback/src/modal/components/Dialog.css.ts +++ b/packages/feedback/src/modal/components/Dialog.css.ts @@ -128,7 +128,7 @@ export function createDialogStyles(): HTMLStyleElement { border: var(--input-border); border-radius: var(--form-content-border-radius); color: var(--input-foreground); - font-size: 14px; + font-size: var(--font-size); font-weight: 500; padding: 6px 12px; } @@ -153,10 +153,10 @@ export function createDialogStyles(): HTMLStyleElement { border: var(--cancel-border); border-radius: var(--form-content-border-radius); cursor: pointer; - font-size: 14px; + font-family: inherit; + font-size: var(--font-size); font-weight: 600; padding: 6px 16px; - font-family: inherit; } .btn[disabled] { opacity: 0.6; From f7db230b4e2f8f65a5c7fdd0012cfffc93fa1c4c Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 8 Apr 2024 22:19:05 +0100 Subject: [PATCH 15/64] feat(remix): Skip span creation for `OPTIONS` and `HEAD` requests. (#11149) Fixes: https://github.com/getsentry/sentry-javascript/issues/11105 Skips span creation for `OPTIONS` and `HEAD` requests in `wrapRequestHandler`, which apparently are the unwanted requests mentioned in #11105. We can also implement a more precise filtering but this seems to resolve it on my local test application. --- packages/remix/src/utils/instrumentServer.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 419e5e4ba0f2..593163a96c24 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -430,6 +430,13 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui return origRequestHandler.call(this, request, loadContext); } + const upperCaseMethod = request.method.toUpperCase(); + + // We don't want to wrap OPTIONS and HEAD requests + if (upperCaseMethod === 'OPTIONS' || upperCaseMethod === 'HEAD') { + return origRequestHandler.call(this, request, loadContext); + } + return withIsolationScope(async isolationScope => { const options = getClient()?.getOptions(); From 7383f8ab00d5cd534ed19e59a260e18d194c23c8 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Mon, 8 Apr 2024 14:37:18 -0700 Subject: [PATCH 16/64] ref(feedback): Refactor Feedback types into @sentry/types and reduce the exported surface area (#11355) You can read this commit-by-commit to watch the refactor unfold The situation is: after https://github.com/getsentry/sentry-javascript/pull/11342 is merged, all the code inside each of the `packages/feedback/src/[core|modal|screenshot]` will be split up into separate integrations. These integrations can't share the same `src/types/*.ts` definitions anymore. Therefore type definitions will need to live in @sentry/types instead. This PR moves all the types, and since they'll be public now in that package, i refactored things to reduce the public surface area and improve names where possible. The only non-type changes in here are where we move `createDialog.ts` and `createInput.ts` into their respective `integration.ts` files, no code changes at all. Related to https://github.com/getsentry/sentry-javascript/issues/11435 --- .../feedback/src/core/createMainStyles.ts | 9 +- packages/feedback/src/core/integration.ts | 38 +++---- packages/feedback/src/core/sendFeedback.ts | 11 +- packages/feedback/src/core/types.ts | 19 ++++ packages/feedback/src/index.ts | 3 - .../src/modal/components/DialogContainer.tsx | 2 +- .../src/modal/components/DialogHeader.tsx | 2 +- .../feedback/src/modal/components/Form.tsx | 31 +++--- .../src/modal/components/SentryLogo.ts | 2 +- packages/feedback/src/modal/createDialog.tsx | 95 ---------------- packages/feedback/src/modal/integration.ts | 22 ---- packages/feedback/src/modal/integration.tsx | 94 ++++++++++++++++ .../components/ScreenshotEditor.tsx | 10 +- .../feedback/src/screenshot/createInput.ts | 33 ------ .../feedback/src/screenshot/integration.ts | 48 ++++++--- packages/feedback/src/types/index.ts | 101 ------------------ packages/feedback/src/util/mergeOptions.ts | 3 +- packages/feedback/src/util/validate.ts | 2 +- packages/types/src/feedback.ts | 31 ------ .../types => types/src/feedback}/config.ts | 0 .../src/types => types/src/feedback}/form.ts | 2 +- packages/types/src/feedback/index.ts | 83 ++++++++++++++ packages/types/src/feedback/sendFeedback.ts | 54 ++++++++++ .../src/types => types/src/feedback}/theme.ts | 0 packages/types/src/index.ts | 13 ++- 25 files changed, 346 insertions(+), 362 deletions(-) create mode 100644 packages/feedback/src/core/types.ts delete mode 100644 packages/feedback/src/modal/createDialog.tsx delete mode 100644 packages/feedback/src/modal/integration.ts create mode 100644 packages/feedback/src/modal/integration.tsx delete mode 100644 packages/feedback/src/screenshot/createInput.ts delete mode 100644 packages/feedback/src/types/index.ts delete mode 100644 packages/types/src/feedback.ts rename packages/{feedback/src/types => types/src/feedback}/config.ts (100%) rename packages/{feedback/src/types => types/src/feedback}/form.ts (72%) create mode 100644 packages/types/src/feedback/index.ts create mode 100644 packages/types/src/feedback/sendFeedback.ts rename packages/{feedback/src/types => types/src/feedback}/theme.ts (100%) diff --git a/packages/feedback/src/core/createMainStyles.ts b/packages/feedback/src/core/createMainStyles.ts index 9eb5bd4dc2ab..ca5f138cb960 100644 --- a/packages/feedback/src/core/createMainStyles.ts +++ b/packages/feedback/src/core/createMainStyles.ts @@ -1,7 +1,7 @@ +import type { FeedbackInternalOptions } from '@sentry/types'; import { DOCUMENT } from '../constants'; -import type { FeedbackTheme, FeedbackThemes } from '../types'; -function getThemedCssVariables(theme: FeedbackTheme): string { +function getThemedCssVariables(theme: FeedbackInternalOptions['themeLight']): string { return ` --background: ${theme.background}; --background-hover: ${theme.backgroundHover}; @@ -39,7 +39,10 @@ function getThemedCssVariables(theme: FeedbackTheme): string { /** * Creates