From 1d87cecda01c19b840010bdac78754bcd9767817 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 11 Oct 2024 11:10:33 +0200 Subject: [PATCH 01/34] fix: Ensure type for `init` is correct in meta frameworks (#13938) Fixes https://github.com/getsentry/sentry-javascript/issues/13932 We have defined the type for the shared client/server types in meta-frameworks incorrectly, `init` now returns `Client | undefined` and not `void` anymore. --- packages/astro/src/index.types.ts | 4 ++-- packages/nextjs/src/index.types.ts | 4 ++-- packages/nuxt/src/index.types.ts | 4 ++-- packages/remix/src/index.types.ts | 4 ++-- packages/solidstart/src/index.types.ts | 4 ++-- packages/sveltekit/src/index.types.ts | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/astro/src/index.types.ts b/packages/astro/src/index.types.ts index 2227679dff21..f30357a91c4a 100644 --- a/packages/astro/src/index.types.ts +++ b/packages/astro/src/index.types.ts @@ -7,14 +7,14 @@ export * from '@sentry/node'; import type { NodeOptions } from '@sentry/node'; -import type { Integration, Options, StackParser } from '@sentry/types'; +import type { Client, Integration, Options, StackParser } from '@sentry/types'; import type * as clientSdk from './index.client'; import type * as serverSdk from './index.server'; import sentryAstro from './index.server'; /** Initializes Sentry Astro SDK */ -export declare function init(options: Options | clientSdk.BrowserOptions | NodeOptions): void; +export declare function init(options: Options | clientSdk.BrowserOptions | NodeOptions): Client | undefined; export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; export declare const contextLinesIntegration: typeof clientSdk.contextLinesIntegration; diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index a272990162b3..8f09999ad738 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -7,7 +7,7 @@ export * from './client'; export * from './server'; export * from './edge'; -import type { Integration, Options, StackParser } from '@sentry/types'; +import type { Client, Integration, Options, StackParser } from '@sentry/types'; import type * as clientSdk from './client'; import type { ServerComponentContext, VercelCronsConfig } from './common/types'; @@ -17,7 +17,7 @@ import type * as serverSdk from './server'; /** Initializes Sentry Next.js SDK */ export declare function init( options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions | edgeSdk.EdgeOptions, -): void; +): Client | undefined; export declare const getClient: typeof clientSdk.getClient; export declare const getRootSpan: typeof serverSdk.getRootSpan; diff --git a/packages/nuxt/src/index.types.ts b/packages/nuxt/src/index.types.ts index 614b27bdefe3..eca64effb5b4 100644 --- a/packages/nuxt/src/index.types.ts +++ b/packages/nuxt/src/index.types.ts @@ -1,4 +1,4 @@ -import type { Integration, Options, StackParser } from '@sentry/types'; +import type { Client, Integration, Options, StackParser } from '@sentry/types'; import type { SentryNuxtClientOptions } from './common/types'; import type * as clientSdk from './index.client'; import type * as serverSdk from './index.server'; @@ -9,7 +9,7 @@ export * from './index.client'; export * from './index.server'; // re-export colliding types -export declare function init(options: Options | SentryNuxtClientOptions | serverSdk.NodeOptions): void; +export declare function init(options: Options | SentryNuxtClientOptions | serverSdk.NodeOptions): Client | undefined; export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; export declare const contextLinesIntegration: typeof clientSdk.contextLinesIntegration; export declare const getDefaultIntegrations: (options: Options) => Integration[]; diff --git a/packages/remix/src/index.types.ts b/packages/remix/src/index.types.ts index 61088e370b45..b3342ff35250 100644 --- a/packages/remix/src/index.types.ts +++ b/packages/remix/src/index.types.ts @@ -3,14 +3,14 @@ export * from './index.client'; export * from './index.server'; -import type { Integration, Options, StackParser } from '@sentry/types'; +import type { Client, Integration, Options, StackParser } from '@sentry/types'; import * as clientSdk from './index.client'; import * as serverSdk from './index.server'; import type { RemixOptions } from './utils/remixOptions'; /** Initializes Sentry Remix SDK */ -export declare function init(options: RemixOptions): void; +export declare function init(options: RemixOptions): Client | undefined; export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; export declare const contextLinesIntegration: typeof clientSdk.contextLinesIntegration; diff --git a/packages/solidstart/src/index.types.ts b/packages/solidstart/src/index.types.ts index 51adf848775a..ef3cd196651b 100644 --- a/packages/solidstart/src/index.types.ts +++ b/packages/solidstart/src/index.types.ts @@ -5,13 +5,13 @@ export * from './client'; export * from './server'; export * from './vite'; -import type { Integration, Options, StackParser } from '@sentry/types'; +import type { Client, Integration, Options, StackParser } from '@sentry/types'; import type * as clientSdk from './client'; import type * as serverSdk from './server'; /** Initializes Sentry Solid Start SDK */ -export declare function init(options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions): void; +export declare function init(options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions): Client | undefined; export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; export declare const contextLinesIntegration: typeof clientSdk.contextLinesIntegration; diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index f2392a276ef4..4cccbf2a1ab7 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/packages/sveltekit/src/index.types.ts @@ -5,14 +5,14 @@ export * from './client'; export * from './vite'; export * from './server'; -import type { Integration, Options, StackParser } from '@sentry/types'; +import type { Client, Integration, Options, StackParser } from '@sentry/types'; import type { HandleClientError, HandleServerError } from '@sveltejs/kit'; import type * as clientSdk from './client'; import type * as serverSdk from './server'; /** Initializes Sentry SvelteKit SDK */ -export declare function init(options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions): void; +export declare function init(options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions): Client | undefined; export declare function handleErrorWithSentry(handleError?: T): T; From 5d5e2f40567f01dcfce5d4128bbafbf514ea5ddb Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 11 Oct 2024 13:02:22 +0200 Subject: [PATCH 02/34] feat(node): Implement Sentry-specific http instrumentation (#13763) This PR is a pretty big change, but it should help us to make custom OTEL support way better/easier to understand in the future. The fundamental problem this PR is trying to change is the restriction that we rely on our instance of `HttpInstrumentation` for Sentry-specific (read: not span-related) things. This made it tricky/annoying for users with a custom OTEL setup that may include `HttpInstrumentation` to get things working, because they may inadvertedly overwrite our instance of the instrumentation (because there can only be a single monkey-patch per module in the regular instrumentations), leading to hard-to-debug and often subtle problems. This PR fixes this by splitting out the non-span related http instrumentation code into a new, dedicated `SentryHttpInstrumentation`, which can be run side-by-side with the OTEL instrumentation (which emits spans, ...). We make this work by basically implementing our own custom, minimal `wrap` method instead of using shimmer. This way, OTEL instrumentation cannot identify the wrapped module as wrapped, and allow to wrap it again. While this is _slightly_ hacky and also means you cannot unwrap the http module, we do not generally support this for the Sentry SDK anyhow. This new Instrumentation does two things: 1. Handles automatic forking of the isolation scope per incoming request. By using our own code, we can actually make this much nicer, as we do not need to retrospectively update the isolation scope anymore, but instead we can do this properly now. 2. Emit breadcrumbs for outgoing requests. With this change, in errors only mode you really do not need our instance of the default `HttpInstrumentation` anymore at all, you can/should just provide your own if you want to capture http spans in a non-Sentry environment. However, this is sadly a bit tricky, because up to now we forced users in this scenario to still use our Http instance and avoid adding their own (instead we allowed users to pass their Http instrumentation config to our Http integration). This means that if we'd simply stop adding our http instrumentation instance when tracing is disabled, these users would stop getting otel spans as well :/ so we sadly can't change this without a major. Instead, I re-introduced the `spans: false` for `httpIntegration({ spans: false })`. When this is set (which for now is opt-in, but probably should be opt-out in v9) we will only register SentryHttpInstrumentation, not HttpInstrumentation, thus not emitting any spans. Users can add their own instance of HttpInstrumentation if they care. One semi-related thing that I noticed while looking into this is that we incorrectly emitted node-fetch spans in errors-only mode. This apparently sneaked in when we migrated to the new undici instrumentation. I extracted this out into a dedicated PR too, but the changes are in this too because tests were a bit fucked up otherwise. On top of https://github.com/getsentry/sentry-javascript/pull/13765 This also includes a bump of import-in-the-middle to 1.11.2, as this includes a fix to properly allow double-wrapping ESM modules. --- .../aws-lambda-layer-cjs/package.json | 3 +- .../aws-serverless-esm/package.json | 3 +- .../aws-serverless-esm/tests/basic.test.ts | 2 +- .../node-otel-without-tracing/package.json | 1 + .../node-otel-without-tracing/src/app.ts | 9 +- .../src/instrument.ts | 7 +- .../tests/errors.test.ts | 21 ++ .../tests/transactions.test.ts | 9 +- packages/node/package.json | 2 +- packages/node/src/integrations/http.ts | 312 ------------------ .../http/SentryHttpInstrumentation.ts | 277 ++++++++++++++++ packages/node/src/integrations/http/index.ts | 227 +++++++++++++ .../node/src/integrations/tracing/index.ts | 4 +- packages/remix/package.json | 1 - packages/remix/src/utils/integrations/http.ts | 18 +- yarn.lock | 70 +--- 16 files changed, 557 insertions(+), 409 deletions(-) delete mode 100644 packages/node/src/integrations/http.ts create mode 100644 packages/node/src/integrations/http/SentryHttpInstrumentation.ts create mode 100644 packages/node/src/integrations/http/index.ts diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json index e16d49c799f4..01375fe0c346 100644 --- a/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json +++ b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json @@ -14,8 +14,7 @@ }, "devDependencies": { "@sentry-internal/test-utils": "link:../../../test-utils", - "@playwright/test": "^1.44.1", - "wait-port": "1.0.4" + "@playwright/test": "^1.44.1" }, "volta": { "extends": "../../package.json" diff --git a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/package.json b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/package.json index ebd28b380d68..67aa6bc247d5 100644 --- a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/package.json +++ b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/package.json @@ -14,8 +14,7 @@ }, "devDependencies": { "@sentry-internal/test-utils": "link:../../../test-utils", - "@playwright/test": "^1.41.1", - "wait-port": "1.0.4" + "@playwright/test": "^1.41.1" }, "volta": { "extends": "../../package.json" diff --git a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts index 83fab96ee117..f6e57655ad08 100644 --- a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts +++ b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts @@ -18,7 +18,7 @@ test('AWS Serverless SDK sends events in ESM mode', async ({ request }) => { ); child_process.execSync('pnpm start', { - stdio: 'ignore', + stdio: 'inherit', }); const transactionEvent = await transactionEventPromise; diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json index afe666c2a8f1..ed01eff7dce2 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json @@ -14,6 +14,7 @@ "@opentelemetry/sdk-trace-node": "1.26.0", "@opentelemetry/exporter-trace-otlp-http": "0.53.0", "@opentelemetry/instrumentation-undici": "0.6.0", + "@opentelemetry/instrumentation-http": "0.53.0", "@opentelemetry/instrumentation": "0.53.0", "@sentry/core": "latest || *", "@sentry/node": "latest || *", diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts index c3fdfb9134a5..383eaf1b4484 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts @@ -7,6 +7,8 @@ import express from 'express'; const app = express(); const port = 3030; +Sentry.setTag('root-level-tag', 'yes'); + app.get('/test-success', function (req, res) { res.send({ version: 'v1' }); }); @@ -23,8 +25,6 @@ app.get('/test-transaction', function (req, res) { await fetch('http://localhost:3030/test-success'); - await Sentry.flush(); - res.send({}); }); }); @@ -38,7 +38,10 @@ app.get('/test-error', async function (req, res) { }); app.get('/test-exception/:id', function (req, _res) { - throw new Error(`This is an exception with id ${req.params.id}`); + const id = req.params.id; + Sentry.setTag(`param-${id}`, id); + + throw new Error(`This is an exception with id ${id}`); }); Sentry.setupExpressErrorHandler(app); diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts index d887696b1e73..47078a504e18 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts @@ -1,7 +1,8 @@ +const { HttpInstrumentation } = require('@opentelemetry/instrumentation-http'); const { NodeTracerProvider, BatchSpanProcessor } = require('@opentelemetry/sdk-trace-node'); const { OTLPTraceExporter } = require('@opentelemetry/exporter-trace-otlp-http'); const Sentry = require('@sentry/node'); -const { SentrySpanProcessor, SentryPropagator } = require('@sentry/opentelemetry'); +const { SentryPropagator } = require('@sentry/opentelemetry'); const { UndiciInstrumentation } = require('@opentelemetry/instrumentation-undici'); const { registerInstrumentations } = require('@opentelemetry/instrumentation'); @@ -15,6 +16,8 @@ Sentry.init({ tunnel: `http://localhost:3031/`, // proxy server // Tracing is completely disabled + integrations: [Sentry.httpIntegration({ spans: false })], + // Custom OTEL setup skipOpenTelemetrySetup: true, }); @@ -37,5 +40,5 @@ provider.register({ }); registerInstrumentations({ - instrumentations: [new UndiciInstrumentation()], + instrumentations: [new UndiciInstrumentation(), new HttpInstrumentation()], }); diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts index 28e63f02090c..1e4526a891a3 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts @@ -28,3 +28,24 @@ test('Sends correct error event', async ({ baseURL }) => { span_id: expect.any(String), }); }); + +test('Isolates requests correctly', async ({ baseURL }) => { + const errorEventPromise1 = waitForError('node-otel-without-tracing', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 555-a'; + }); + const errorEventPromise2 = waitForError('node-otel-without-tracing', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 555-b'; + }); + + fetch(`${baseURL}/test-exception/555-a`); + fetch(`${baseURL}/test-exception/555-b`); + + const errorEvent1 = await errorEventPromise1; + const errorEvent2 = await errorEventPromise2; + + expect(errorEvent1.transaction).toEqual('GET /test-exception/555-a'); + expect(errorEvent1.tags).toEqual({ 'root-level-tag': 'yes', 'param-555-a': '555-a' }); + + expect(errorEvent2.transaction).toEqual('GET /test-exception/555-b'); + expect(errorEvent2.tags).toEqual({ 'root-level-tag': 'yes', 'param-555-b': '555-b' }); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts index 9c91a0ed9531..bb069b7e3e11 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts @@ -12,9 +12,7 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { const scopeSpans = json.resourceSpans?.[0]?.scopeSpans; - const httpScope = scopeSpans?.find( - scopeSpan => scopeSpan.scope.name === '@opentelemetry_sentry-patched/instrumentation-http', - ); + const httpScope = scopeSpans?.find(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http'); return ( httpScope && @@ -40,9 +38,7 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { // But our default node-fetch spans are not emitted expect(scopeSpans.length).toEqual(2); - const httpScopes = scopeSpans?.filter( - scopeSpan => scopeSpan.scope.name === '@opentelemetry_sentry-patched/instrumentation-http', - ); + const httpScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http'); const undiciScopes = scopeSpans?.filter( scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-undici', ); @@ -114,7 +110,6 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { { key: 'net.peer.port', value: { intValue: expect.any(Number) } }, { key: 'http.status_code', value: { intValue: 200 } }, { key: 'http.status_text', value: { stringValue: 'OK' } }, - { key: 'sentry.origin', value: { stringValue: 'auto.http.otel.http' } }, ]), droppedAttributesCount: 0, events: [], diff --git a/packages/node/package.json b/packages/node/package.json index d9ca6f86074e..8145cf18a7d6 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -99,7 +99,7 @@ "@sentry/opentelemetry": "8.34.0", "@sentry/types": "8.34.0", "@sentry/utils": "8.34.0", - "import-in-the-middle": "^1.11.0" + "import-in-the-middle": "^1.11.2" }, "devDependencies": { "@types/node": "^14.18.0" diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts deleted file mode 100644 index d6796aa866e5..000000000000 --- a/packages/node/src/integrations/http.ts +++ /dev/null @@ -1,312 +0,0 @@ -import type { ClientRequest, IncomingMessage, RequestOptions, ServerResponse } from 'node:http'; -import type { Span } from '@opentelemetry/api'; -import { diag } from '@opentelemetry/api'; -import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; -import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; - -import { - addBreadcrumb, - defineIntegration, - getCapturedScopesOnSpan, - getCurrentScope, - getIsolationScope, - setCapturedScopesOnSpan, -} from '@sentry/core'; -import { getClient } from '@sentry/opentelemetry'; -import type { IntegrationFn, SanitizedRequestData } from '@sentry/types'; - -import { - getBreadcrumbLogLevelFromHttpStatusCode, - getSanitizedUrlString, - parseUrl, - stripUrlQueryAndFragment, -} from '@sentry/utils'; -import type { NodeClient } from '../sdk/client'; -import { setIsolationScope } from '../sdk/scope'; -import type { HTTPModuleRequestIncomingMessage } from '../transports/http-module'; -import { addOriginToSpan } from '../utils/addOriginToSpan'; -import { getRequestUrl } from '../utils/getRequestUrl'; - -const INTEGRATION_NAME = 'Http'; - -const INSTRUMENTATION_NAME = '@opentelemetry_sentry-patched/instrumentation-http'; - -interface HttpOptions { - /** - * Whether breadcrumbs should be recorded for requests. - * Defaults to true - */ - breadcrumbs?: boolean; - - /** - * Do not capture spans or breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`. - * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. - * - * The `url` param contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request. - * For example: `'https://someService.com/users/details?id=123'` - * - * The `request` param contains the original {@type RequestOptions} object used to make the outgoing request. - * You can use it to filter on additional properties like method, headers, etc. - */ - ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; - - /** - * Do not capture spans or breadcrumbs for incoming HTTP requests to URLs where the given callback returns `true`. - * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. - * - * The `urlPath` param consists of the URL path and query string (if any) of the incoming request. - * For example: `'/users/details?id=123'` - * - * The `request` param contains the original {@type IncomingMessage} object of the incoming request. - * You can use it to filter on additional properties like method, headers, etc. - */ - ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean; - - /** - * Additional instrumentation options that are passed to the underlying HttpInstrumentation. - */ - instrumentation?: { - requestHook?: (span: Span, req: ClientRequest | HTTPModuleRequestIncomingMessage) => void; - responseHook?: (span: Span, response: HTTPModuleRequestIncomingMessage | ServerResponse) => void; - applyCustomAttributesOnSpan?: ( - span: Span, - request: ClientRequest | HTTPModuleRequestIncomingMessage, - response: HTTPModuleRequestIncomingMessage | ServerResponse, - ) => void; - - /** - * You can pass any configuration through to the underlying instrumention. - * Note that there are no semver guarantees for this! - */ - _experimentalConfig?: ConstructorParameters[0]; - }; - - /** Allows to pass a custom version of HttpInstrumentation. We use this for Next.js. */ - _instrumentation?: typeof HttpInstrumentation; -} - -let _httpOptions: HttpOptions = {}; -let _httpInstrumentation: HttpInstrumentation | undefined; - -/** - * Instrument the HTTP module. - * This can only be instrumented once! If this called again later, we just update the options. - */ -export const instrumentHttp = Object.assign( - function (): void { - if (_httpInstrumentation) { - return; - } - - const _InstrumentationClass = _httpOptions._instrumentation || HttpInstrumentation; - - _httpInstrumentation = new _InstrumentationClass({ - ..._httpOptions.instrumentation?._experimentalConfig, - ignoreOutgoingRequestHook: request => { - const url = getRequestUrl(request); - - if (!url) { - return false; - } - - const _ignoreOutgoingRequests = _httpOptions.ignoreOutgoingRequests; - if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { - return true; - } - - return false; - }, - - ignoreIncomingRequestHook: request => { - // request.url is the only property that holds any information about the url - // it only consists of the URL path and query string (if any) - const urlPath = request.url; - - const method = request.method?.toUpperCase(); - // We do not capture OPTIONS/HEAD requests as transactions - if (method === 'OPTIONS' || method === 'HEAD') { - return true; - } - - const _ignoreIncomingRequests = _httpOptions.ignoreIncomingRequests; - if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { - return true; - } - - return false; - }, - - requireParentforOutgoingSpans: false, - requireParentforIncomingSpans: false, - requestHook: (span, req) => { - addOriginToSpan(span, 'auto.http.otel.http'); - - // both, incoming requests and "client" requests made within the app trigger the requestHook - // we only want to isolate and further annotate incoming requests (IncomingMessage) - if (_isClientRequest(req)) { - _httpOptions.instrumentation?.requestHook?.(span, req); - return; - } - - const scopes = getCapturedScopesOnSpan(span); - - const isolationScope = (scopes.isolationScope || getIsolationScope()).clone(); - const scope = scopes.scope || getCurrentScope(); - - // Update the isolation scope, isolate this request - isolationScope.setSDKProcessingMetadata({ request: req }); - - const client = getClient(); - if (client && client.getOptions().autoSessionTracking) { - 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 - // update the transactionName once we get a parameterized route. - const httpMethod = (req.method || 'GET').toUpperCase(); - const httpTarget = stripUrlQueryAndFragment(req.url || '/'); - - const bestEffortTransactionName = `${httpMethod} ${httpTarget}`; - - isolationScope.setTransactionName(bestEffortTransactionName); - - if (isKnownPrefetchRequest(req)) { - span.setAttribute('sentry.http.prefetch', true); - } - - _httpOptions.instrumentation?.requestHook?.(span, req); - }, - responseHook: (span, res) => { - const client = getClient(); - if (client && client.getOptions().autoSessionTracking) { - setImmediate(() => { - client['_captureRequestSession'](); - }); - } - - _httpOptions.instrumentation?.responseHook?.(span, res); - }, - applyCustomAttributesOnSpan: ( - span: Span, - request: ClientRequest | HTTPModuleRequestIncomingMessage, - response: HTTPModuleRequestIncomingMessage | ServerResponse, - ) => { - const _breadcrumbs = typeof _httpOptions.breadcrumbs === 'undefined' ? true : _httpOptions.breadcrumbs; - if (_breadcrumbs) { - _addRequestBreadcrumb(request, response); - } - - _httpOptions.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); - }, - }); - - // We want to update the logger namespace so we can better identify what is happening here - try { - _httpInstrumentation['_diag'] = diag.createComponentLogger({ - namespace: INSTRUMENTATION_NAME, - }); - - // @ts-expect-error This is marked as read-only, but we overwrite it anyhow - _httpInstrumentation.instrumentationName = INSTRUMENTATION_NAME; - } catch { - // ignore errors here... - } - addOpenTelemetryInstrumentation(_httpInstrumentation); - }, - { - id: INTEGRATION_NAME, - }, -); - -const _httpIntegration = ((options: HttpOptions = {}) => { - return { - name: INTEGRATION_NAME, - setupOnce() { - _httpOptions = options; - instrumentHttp(); - }, - }; -}) satisfies IntegrationFn; - -/** - * The http integration instruments Node's internal http and https modules. - * It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span. - */ -export const httpIntegration = defineIntegration(_httpIntegration); - -/** Add a breadcrumb for outgoing requests. */ -function _addRequestBreadcrumb( - request: ClientRequest | HTTPModuleRequestIncomingMessage, - response: HTTPModuleRequestIncomingMessage | ServerResponse, -): void { - // Only generate breadcrumbs for outgoing requests - if (!_isClientRequest(request)) { - return; - } - - const data = getBreadcrumbData(request); - const statusCode = response.statusCode; - const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode); - - addBreadcrumb( - { - category: 'http', - data: { - status_code: statusCode, - ...data, - }, - type: 'http', - level, - }, - { - event: 'response', - request, - response, - }, - ); -} - -function getBreadcrumbData(request: ClientRequest): Partial { - try { - // `request.host` does not contain the port, but the host header does - const host = request.getHeader('host') || request.host; - const url = new URL(request.path, `${request.protocol}//${host}`); - const parsedUrl = parseUrl(url.toString()); - - const data: Partial = { - url: getSanitizedUrlString(parsedUrl), - 'http.method': request.method || 'GET', - }; - - if (parsedUrl.search) { - data['http.query'] = parsedUrl.search; - } - if (parsedUrl.hash) { - data['http.fragment'] = parsedUrl.hash; - } - - return data; - } catch { - return {}; - } -} - -/** - * Determines if @param req is a ClientRequest, meaning the request was created within the express app - * and it's an outgoing request. - * Checking for properties instead of using `instanceOf` to avoid importing the request classes. - */ -function _isClientRequest(req: ClientRequest | HTTPModuleRequestIncomingMessage): req is ClientRequest { - return 'outputData' in req && 'outputSize' in req && !('client' in req) && !('statusCode' in req); -} - -/** - * Detects if an incoming request is a prefetch request. - */ -function isKnownPrefetchRequest(req: HTTPModuleRequestIncomingMessage): boolean { - // Currently only handles Next.js prefetch requests but may check other frameworks in the future. - return req.headers['next-router-prefetch'] === '1'; -} diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts new file mode 100644 index 000000000000..62922b1b3921 --- /dev/null +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -0,0 +1,277 @@ +import type * as http from 'node:http'; +import type * as https from 'node:https'; +import { VERSION } from '@opentelemetry/core'; +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; +import { addBreadcrumb, getClient, getIsolationScope, withIsolationScope } from '@sentry/core'; +import type { SanitizedRequestData } from '@sentry/types'; +import { + getBreadcrumbLogLevelFromHttpStatusCode, + getSanitizedUrlString, + parseUrl, + stripUrlQueryAndFragment, +} from '@sentry/utils'; +import type { NodeClient } from '../../sdk/client'; + +type Http = typeof http; +type Https = typeof https; + +type SentryHttpInstrumentationOptions = InstrumentationConfig & { + breadcrumbs?: boolean; +}; + +/** + * This custom HTTP instrumentation is used to isolate incoming requests and annotate them with additional information. + * It does not emit any spans. + * + * The reason this is isolated from the OpenTelemetry instrumentation is that users may overwrite this, + * which would lead to Sentry not working as expected. + * + * Important note: Contrary to other OTEL instrumentation, this one cannot be unwrapped. + * It only does minimal things though and does not emit any spans. + * + * This is heavily inspired & adapted from: + * https://github.com/open-telemetry/opentelemetry-js/blob/f8ab5592ddea5cba0a3b33bf8d74f27872c0367f/experimental/packages/opentelemetry-instrumentation-http/src/http.ts + */ +export class SentryHttpInstrumentation extends InstrumentationBase { + public constructor(config: SentryHttpInstrumentationOptions = {}) { + super('@sentry/instrumentation-http', VERSION, config); + } + + /** @inheritdoc */ + public init(): [InstrumentationNodeModuleDefinition, InstrumentationNodeModuleDefinition] { + return [this._getHttpsInstrumentation(), this._getHttpInstrumentation()]; + } + + /** Get the instrumentation for the http module. */ + private _getHttpInstrumentation(): InstrumentationNodeModuleDefinition { + return new InstrumentationNodeModuleDefinition( + 'http', + ['*'], + (moduleExports: Http): Http => { + // Patch incoming requests for request isolation + stealthWrap(moduleExports.Server.prototype, 'emit', this._getPatchIncomingRequestFunction()); + + // Patch outgoing requests for breadcrumbs + const patchedRequest = stealthWrap(moduleExports, 'request', this._getPatchOutgoingRequestFunction()); + stealthWrap(moduleExports, 'get', this._getPatchOutgoingGetFunction(patchedRequest)); + + return moduleExports; + }, + () => { + // no unwrap here + }, + ); + } + + /** Get the instrumentation for the https module. */ + private _getHttpsInstrumentation(): InstrumentationNodeModuleDefinition { + return new InstrumentationNodeModuleDefinition( + 'https', + ['*'], + (moduleExports: Https): Https => { + // Patch incoming requests for request isolation + stealthWrap(moduleExports.Server.prototype, 'emit', this._getPatchIncomingRequestFunction()); + + // Patch outgoing requests for breadcrumbs + const patchedRequest = stealthWrap(moduleExports, 'request', this._getPatchOutgoingRequestFunction()); + stealthWrap(moduleExports, 'get', this._getPatchOutgoingGetFunction(patchedRequest)); + + return moduleExports; + }, + () => { + // no unwrap here + }, + ); + } + + /** + * Patch the incoming request function for request isolation. + */ + private _getPatchIncomingRequestFunction(): ( + original: (event: string, ...args: unknown[]) => boolean, + ) => (this: unknown, event: string, ...args: unknown[]) => boolean { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const instrumentation = this; + + return ( + original: (event: string, ...args: unknown[]) => boolean, + ): ((this: unknown, event: string, ...args: unknown[]) => boolean) => { + return function incomingRequest(this: unknown, event: string, ...args: unknown[]): boolean { + // Only traces request events + if (event !== 'request') { + return original.apply(this, [event, ...args]); + } + + instrumentation._diag.debug('http instrumentation for incoming request'); + + const request = args[0] as http.IncomingMessage; + + const isolationScope = getIsolationScope().clone(); + + // Update the isolation scope, isolate this request + isolationScope.setSDKProcessingMetadata({ request }); + + const client = getClient(); + if (client && client.getOptions().autoSessionTracking) { + isolationScope.setRequestSession({ status: 'ok' }); + } + + // 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 httpMethod = (request.method || 'GET').toUpperCase(); + const httpTarget = stripUrlQueryAndFragment(request.url || '/'); + + const bestEffortTransactionName = `${httpMethod} ${httpTarget}`; + + isolationScope.setTransactionName(bestEffortTransactionName); + + return withIsolationScope(isolationScope, () => { + return original.apply(this, [event, ...args]); + }); + }; + }; + } + + /** + * Patch the outgoing request function for breadcrumbs. + */ + private _getPatchOutgoingRequestFunction(): ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + original: (...args: any[]) => http.ClientRequest, + ) => (...args: unknown[]) => http.ClientRequest { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const instrumentation = this; + + return (original: (...args: unknown[]) => http.ClientRequest): ((...args: unknown[]) => http.ClientRequest) => { + return function outgoingRequest(this: unknown, ...args: unknown[]): http.ClientRequest { + instrumentation._diag.debug('http instrumentation for outgoing requests'); + + const request = original.apply(this, args) as ReturnType; + + request.prependListener('response', (response: http.IncomingMessage) => { + const breadcrumbs = instrumentation.getConfig().breadcrumbs; + const _breadcrumbs = typeof breadcrumbs === 'undefined' ? true : breadcrumbs; + if (_breadcrumbs) { + addRequestBreadcrumb(request, response); + } + }); + + return request; + }; + }; + } + + /** Path the outgoing get function for breadcrumbs. */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private _getPatchOutgoingGetFunction(clientRequest: (...args: any[]) => http.ClientRequest) { + return (_original: unknown): ((...args: unknown[]) => http.ClientRequest) => { + // Re-implement http.get. This needs to be done (instead of using + // getPatchOutgoingRequestFunction to patch it) because we need to + // set the trace context header before the returned http.ClientRequest is + // ended. The Node.js docs state that the only differences between + // request and get are that (1) get defaults to the HTTP GET method and + // (2) the returned request object is ended immediately. The former is + // already true (at least in supported Node versions up to v10), so we + // simply follow the latter. Ref: + // https://nodejs.org/dist/latest/docs/api/http.html#http_http_get_options_callback + // https://github.com/googleapis/cloud-trace-nodejs/blob/master/src/instrumentations/instrumentation-http.ts#L198 + return function outgoingGetRequest(...args: unknown[]): http.ClientRequest { + const req = clientRequest(...args); + req.end(); + return req; + }; + }; + } +} + +/** + * This is a minimal version of `wrap` from shimmer: + * https://github.com/othiym23/shimmer/blob/master/index.js + * + * In contrast to the original implementation, this version does not allow to unwrap, + * and does not make it clear that the method is wrapped. + * This is necessary because we want to wrap the http module with our own code, + * while still allowing to use the HttpInstrumentation from OTEL. + * + * Without this, if we'd just use `wrap` from shimmer, the OTEL instrumentation would remove our wrapping, + * because it only allows any module to be wrapped a single time. + */ +function stealthWrap( + nodule: Nodule, + name: FieldName, + wrapper: (original: Nodule[FieldName]) => Nodule[FieldName], +): Nodule[FieldName] { + const original = nodule[name]; + const wrapped = wrapper(original); + + defineProperty(nodule, name, wrapped); + return wrapped; +} + +// Sets a property on an object, preserving its enumerability. +function defineProperty( + obj: Nodule, + name: FieldName, + value: Nodule[FieldName], +): void { + const enumerable = !!obj[name] && Object.prototype.propertyIsEnumerable.call(obj, name); + + Object.defineProperty(obj, name, { + configurable: true, + enumerable: enumerable, + writable: true, + value: value, + }); +} + +/** Add a breadcrumb for outgoing requests. */ +function addRequestBreadcrumb(request: http.ClientRequest, response: http.IncomingMessage): void { + const data = getBreadcrumbData(request); + + const statusCode = response.statusCode; + const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode); + + addBreadcrumb( + { + category: 'http', + data: { + status_code: statusCode, + ...data, + }, + type: 'http', + level, + }, + { + event: 'response', + request, + response, + }, + ); +} + +function getBreadcrumbData(request: http.ClientRequest): Partial { + try { + // `request.host` does not contain the port, but the host header does + const host = request.getHeader('host') || request.host; + const url = new URL(request.path, `${request.protocol}//${host}`); + const parsedUrl = parseUrl(url.toString()); + + const data: Partial = { + url: getSanitizedUrlString(parsedUrl), + 'http.method': request.method || 'GET', + }; + + if (parsedUrl.search) { + data['http.query'] = parsedUrl.search; + } + if (parsedUrl.hash) { + data['http.fragment'] = parsedUrl.hash; + } + + return data; + } catch { + return {}; + } +} diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts new file mode 100644 index 000000000000..9c8d13b58127 --- /dev/null +++ b/packages/node/src/integrations/http/index.ts @@ -0,0 +1,227 @@ +import type { ClientRequest, IncomingMessage, RequestOptions, ServerResponse } from 'node:http'; +import { diag } from '@opentelemetry/api'; +import type { HttpInstrumentationConfig } from '@opentelemetry/instrumentation-http'; +import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; + +import { defineIntegration } from '@sentry/core'; +import { getClient } from '@sentry/opentelemetry'; +import type { IntegrationFn, Span } from '@sentry/types'; + +import { generateInstrumentOnce } from '../../otel/instrument'; +import type { NodeClient } from '../../sdk/client'; +import type { HTTPModuleRequestIncomingMessage } from '../../transports/http-module'; +import { addOriginToSpan } from '../../utils/addOriginToSpan'; +import { getRequestUrl } from '../../utils/getRequestUrl'; +import { SentryHttpInstrumentation } from './SentryHttpInstrumentation'; + +const INTEGRATION_NAME = 'Http'; + +const INSTRUMENTATION_NAME = '@opentelemetry_sentry-patched/instrumentation-http'; + +interface HttpOptions { + /** + * Whether breadcrumbs should be recorded for requests. + * Defaults to true + */ + breadcrumbs?: boolean; + + /** + * If set to false, do not emit any spans. + * This will ensure that the default HttpInstrumentation from OpenTelemetry is not setup, + * only the Sentry-specific instrumentation for request isolation is applied. + */ + spans?: boolean; + + /** + * Do not capture spans or breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`. + * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. + * + * The `url` param contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request. + * For example: `'https://someService.com/users/details?id=123'` + * + * The `request` param contains the original {@type RequestOptions} object used to make the outgoing request. + * You can use it to filter on additional properties like method, headers, etc. + */ + ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; + + /** + * Do not capture spans or breadcrumbs for incoming HTTP requests to URLs where the given callback returns `true`. + * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. + * + * The `urlPath` param consists of the URL path and query string (if any) of the incoming request. + * For example: `'/users/details?id=123'` + * + * The `request` param contains the original {@type IncomingMessage} object of the incoming request. + * You can use it to filter on additional properties like method, headers, etc. + */ + ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean; + + /** + * If true, do not generate spans for incoming requests at all. + * This is used by Remix to avoid generating spans for incoming requests, as it generates its own spans. + */ + disableIncomingRequestSpans?: boolean; + + /** + * Additional instrumentation options that are passed to the underlying HttpInstrumentation. + */ + instrumentation?: { + requestHook?: (span: Span, req: ClientRequest | HTTPModuleRequestIncomingMessage) => void; + responseHook?: (span: Span, response: HTTPModuleRequestIncomingMessage | ServerResponse) => void; + applyCustomAttributesOnSpan?: ( + span: Span, + request: ClientRequest | HTTPModuleRequestIncomingMessage, + response: HTTPModuleRequestIncomingMessage | ServerResponse, + ) => void; + + /** + * You can pass any configuration through to the underlying instrumention. + * Note that there are no semver guarantees for this! + */ + _experimentalConfig?: ConstructorParameters[0]; + }; +} + +export const instrumentSentryHttp = generateInstrumentOnce<{ breadcrumbs?: boolean }>( + `${INTEGRATION_NAME}.sentry`, + options => { + return new SentryHttpInstrumentation({ breadcrumbs: options?.breadcrumbs }); + }, +); + +export const instrumentOtelHttp = generateInstrumentOnce(INTEGRATION_NAME, config => { + const instrumentation = new HttpInstrumentation(config); + + // We want to update the logger namespace so we can better identify what is happening here + try { + instrumentation['_diag'] = diag.createComponentLogger({ + namespace: INSTRUMENTATION_NAME, + }); + // @ts-expect-error We are writing a read-only property here... + instrumentation.instrumentationName = INSTRUMENTATION_NAME; + } catch { + // ignore errors here... + } + + return instrumentation; +}); + +/** + * Instrument the HTTP and HTTPS modules. + */ +const instrumentHttp = (options: HttpOptions = {}): void => { + // This is the "regular" OTEL instrumentation that emits spans + if (options.spans !== false) { + const instrumentationConfig = getConfigWithDefaults(options); + instrumentOtelHttp(instrumentationConfig); + } + + // This is the Sentry-specific instrumentation that isolates requests & creates breadcrumbs + // Note that this _has_ to be wrapped after the OTEL instrumentation, + // otherwise the isolation will not work correctly + instrumentSentryHttp(options); +}; + +const _httpIntegration = ((options: HttpOptions = {}) => { + return { + name: INTEGRATION_NAME, + setupOnce() { + instrumentHttp(options); + }, + }; +}) satisfies IntegrationFn; + +/** + * The http integration instruments Node's internal http and https modules. + * It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span. + */ +export const httpIntegration = defineIntegration(_httpIntegration); + +/** + * Determines if @param req is a ClientRequest, meaning the request was created within the express app + * and it's an outgoing request. + * Checking for properties instead of using `instanceOf` to avoid importing the request classes. + */ +function _isClientRequest(req: ClientRequest | HTTPModuleRequestIncomingMessage): req is ClientRequest { + return 'outputData' in req && 'outputSize' in req && !('client' in req) && !('statusCode' in req); +} + +/** + * Detects if an incoming request is a prefetch request. + */ +function isKnownPrefetchRequest(req: HTTPModuleRequestIncomingMessage): boolean { + // Currently only handles Next.js prefetch requests but may check other frameworks in the future. + return req.headers['next-router-prefetch'] === '1'; +} + +function getConfigWithDefaults(options: Partial = {}): HttpInstrumentationConfig { + const instrumentationConfig = { + ...options.instrumentation?._experimentalConfig, + + disableIncomingRequestInstrumentation: options.disableIncomingRequestSpans, + + ignoreOutgoingRequestHook: request => { + const url = getRequestUrl(request); + + if (!url) { + return false; + } + + const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; + if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { + return true; + } + + return false; + }, + + ignoreIncomingRequestHook: request => { + // request.url is the only property that holds any information about the url + // it only consists of the URL path and query string (if any) + const urlPath = request.url; + + const method = request.method?.toUpperCase(); + // We do not capture OPTIONS/HEAD requests as transactions + if (method === 'OPTIONS' || method === 'HEAD') { + return true; + } + + const _ignoreIncomingRequests = options.ignoreIncomingRequests; + if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { + return true; + } + + return false; + }, + + requireParentforOutgoingSpans: false, + requireParentforIncomingSpans: false, + requestHook: (span, req) => { + addOriginToSpan(span, 'auto.http.otel.http'); + if (!_isClientRequest(req) && isKnownPrefetchRequest(req)) { + span.setAttribute('sentry.http.prefetch', true); + } + + options.instrumentation?.requestHook?.(span, req); + }, + responseHook: (span, res) => { + const client = getClient(); + if (client && client.getOptions().autoSessionTracking) { + setImmediate(() => { + client['_captureRequestSession'](); + }); + } + + options.instrumentation?.responseHook?.(span, res); + }, + applyCustomAttributesOnSpan: ( + span: Span, + request: ClientRequest | HTTPModuleRequestIncomingMessage, + response: HTTPModuleRequestIncomingMessage | ServerResponse, + ) => { + options.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); + }, + } satisfies HttpInstrumentationConfig; + + return instrumentationConfig; +} diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index 1181179a57d3..328767c403be 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -1,5 +1,5 @@ import type { Integration } from '@sentry/types'; -import { instrumentHttp } from '../http'; +import { instrumentOtelHttp } from '../http'; import { amqplibIntegration, instrumentAmqplib } from './amqplib'; import { connectIntegration, instrumentConnect } from './connect'; @@ -54,7 +54,7 @@ export function getAutoPerformanceIntegrations(): Integration[] { // eslint-disable-next-line @typescript-eslint/no-explicit-any export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) => void) & { id: string })[] { return [ - instrumentHttp, + instrumentOtelHttp, instrumentExpress, instrumentConnect, instrumentFastify, diff --git a/packages/remix/package.json b/packages/remix/package.json index 042da8843032..4ed3b71f626c 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -52,7 +52,6 @@ "access": "public" }, "dependencies": { - "@opentelemetry/instrumentation-http": "0.53.0", "@remix-run/router": "1.x", "@sentry/cli": "^2.35.0", "@sentry/core": "8.34.0", diff --git a/packages/remix/src/utils/integrations/http.ts b/packages/remix/src/utils/integrations/http.ts index d3a7ae03e351..be519a36806a 100644 --- a/packages/remix/src/utils/integrations/http.ts +++ b/packages/remix/src/utils/integrations/http.ts @@ -1,21 +1,6 @@ -// This integration is ported from the Next.JS SDK. -import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; import { httpIntegration as originalHttpIntegration } from '@sentry/node'; import type { IntegrationFn } from '@sentry/types'; -class RemixHttpIntegration extends HttpInstrumentation { - // Instead of the default behavior, we just don't do any wrapping for incoming requests - protected _getPatchIncomingRequestFunction(_component: 'http' | 'https') { - return ( - original: (event: string, ...args: unknown[]) => boolean, - ): ((this: unknown, event: string, ...args: unknown[]) => boolean) => { - return function incomingRequest(this: unknown, event: string, ...args: unknown[]): boolean { - return original.apply(this, [event, ...args]); - }; - }; - } -} - type HttpOptions = Parameters[0]; /** @@ -25,6 +10,7 @@ type HttpOptions = Parameters[0]; export const httpIntegration = ((options: HttpOptions = {}) => { return originalHttpIntegration({ ...options, - _instrumentation: RemixHttpIntegration, + // We disable incoming request spans here, because otherwise we'd end up with duplicate spans. + disableIncomingRequestSpans: true, }); }) satisfies IntegrationFn; diff --git a/yarn.lock b/yarn.lock index 63ee9ec0b8bd..5ab8ce9b9952 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9826,17 +9826,7 @@ dependencies: "@types/unist" "*" -"@types/history-4@npm:@types/history@4.7.8": - version "4.7.8" - resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" - integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== - -"@types/history-5@npm:@types/history@4.7.8": - version "4.7.8" - resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" - integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== - -"@types/history@*": +"@types/history-4@npm:@types/history@4.7.8", "@types/history-5@npm:@types/history@4.7.8", "@types/history@*": version "4.7.8" resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== @@ -10164,15 +10154,7 @@ "@types/history" "^3" "@types/react" "*" -"@types/react-router-4@npm:@types/react-router@5.1.14": - version "5.1.14" - resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.14.tgz#e0442f4eb4c446541ad7435d44a97f8fe6df40da" - integrity sha512-LAJpqYUaCTMT2anZheoidiIymt8MuX286zoVFPM3DVb23aQBH0mAkFvzpd4LKqiolV8bBtZWT5Qp7hClCNDENw== - dependencies: - "@types/history" "*" - "@types/react" "*" - -"@types/react-router-5@npm:@types/react-router@5.1.14": +"@types/react-router-4@npm:@types/react-router@5.1.14", "@types/react-router-5@npm:@types/react-router@5.1.14": version "5.1.14" resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.14.tgz#e0442f4eb4c446541ad7435d44a97f8fe6df40da" integrity sha512-LAJpqYUaCTMT2anZheoidiIymt8MuX286zoVFPM3DVb23aQBH0mAkFvzpd4LKqiolV8bBtZWT5Qp7hClCNDENw== @@ -20791,10 +20773,10 @@ import-in-the-middle@1.4.2: cjs-module-lexer "^1.2.2" module-details-from-path "^1.0.3" -import-in-the-middle@^1.11.0, import-in-the-middle@^1.8.1: - version "1.11.0" - resolved "https://registry.yarnpkg.com/import-in-the-middle/-/import-in-the-middle-1.11.0.tgz#a94c4925b8da18256cde3b3b7b38253e6ca5e708" - integrity sha512-5DimNQGoe0pLUHbR9qK84iWaWjjbsxiqXnw6Qz64+azRgleqv9k2kTt5fw7QsOpmaGYtuxxursnPPsnTKEx10Q== +import-in-the-middle@^1.11.2, import-in-the-middle@^1.8.1: + version "1.11.2" + resolved "https://registry.yarnpkg.com/import-in-the-middle/-/import-in-the-middle-1.11.2.tgz#dd848e72b63ca6cd7c34df8b8d97fc9baee6174f" + integrity sha512-gK6Rr6EykBcc6cVWRSBR5TWf8nn6hZMYSRYqCcHa0l0d1fPK7JSYo6+Mlmck76jIX9aL/IZ71c06U2VpFwl1zA== dependencies: acorn "^8.8.2" acorn-import-attributes "^1.9.5" @@ -28644,7 +28626,7 @@ react-is@^18.0.0: dependencies: "@remix-run/router" "1.0.2" -"react-router-6@npm:react-router@6.3.0": +"react-router-6@npm:react-router@6.3.0", react-router@6.3.0: version "6.3.0" resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== @@ -28659,13 +28641,6 @@ react-router-dom@^6.2.2: history "^5.2.0" react-router "6.3.0" -react-router@6.3.0: - version "6.3.0" - resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" - integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== - dependencies: - history "^5.2.0" - react@^18.0.0: version "18.0.0" resolved "https://registry.yarnpkg.com/react/-/react-18.0.0.tgz#b468736d1f4a5891f38585ba8e8fb29f91c3cb96" @@ -31114,16 +31089,7 @@ string-template@~0.2.1: resolved "https://registry.yarnpkg.com/string-template/-/string-template-0.2.1.tgz#42932e598a352d01fc22ec3367d9d84eec6c9add" integrity sha1-QpMuWYo1LQH8IuwzZ9nYTuxsmt0= -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -string-width@4.2.3, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", string-width@4.2.3, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -31235,14 +31201,7 @@ stringify-object@^3.2.1: is-obj "^1.0.1" is-regexp "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -34259,16 +34218,7 @@ wrangler@^3.67.1: optionalDependencies: fsevents "~2.3.2" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - -wrap-ansi@7.0.0, wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@7.0.0, wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== From 10533fe035d775a3a1117bd41cc5dcf0106ec21e Mon Sep 17 00:00:00 2001 From: Yevhenii Zakrepa Date: Fri, 11 Oct 2024 14:20:18 +0300 Subject: [PATCH 03/34] fix(module): keep version for node ESM package (#13922) Module federation needs package version for comparison fixes https://github.com/getsentry/sentry-javascript/discussions/12433 --- dev-packages/rollup-utils/plugins/make-esm-plugin.mjs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dev-packages/rollup-utils/plugins/make-esm-plugin.mjs b/dev-packages/rollup-utils/plugins/make-esm-plugin.mjs index 04dd68beaa1c..ad18856c011a 100644 --- a/dev-packages/rollup-utils/plugins/make-esm-plugin.mjs +++ b/dev-packages/rollup-utils/plugins/make-esm-plugin.mjs @@ -15,9 +15,12 @@ export function makePackageNodeEsm() { const packageJSON = JSON.parse(fs.readFileSync(packageJSONPath, 'utf-8')); const sideEffects = packageJSON.sideEffects; + // For module federation we need to keep the version of the package + const version = packageJSON.version; const newPackageJSON = { type: 'module', + version, sideEffects, }; From 26ec07532b9ae226a6a92672afb2de1ceb14a7e6 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 11 Oct 2024 13:27:54 +0200 Subject: [PATCH 04/34] ci: Ensure we check correctly for bot users (#13955) It seems that `github.actor` is not necessarily the PR author, but possibly the user that merged the PR. You can see e.g. here: https://github.com/getsentry/sentry-javascript/actions/runs/11270299315/job/31340702772 how it still ran for a dependabot PR. --- .github/workflows/external-contributors.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/external-contributors.yml b/.github/workflows/external-contributors.yml index 3d5954de8bfa..aac1805c1bb5 100644 --- a/.github/workflows/external-contributors.yml +++ b/.github/workflows/external-contributors.yml @@ -18,7 +18,7 @@ jobs: && github.event.pull_request.author_association != 'COLLABORATOR' && github.event.pull_request.author_association != 'MEMBER' && github.event.pull_request.author_association != 'OWNER' - && endsWith(github.actor, '[bot]') == false + && endsWith(github.event.pull_request.user.login, '[bot]') == false steps: - uses: actions/checkout@v4 - name: Set up Node From 9af0d849fc579f7066f60df9d45d99695bb9a380 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Fri, 11 Oct 2024 13:37:43 +0200 Subject: [PATCH 05/34] ref: Add external contributor to CHANGELOG.md (#13956) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13922 Co-authored-by: mydea <2411343+mydea@users.noreply.github.com> --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f32d3f4387b8..2abc4bbb61e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +Work in this release was contributed by @ZakrepaShe. Thank you for your contribution! + ## 8.34.0 ### Important Changes From 81df165b80a092630a41a498ee0a71d9e3a36c11 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 11 Oct 2024 15:42:25 +0200 Subject: [PATCH 06/34] feat(core): Make stream instrumentation opt-in (#13951) --- CHANGELOG.md | 6 +++ .../react-router-6/src/index.tsx | 1 + .../src/tracing/browserTracingIntegration.ts | 10 +++++ packages/browser/src/tracing/request.ts | 39 ++++++++++++++----- packages/browser/test/tracing/request.test.ts | 8 ++++ 5 files changed, 55 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2abc4bbb61e5..0a7cb97c8c4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +- **feat(core): Make stream instrumentation opt-in + ([#13951](https://github.com/getsentry/sentry-javascript/pull/13951))** + +This change adds a new option `trackFetchStreamPerformance` to the browser tracing integration. Only when set to `true`, +Sentry will instrument streams via fetch. + Work in this release was contributed by @ZakrepaShe. Thank you for your contribution! ## 8.34.0 diff --git a/dev-packages/e2e-tests/test-applications/react-router-6/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-6/src/index.tsx index 434c1677bf88..8c219563e5a4 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-6/src/index.tsx @@ -26,6 +26,7 @@ Sentry.init({ useNavigationType, createRoutesFromChildren, matchRoutes, + trackFetchStreamPerformance: true, }), replay, ], diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 523edd7e4262..b3f8d9abdba8 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -132,6 +132,14 @@ export interface BrowserTracingOptions { */ traceXHR: boolean; + /** + * Flag to disable tracking of long-lived streams, like server-sent events (SSE) via fetch. + * Do not enable this in case you have live streams or very long running streams. + * + * Default: false + */ + trackFetchStreamPerformance: boolean; + /** * If true, Sentry will capture http timings and add them to the corresponding http spans. * @@ -200,6 +208,7 @@ export const browserTracingIntegration = ((_options: Partial): void { - const { traceFetch, traceXHR, shouldCreateSpanForRequest, enableHTTPTimings, tracePropagationTargets } = { + const { + traceFetch, + traceXHR, + trackFetchStreamPerformance, + shouldCreateSpanForRequest, + enableHTTPTimings, + tracePropagationTargets, + } = { traceFetch: defaultRequestInstrumentationOptions.traceFetch, traceXHR: defaultRequestInstrumentationOptions.traceXHR, + trackFetchStreamPerformance: defaultRequestInstrumentationOptions.trackFetchStreamPerformance, ..._options, }; @@ -136,14 +155,16 @@ export function instrumentOutgoingRequests(client: Client, _options?: Partial { - if (handlerData.response) { - const span = responseToSpanId.get(handlerData.response); - if (span && handlerData.endTimestamp) { - spanIdToEndTimestamp.set(span, handlerData.endTimestamp); + if (trackFetchStreamPerformance) { + addFetchEndInstrumentationHandler(handlerData => { + if (handlerData.response) { + const span = responseToSpanId.get(handlerData.response); + if (span && handlerData.endTimestamp) { + spanIdToEndTimestamp.set(span, handlerData.endTimestamp); + } } - } - }); + }); + } addFetchInstrumentationHandler(handlerData => { const createdSpan = instrumentFetchRequest(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); diff --git a/packages/browser/test/tracing/request.test.ts b/packages/browser/test/tracing/request.test.ts index f1067c9e4b52..4f1c0bfdaf0a 100644 --- a/packages/browser/test/tracing/request.test.ts +++ b/packages/browser/test/tracing/request.test.ts @@ -54,6 +54,14 @@ describe('instrumentOutgoingRequests', () => { expect(addXhrSpy).not.toHaveBeenCalled(); }); + + it('does instrument streaming requests if trackFetchStreamPerformance is true', () => { + const addFetchEndSpy = vi.spyOn(utils, 'addFetchEndInstrumentationHandler'); + + instrumentOutgoingRequests(client, { trackFetchStreamPerformance: true }); + + expect(addFetchEndSpy).toHaveBeenCalledWith(expect.any(Function)); + }); }); interface ProtocolInfo { From 8dd854fd09e86d79ff4a8b8183951dc0157aa738 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E5=85=BD=E5=85=BD?= Date: Fri, 11 Oct 2024 10:02:23 -0400 Subject: [PATCH 07/34] feat(node): Expose `suppressTracing` API (#13875) --- packages/astro/src/index.server.ts | 1 + packages/aws-serverless/src/index.ts | 1 + packages/browser/src/exports.ts | 1 + packages/bun/src/index.ts | 1 + packages/cloudflare/src/index.ts | 1 + packages/deno/src/index.ts | 1 + packages/google-cloud-serverless/src/index.ts | 1 + packages/node/src/index.ts | 1 + packages/remix/src/index.server.ts | 1 + packages/solidstart/src/server/index.ts | 1 + packages/sveltekit/src/server/index.ts | 1 + packages/vercel-edge/src/index.ts | 1 + 12 files changed, 12 insertions(+) diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 91ea79f833bc..dacb42643b99 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -119,6 +119,7 @@ export { spotlightIntegration, startInactiveSpan, startNewTrace, + suppressTracing, startSession, startSpan, startSpanManual, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 7b0e05c9a48f..ee70e8956c6f 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -66,6 +66,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + suppressTracing, withActiveSpan, getRootSpan, getSpanDescendants, diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 94b4bd0b3c2a..fe5179f77661 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -40,6 +40,7 @@ export { setCurrentClient, Scope, continueTrace, + suppressTracing, SDK_VERSION, setContext, setExtra, diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index 7026e6800b14..7ccca26a3da6 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -87,6 +87,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + suppressTracing, withActiveSpan, getRootSpan, getSpanDescendants, diff --git a/packages/cloudflare/src/index.ts b/packages/cloudflare/src/index.ts index 0c02fb8ca810..7c0aa6018313 100644 --- a/packages/cloudflare/src/index.ts +++ b/packages/cloudflare/src/index.ts @@ -61,6 +61,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + suppressTracing, withActiveSpan, getSpanDescendants, continueTrace, diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index 1f983b476b74..8a0a2a49fdcc 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -61,6 +61,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + suppressTracing, metricsDefault as metrics, inboundFiltersIntegration, linkedErrorsIntegration, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index c2d743eb1bf1..bda96f062966 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -67,6 +67,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + suppressTracing, withActiveSpan, getRootSpan, getSpanDescendants, diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index bc63094e2e87..f77ef88548f9 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -124,6 +124,7 @@ export { startSpanManual, startInactiveSpan, startNewTrace, + suppressTracing, getActiveSpan, withActiveSpan, getRootSpan, diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index 9218933b9896..098bd1293080 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -119,6 +119,7 @@ export { spotlightIntegration, startInactiveSpan, startNewTrace, + suppressTracing, startSession, startSpan, startSpanManual, diff --git a/packages/solidstart/src/server/index.ts b/packages/solidstart/src/server/index.ts index b22fbbe1de0d..d537ddd51e88 100644 --- a/packages/solidstart/src/server/index.ts +++ b/packages/solidstart/src/server/index.ts @@ -110,6 +110,7 @@ export { spotlightIntegration, startInactiveSpan, startNewTrace, + suppressTracing, startSession, startSpan, startSpanManual, diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index e9fb6f256192..72b459e2fde3 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -112,6 +112,7 @@ export { spotlightIntegration, startInactiveSpan, startNewTrace, + suppressTracing, startSession, startSpan, startSpanManual, diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index 8be93345fa3d..7eb7893e974a 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -61,6 +61,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + suppressTracing, withActiveSpan, getSpanDescendants, continueTrace, From e1d7a9dfa16287a850483ff67c6c36863608db32 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 14 Oct 2024 08:38:50 +0200 Subject: [PATCH 08/34] test(loader): Update Loader Script & test error in `sentryOnLoad` (#13952) Updates the loader & adds tests for https://github.com/getsentry/sentry/pull/78993 --- .../fixtures/loader.js | 2 +- .../loader/onLoad/sentryOnLoadError/init.js | 1 + .../onLoad/sentryOnLoadError/subject.js | 1 + .../onLoad/sentryOnLoadError/template.html | 16 ++++++++++ .../loader/onLoad/sentryOnLoadError/test.ts | 31 +++++++++++++++++++ 5 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 dev-packages/browser-integration-tests/loader-suites/loader/onLoad/sentryOnLoadError/init.js create mode 100644 dev-packages/browser-integration-tests/loader-suites/loader/onLoad/sentryOnLoadError/subject.js create mode 100644 dev-packages/browser-integration-tests/loader-suites/loader/onLoad/sentryOnLoadError/template.html create mode 100644 dev-packages/browser-integration-tests/loader-suites/loader/onLoad/sentryOnLoadError/test.ts diff --git a/dev-packages/browser-integration-tests/fixtures/loader.js b/dev-packages/browser-integration-tests/fixtures/loader.js index 8e30865d35ad..ba4c048cf48f 100644 --- a/dev-packages/browser-integration-tests/fixtures/loader.js +++ b/dev-packages/browser-integration-tests/fixtures/loader.js @@ -1,4 +1,4 @@ -!function(n,e,r,t,i,o,a,c,s){for(var u=s,f=0;f-1){u&&"no"===document.scripts[f].getAttribute("data-lazy")&&(u=!1);break}var p=[];function l(n){return"e"in n}function d(n){return"p"in n}function _(n){return"f"in n}var v=[];function y(n){u&&(l(n)||d(n)||_(n)&&n.f.indexOf("capture")>-1||_(n)&&n.f.indexOf("showReportDialog")>-1)&&m(),v.push(n)}function g(){y({e:[].slice.call(arguments)})}function h(n){y({p:n})}function E(){try{n.SENTRY_SDK_SOURCE="loader";var e=n[i],o=e.init;e.init=function(i){n.removeEventListener(r,g),n.removeEventListener(t,h);var a=c;for(var s in i)Object.prototype.hasOwnProperty.call(i,s)&&(a[s]=i[s]);!function(n,e){var r=n.integrations||[];if(!Array.isArray(r))return;var t=r.map((function(n){return n.name}));n.tracesSampleRate&&-1===t.indexOf("BrowserTracing")&&(e.browserTracingIntegration?r.push(e.browserTracingIntegration({enableInp:!0})):e.BrowserTracing&&r.push(new e.BrowserTracing));(n.replaysSessionSampleRate||n.replaysOnErrorSampleRate)&&-1===t.indexOf("Replay")&&(e.replayIntegration?r.push(e.replayIntegration()):e.Replay&&r.push(new e.Replay));n.integrations=r}(a,e),o(a)},setTimeout((function(){return function(e){try{"function"==typeof n.sentryOnLoad&&(n.sentryOnLoad(),n.sentryOnLoad=void 0);for(var r=0;r-1){u&&"no"===document.scripts[f].getAttribute("data-lazy")&&(u=!1);break}var p=[];function l(n){return"e"in n}function d(n){return"p"in n}function _(n){return"f"in n}var v=[];function y(n){u&&(l(n)||d(n)||_(n)&&n.f.indexOf("capture")>-1||_(n)&&n.f.indexOf("showReportDialog")>-1)&&L(),v.push(n)}function h(){y({e:[].slice.call(arguments)})}function g(n){y({p:n})}function E(){try{n.SENTRY_SDK_SOURCE="loader";var e=n[o],i=e.init;e.init=function(o){n.removeEventListener(r,h),n.removeEventListener(t,g);var a=c;for(var s in o)Object.prototype.hasOwnProperty.call(o,s)&&(a[s]=o[s]);!function(n,e){var r=n.integrations||[];if(!Array.isArray(r))return;var t=r.map((function(n){return n.name}));n.tracesSampleRate&&-1===t.indexOf("BrowserTracing")&&(e.browserTracingIntegration?r.push(e.browserTracingIntegration({enableInp:!0})):e.BrowserTracing&&r.push(new e.BrowserTracing));(n.replaysSessionSampleRate||n.replaysOnErrorSampleRate)&&-1===t.indexOf("Replay")&&(e.replayIntegration?r.push(e.replayIntegration()):e.Replay&&r.push(new e.Replay));n.integrations=r}(a,e),i(a)},setTimeout((function(){return function(e){try{"function"==typeof n.sentryOnLoad&&(n.sentryOnLoad(),n.sentryOnLoad=void 0)}catch(n){console.error("Error while calling `sentryOnLoad` handler:"),console.error(n)}try{for(var r=0;r + + + + + + + diff --git a/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/sentryOnLoadError/test.ts b/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/sentryOnLoadError/test.ts new file mode 100644 index 000000000000..ab2616ef382d --- /dev/null +++ b/dev-packages/browser-integration-tests/loader-suites/loader/onLoad/sentryOnLoadError/test.ts @@ -0,0 +1,31 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers'; + +sentryTest( + 'sentryOnLoad callback is called before Sentry.onLoad() and handles errors in handler', + async ({ getLocalTestUrl, page }) => { + const errors: string[] = []; + + page.on('console', msg => { + if (msg.type() === 'error') { + errors.push(msg.text()); + } + }); + + const url = await getLocalTestUrl({ testDir: __dirname }); + const req = await waitForErrorRequestOnUrl(page, url); + + const eventData = envelopeRequestParser(req); + + expect(eventData.message).toBe('Test exception'); + + expect(await page.evaluate('Sentry.getClient().getOptions().tracesSampleRate')).toEqual(0.123); + + expect(errors).toEqual([ + 'Error while calling `sentryOnLoad` handler:', + expect.stringContaining('Error: sentryOnLoad error'), + ]); + }, +); From eb40643d674d418739f76025359af3e290c73f38 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 14 Oct 2024 08:39:29 +0200 Subject: [PATCH 09/34] dev(e2e): Fix nestjs version constraint (#13964) See #13744 for more information --- .../test-applications/nestjs-basic-with-graphql/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/package.json b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/package.json index 45eccd244d9b..62606e825e33 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/package.json +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/package.json @@ -21,7 +21,7 @@ "@nestjs/core": "^10.3.10", "@nestjs/graphql": "^12.2.0", "@nestjs/platform-express": "^10.3.10", - "@sentry/nestjs": "^8.21.0", + "@sentry/nestjs": "latest || *", "graphql": "^16.9.0", "reflect-metadata": "^0.1.13", "rxjs": "^7.8.1" From 0ccf8ceb832a9dbb05e30c2e59635e9efa187597 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 14 Oct 2024 08:42:11 +0200 Subject: [PATCH 10/34] ref: Add external contributor to CHANGELOG.md (#13959) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13875 Co-authored-by: mydea <2411343+mydea@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a7cb97c8c4f..b614cc109c2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ This change adds a new option `trackFetchStreamPerformance` to the browser tracing integration. Only when set to `true`, Sentry will instrument streams via fetch. -Work in this release was contributed by @ZakrepaShe. Thank you for your contribution! +Work in this release was contributed by @ZakrepaShe and @zhiyan114. Thank you for your contributions! ## 8.34.0 From 50ab94883dc527961cbf275c6b5700326d37da92 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 14 Oct 2024 14:53:04 +0200 Subject: [PATCH 11/34] test(browser): Add test for current DSC transaction name updating behavior (#13953) Add a browser integration test that describes the current behaviour of how we update the `transaction` field in the DSC which is propagated via the `baggage` header and added to envelopes as the `trace` header. --- .../tracing/dsc-txn-name-update/init.js | 11 ++ .../tracing/dsc-txn-name-update/subject.js | 34 ++++ .../tracing/dsc-txn-name-update/template.html | 5 + .../tracing/dsc-txn-name-update/test.ts | 185 ++++++++++++++++++ .../trace-lifetime/pageload-meta/subject.js | 16 ++ .../tracing/trace-lifetime/pageload/test.ts | 143 ++++++++++++++ .../suites/tracing/trace-lifetime/subject.js | 7 + .../tracing/trace-lifetime/template.html | 1 + .../dsc-txn-name-update/scenario-events.ts | 29 +++ .../dsc-txn-name-update/scenario-headers.ts | 45 +++++ .../tracing/dsc-txn-name-update/test.ts | 123 ++++++++++++ 11 files changed, 599 insertions(+) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/subject.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/scenario-events.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/scenario-headers.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/init.js b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/init.js new file mode 100644 index 000000000000..9c7cdb7e11b6 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/init.js @@ -0,0 +1,11 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration({ instrumentNavigation: false, instrumentPageLoad: false })], + tracesSampleRate: 1, + tracePropagationTargets: ['example.com'], + release: '1.1.1', +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/subject.js b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/subject.js new file mode 100644 index 000000000000..163082710a13 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/subject.js @@ -0,0 +1,34 @@ +const btnStartSpan = document.getElementById('btnStartSpan'); +const btnUpdateName = document.getElementById('btnUpdateName'); +const btnMakeRequest = document.getElementById('btnMakeRequest'); +const btnCaptureError = document.getElementById('btnCaptureError'); +const btnEndSpan = document.getElementById('btnEndSpan'); + +btnStartSpan.addEventListener('click', () => { + Sentry.startSpanManual( + { name: 'test-root-span', attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } }, + async span => { + window.__traceId = span.spanContext().traceId; + await new Promise(resolve => { + btnEndSpan.addEventListener('click', resolve); + }); + span.end(); + }, + ); +}); + +let updateCnt = 0; +btnUpdateName.addEventListener('click', () => { + const span = Sentry.getActiveSpan(); + const rootSpan = Sentry.getRootSpan(span); + rootSpan.updateName(`updated-root-span-${++updateCnt}`); + rootSpan.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); +}); + +btnMakeRequest.addEventListener('click', () => { + fetch('https://example.com/api'); +}); + +btnCaptureError.addEventListener('click', () => { + Sentry.captureException(new Error('test-error')); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/template.html b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/template.html new file mode 100644 index 000000000000..9ad1d0cfe584 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/template.html @@ -0,0 +1,5 @@ + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts new file mode 100644 index 000000000000..e8c21a66647f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts @@ -0,0 +1,185 @@ +import { expect } from '@playwright/test'; +import type { Page } from '@playwright/test'; +import type { DynamicSamplingContext } from '@sentry/types'; +import { sentryTest } from '../../../utils/fixtures'; +import type { EventAndTraceHeader } from '../../../utils/helpers'; +import { + eventAndTraceHeaderRequestParser, + getMultipleSentryEnvelopeRequests, + shouldSkipTracingTest, +} from '../../../utils/helpers'; + +sentryTest('updates the DSC when the txn name is updated and high-quality', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + + /* + Test Steps: + 1. Start new span with LQ txn name (source: url) + 2. Make request and check that baggage has no transaction name + 3. Capture error and check that envelope trace header has no transaction name + 4. Update span name and source to HQ (source: route) + 5. Make request and check that baggage has HQ txn name + 6. Capture error and check that envelope trace header has HQ txn name + 7. Update span name again with HQ name (source: route) + 8. Make request and check that baggage has updated HQ txn name + 9. Capture error and check that envelope trace header has updated HQ txn name + 10. End span and check that envelope trace header has updated HQ txn name + 11. Make another request and check that there's no span information in baggage + 12. Capture an error and check that envelope trace header has no span information + */ + + // 1 + await page.locator('#btnStartSpan').click(); + const traceId = await page.evaluate(() => { + return (window as any).__traceId; + }); + + expect(traceId).toMatch(/^[0-9a-f]{32}$/); + + // 2 + const baggageItems = await makeRequestAndGetBaggageItems(page); + expect(baggageItems).toEqual([ + 'sentry-environment=production', + 'sentry-public_key=public', + 'sentry-release=1.1.1', + 'sentry-sample_rate=1', + 'sentry-sampled=true', + `sentry-trace_id=${traceId}`, + ]); + + // 3 + const errorEnvelopeTraceHeader = await captureErrorAndGetEnvelopeTraceHeader(page); + expect(errorEnvelopeTraceHeader).toEqual({ + environment: 'production', + public_key: 'public', + release: '1.1.1', + sample_rate: '1', + sampled: 'true', + trace_id: traceId, + }); + + // 4 + await page.locator('#btnUpdateName').click(); + + // 5 + const baggageItemsAfterUpdate = await makeRequestAndGetBaggageItems(page); + expect(baggageItemsAfterUpdate).toEqual([ + 'sentry-environment=production', + 'sentry-public_key=public', + 'sentry-release=1.1.1', + 'sentry-sample_rate=1', + 'sentry-sampled=true', + `sentry-trace_id=${traceId}`, + 'sentry-transaction=updated-root-span-1', + ]); + + // 6 + const errorEnvelopeTraceHeaderAfterUpdate = await captureErrorAndGetEnvelopeTraceHeader(page); + expect(errorEnvelopeTraceHeaderAfterUpdate).toEqual({ + environment: 'production', + public_key: 'public', + release: '1.1.1', + sample_rate: '1', + sampled: 'true', + trace_id: traceId, + transaction: 'updated-root-span-1', + }); + + // 7 + await page.locator('#btnUpdateName').click(); + + // 8 + const baggageItemsAfterSecondUpdate = await makeRequestAndGetBaggageItems(page); + expect(baggageItemsAfterSecondUpdate).toEqual([ + 'sentry-environment=production', + 'sentry-public_key=public', + 'sentry-release=1.1.1', + 'sentry-sample_rate=1', + 'sentry-sampled=true', + `sentry-trace_id=${traceId}`, + 'sentry-transaction=updated-root-span-2', + ]); + + // 9 + const errorEnvelopeTraceHeaderAfterSecondUpdate = await captureErrorAndGetEnvelopeTraceHeader(page); + expect(errorEnvelopeTraceHeaderAfterSecondUpdate).toEqual({ + environment: 'production', + public_key: 'public', + release: '1.1.1', + sample_rate: '1', + sampled: 'true', + trace_id: traceId, + transaction: 'updated-root-span-2', + }); + + // 10 + const txnEventPromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'transaction' }, + eventAndTraceHeaderRequestParser, + ); + + await page.locator('#btnEndSpan').click(); + + const [txnEvent, txnEnvelopeTraceHeader] = (await txnEventPromise)[0]; + expect(txnEnvelopeTraceHeader).toEqual({ + environment: 'production', + public_key: 'public', + release: '1.1.1', + sample_rate: '1', + sampled: 'true', + trace_id: traceId, + transaction: 'updated-root-span-2', + }); + + expect(txnEvent.transaction).toEqual('updated-root-span-2'); + + // 11 + const baggageItemsAfterEnd = await makeRequestAndGetBaggageItems(page); + expect(baggageItemsAfterEnd).toEqual([ + 'sentry-environment=production', + 'sentry-public_key=public', + 'sentry-release=1.1.1', + `sentry-trace_id=${traceId}`, + ]); + + // 12 + const errorEnvelopeTraceHeaderAfterEnd = await captureErrorAndGetEnvelopeTraceHeader(page); + expect(errorEnvelopeTraceHeaderAfterEnd).toEqual({ + environment: 'production', + public_key: 'public', + release: '1.1.1', + trace_id: traceId, + }); +}); + +async function makeRequestAndGetBaggageItems(page: Page): Promise { + const requestPromise = page.waitForRequest('https://example.com/*'); + await page.locator('#btnMakeRequest').click(); + const request = await requestPromise; + + const baggage = await request.headerValue('baggage'); + + return baggage?.split(',').sort() ?? []; +} + +async function captureErrorAndGetEnvelopeTraceHeader(page: Page): Promise { + const errorEventPromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'event' }, + eventAndTraceHeaderRequestParser, + ); + + await page.locator('#btnCaptureError').click(); + + const [, errorEnvelopeTraceHeader] = (await errorEventPromise)[0]; + return errorEnvelopeTraceHeader; +} diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/subject.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/subject.js new file mode 100644 index 000000000000..9528f861a723 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/subject.js @@ -0,0 +1,16 @@ +const errorBtn = document.getElementById('errorBtn'); +errorBtn.addEventListener('click', () => { + throw new Error(`Sentry Test Error ${Math.random()}`); +}); + +const fetchBtn = document.getElementById('fetchBtn'); +fetchBtn.addEventListener('click', async () => { + await fetch('http://example.com'); +}); + +const xhrBtn = document.getElementById('xhrBtn'); +xhrBtn.addEventListener('click', () => { + const xhr = new XMLHttpRequest(); + xhr.open('GET', 'http://example.com'); + xhr.send(); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts index 2d0933002e7f..5ef3cb81ad28 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts @@ -294,6 +294,149 @@ sentryTest( }, ); +// sentryTest( +// 'outgoing fetch request after pageload has pageload traceId in headers', +// async ({ getLocalTestUrl, page }) => { +// if (shouldSkipTracingTest()) { +// sentryTest.skip(); +// } + +// const url = await getLocalTestUrl({ testDir: __dirname }); + +// await page.route('http://example.com/**', route => { +// return route.fulfill({ +// status: 200, +// contentType: 'application/json', +// body: JSON.stringify({}), +// }); +// }); + +// const pageloadEventPromise = getFirstSentryEnvelopeRequest( +// page, +// undefined, +// eventAndTraceHeaderRequestParser, +// ); +// await page.goto(url); +// const [pageloadEvent, pageloadTraceHeader] = await pageloadEventPromise; + +// const pageloadTraceContext = pageloadEvent.contexts?.trace; +// const pageloadTraceId = pageloadTraceContext?.trace_id; + +// expect(pageloadEvent.type).toEqual('transaction'); +// expect(pageloadTraceContext).toMatchObject({ +// op: 'pageload', +// trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), +// span_id: expect.stringMatching(/^[0-9a-f]{16}$/), +// }); +// expect(pageloadTraceContext).not.toHaveProperty('parent_span_id'); + +// expect(pageloadTraceHeader).toEqual({ +// environment: 'production', +// public_key: 'public', +// sample_rate: '1', +// sampled: 'true', +// trace_id: pageloadTraceId, +// }); + +// const requestPromise = page.waitForRequest('http://example.com/*'); +// await page.locator('#xhrBtn').click(); +// const request = await requestPromise; + +// const headers = request.headers(); + +// // sampling decision is propagated from active span sampling decision +// expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); +// expect(headers['baggage']).toEqual( +// `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`, +// ); +// }, +// ) + +// sentryTest( +// 'custom span and request headers after pageload have pageload traceId ', +// async ({ getLocalTestUrl, page }) => { +// if (shouldSkipTracingTest()) { +// sentryTest.skip(); +// } + +// const url = await getLocalTestUrl({ testDir: __dirname }); + +// await page.route('http://example.com/**', route => { +// return route.fulfill({ +// status: 200, +// contentType: 'application/json', +// body: JSON.stringify({}), +// }); +// }); + +// const pageloadEventPromise = getFirstSentryEnvelopeRequest( +// page, +// undefined, +// eventAndTraceHeaderRequestParser, +// ); + +// await page.goto(url); + +// const [pageloadEvent, pageloadTraceHeader] = await pageloadEventPromise; + +// const pageloadTraceContext = pageloadEvent.contexts?.trace; +// const pageloadTraceId = pageloadTraceContext?.trace_id; + +// expect(pageloadEvent.type).toEqual('transaction'); +// expect(pageloadTraceContext).toMatchObject({ +// op: 'pageload', +// trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), +// span_id: expect.stringMatching(/^[0-9a-f]{16}$/), +// }); +// expect(pageloadTraceContext).not.toHaveProperty('parent_span_id'); + +// expect(pageloadTraceHeader).toEqual({ +// environment: 'production', +// public_key: 'public', +// sample_rate: '1', +// sampled: 'true', +// trace_id: pageloadTraceId, +// }); + +// const requestPromise = page.waitForRequest('http://example.com/**'); +// const customTransactionEventPromise = getFirstSentryEnvelopeRequest( +// page, +// undefined, +// eventAndTraceHeaderRequestParser, +// ); + +// await page.locator('#spanAndFetchBtn').click(); + +// const [[customTransactionEvent, customTransactionTraceHeader], request] = await Promise.all([ +// customTransactionEventPromise, +// requestPromise, +// ]); + +// const customTransactionTraceContext = customTransactionEvent.contexts?.trace; + +// expect(customTransactionEvent.type).toEqual('transaction'); +// expect(customTransactionTraceContext).toMatchObject({ +// trace_id: pageloadTraceId, +// }); + +// expect(customTransactionTraceHeader).toEqual({ +// environment: 'production', +// public_key: 'public', +// sample_rate: '1', +// sampled: 'true', +// trace_id: pageloadTraceId, +// }); + +// const headers = request.headers(); + +// // sampling decision is propagated from active span sampling decision +// expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); +// expect(headers['baggage']).toEqual( +// `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`, +// ); +// }, +// ); + sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) { sentryTest.skip(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js index 9528f861a723..a2f6271463ce 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js @@ -14,3 +14,10 @@ xhrBtn.addEventListener('click', () => { xhr.open('GET', 'http://example.com'); xhr.send(); }); + +const spanAndFetchBtn = document.getElementById('spanAndFetchBtn'); +spanAndFetchBtn.addEventListener('click', () => { + Sentry.startSpan({ name: 'custom-root-span' }, async () => { + await fetch('http://example.com'); + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html index a3c17f442605..a112e5c46771 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html @@ -7,5 +7,6 @@ + diff --git a/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/scenario-events.ts b/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/scenario-events.ts new file mode 100644 index 000000000000..892167fa55b4 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/scenario-events.ts @@ -0,0 +1,29 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan( + { name: 'initial-name', attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } }, + async span => { + Sentry.captureMessage('message-1'); + + span.updateName('updated-name-1'); + span.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + + Sentry.captureMessage('message-2'); + + span.updateName('updated-name-2'); + span.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom'); + + Sentry.captureMessage('message-3'); + + span.end(); + }, +); diff --git a/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/scenario-headers.ts b/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/scenario-headers.ts new file mode 100644 index 000000000000..8c9c01e21444 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/scenario-headers.ts @@ -0,0 +1,45 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +import * as http from 'http'; + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan( + { name: 'initial-name', attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } }, + async span => { + await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`); + + span.updateName('updated-name-1'); + span.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + + await makeHttpRequest(`${process.env.SERVER_URL}/api/v1`); + + span.updateName('updated-name-2'); + span.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom'); + await makeHttpRequest(`${process.env.SERVER_URL}/api/v2`); + + span.end(); + }, +); + +function makeHttpRequest(url: string): Promise { + return new Promise(resolve => { + http + .request(url, httpRes => { + httpRes.on('data', () => { + // we don't care about data + }); + httpRes.on('end', () => { + resolve(); + }); + }) + .end(); + }); +} diff --git a/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/test.ts b/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/test.ts new file mode 100644 index 000000000000..cefaba1ad97f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/dsc-txn-name-update/test.ts @@ -0,0 +1,123 @@ +import { createRunner } from '../../../utils/runner'; +import { createTestServer } from '../../../utils/server'; + +test('adds current transaction name to baggage when the txn name is high-quality', done => { + expect.assertions(5); + + let traceId: string | undefined; + + createTestServer(done) + .get('/api/v0', headers => { + const baggageItems = getBaggageHeaderItems(headers); + traceId = baggageItems.find(item => item.startsWith('sentry-trace_id='))?.split('=')[1] as string; + + expect(traceId).toMatch(/^[0-9a-f]{32}$/); + + expect(baggageItems).toEqual([ + 'sentry-environment=production', + 'sentry-public_key=public', + 'sentry-release=1.0', + 'sentry-sample_rate=1', + 'sentry-sampled=true', + `sentry-trace_id=${traceId}`, + ]); + }) + .get('/api/v1', headers => { + expect(getBaggageHeaderItems(headers)).toEqual([ + 'sentry-environment=production', + 'sentry-public_key=public', + 'sentry-release=1.0', + 'sentry-sample_rate=1', + 'sentry-sampled=true', + `sentry-trace_id=${traceId}`, + 'sentry-transaction=updated-name-1', + ]); + }) + .get('/api/v2', headers => { + expect(getBaggageHeaderItems(headers)).toEqual([ + 'sentry-environment=production', + 'sentry-public_key=public', + 'sentry-release=1.0', + 'sentry-sample_rate=1', + 'sentry-sampled=true', + `sentry-trace_id=${traceId}`, + 'sentry-transaction=updated-name-2', + ]); + }) + .start() + .then(([SERVER_URL, closeTestServer]) => { + createRunner(__dirname, 'scenario-headers.ts') + .withEnv({ SERVER_URL }) + .expect({ + transaction: {}, + }) + .start(closeTestServer); + }); +}); + +test('adds current transaction name to trace envelope header when the txn name is high-quality', done => { + expect.assertions(4); + + createRunner(__dirname, 'scenario-events.ts') + .expectHeader({ + event: { + trace: { + environment: 'production', + public_key: 'public', + release: '1.0', + sample_rate: '1', + sampled: 'true', + trace_id: expect.any(String), + }, + }, + }) + .expectHeader({ + event: { + trace: { + environment: 'production', + public_key: 'public', + release: '1.0', + sample_rate: '1', + sampled: 'true', + trace_id: expect.any(String), + transaction: 'updated-name-1', + }, + }, + }) + .expectHeader({ + event: { + trace: { + environment: 'production', + public_key: 'public', + release: '1.0', + sample_rate: '1', + sampled: 'true', + trace_id: expect.any(String), + transaction: 'updated-name-2', + }, + }, + }) + .expectHeader({ + transaction: { + trace: { + environment: 'production', + public_key: 'public', + release: '1.0', + sample_rate: '1', + sampled: 'true', + trace_id: expect.any(String), + transaction: 'updated-name-2', + }, + }, + }) + .start(done); +}); + +function getBaggageHeaderItems(headers: Record) { + const baggage = headers['baggage'] as string; + const baggageItems = baggage + .split(',') + .map(b => b.trim()) + .sort(); + return baggageItems; +} From 8249a5e50e67b25fe29a5260dec5d70a814c4df2 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 14 Oct 2024 16:52:17 +0200 Subject: [PATCH 12/34] feat(nuxt): Add Rollup plugin to wrap server entry with `import()` (#13945) Feature Issue: https://github.com/getsentry/sentry-javascript/issues/13943 Adds a Rollup plugin to wrap the server entry with `import()` to load it after Sentry was initialized. The plugin is not yet in use (will do this in another PR - see linked issue above) --- packages/nuxt/src/vite/addServerConfig.ts | 117 +++++++++++++++++++++- packages/nuxt/src/vite/utils.ts | 56 +++++++++++ packages/nuxt/test/vite/utils.test.ts | 70 ++++++++++++- 3 files changed, 238 insertions(+), 5 deletions(-) diff --git a/packages/nuxt/src/vite/addServerConfig.ts b/packages/nuxt/src/vite/addServerConfig.ts index 845228c58b0c..5a5d2e5bd627 100644 --- a/packages/nuxt/src/vite/addServerConfig.ts +++ b/packages/nuxt/src/vite/addServerConfig.ts @@ -3,7 +3,17 @@ import { createResolver } from '@nuxt/kit'; import type { Nuxt } from '@nuxt/schema'; import { consoleSandbox } from '@sentry/utils'; import type { Nitro } from 'nitropack'; +import type { InputPluginOption } from 'rollup'; import type { SentryNuxtModuleOptions } from '../common/types'; +import { + QUERY_END_INDICATOR, + SENTRY_FUNCTIONS_REEXPORT, + SENTRY_WRAPPED_ENTRY, + constructFunctionReExport, + removeSentryQueryFromPath, +} from './utils'; + +const SERVER_CONFIG_FILENAME = 'sentry.server.config'; /** * Adds the `sentry.server.config.ts` file as `sentry.server.config.mjs` to the `.output` directory to be able to reference this file in the node --import option. @@ -23,7 +33,7 @@ export function addServerConfigToBuild( 'server' in viteInlineConfig.build.rollupOptions.input ) { // Create a rollup entry for the server config to add it as `sentry.server.config.mjs` to the build - (viteInlineConfig.build.rollupOptions.input as { [entryName: string]: string })['sentry.server.config'] = + (viteInlineConfig.build.rollupOptions.input as { [entryName: string]: string })[SERVER_CONFIG_FILENAME] = createResolver(nuxt.options.srcDir).resolve(`/${serverConfigFile}`); } @@ -34,8 +44,8 @@ export function addServerConfigToBuild( nitro.hooks.hook('close', async () => { const buildDirResolver = createResolver(nitro.options.buildDir); const serverDirResolver = createResolver(nitro.options.output.serverDir); - const source = buildDirResolver.resolve('dist/server/sentry.server.config.mjs'); - const destination = serverDirResolver.resolve('sentry.server.config.mjs'); + const source = buildDirResolver.resolve(`dist/server/${SERVER_CONFIG_FILENAME}.mjs`); + const destination = serverDirResolver.resolve(`${SERVER_CONFIG_FILENAME}.mjs`); try { await fs.promises.access(source, fs.constants.F_OK); @@ -85,7 +95,7 @@ export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nitro try { fs.readFile(entryFilePath, 'utf8', (err, data) => { - const updatedContent = `import './sentry.server.config.mjs';\n${data}`; + const updatedContent = `import './${SERVER_CONFIG_FILENAME}.mjs';\n${data}`; fs.writeFile(entryFilePath, updatedContent, 'utf8', () => { if (moduleOptions.debug) { @@ -111,3 +121,102 @@ export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nitro } }); } + +/** + * This function modifies the Rollup configuration to include a plugin that wraps the entry file with a dynamic import (`import()`) + * and adds the Sentry server config with the static `import` declaration. + * + * With this, the Sentry server config can be loaded before all other modules of the application (which is needed for import-in-the-middle). + * See: https://nodejs.org/api/module.html#enabling + */ +export function addDynamicImportEntryFileWrapper(nitro: Nitro, serverConfigFile: string): void { + if (!nitro.options.rollupConfig) { + nitro.options.rollupConfig = { output: {} }; + } + + if (nitro.options.rollupConfig?.plugins === null || nitro.options.rollupConfig?.plugins === undefined) { + nitro.options.rollupConfig.plugins = []; + } else if (!Array.isArray(nitro.options.rollupConfig.plugins)) { + // `rollupConfig.plugins` can be a single plugin, so we want to put it into an array so that we can push our own plugin + nitro.options.rollupConfig.plugins = [nitro.options.rollupConfig.plugins]; + } + + nitro.options.rollupConfig.plugins.push( + // @ts-expect-error - This is the correct type, but it shows an error because of two different definitions + wrapEntryWithDynamicImport(createResolver(nitro.options.srcDir).resolve(`/${serverConfigFile}`)), + ); +} + +/** + * A Rollup plugin which wraps the server entry with a dynamic `import()`. This makes it possible to initialize Sentry first + * by using a regular `import` and load the server after that. + * This also works with serverless `handler` functions, as it re-exports the `handler`. + */ +function wrapEntryWithDynamicImport(resolvedSentryConfigPath: string): InputPluginOption { + return { + name: 'sentry-wrap-entry-with-dynamic-import', + async resolveId(source, importer, options) { + if (source.includes(`/${SERVER_CONFIG_FILENAME}`)) { + return { id: source, moduleSideEffects: true }; + } + + if (source === 'import-in-the-middle/hook.mjs') { + // We are importing "import-in-the-middle" in the returned code of the `load()` function below + // By setting `moduleSideEffects` to `true`, the import is added to the bundle, although nothing is imported from it + // By importing "import-in-the-middle/hook.mjs", we can make sure this file is included, as not all node builders are including files imported with `module.register()`. + // Prevents the error "Failed to register ESM hook Error: Cannot find module 'import-in-the-middle/hook.mjs'" + return { id: source, moduleSideEffects: true, external: true }; + } + + if (options.isEntry && !source.includes(`.mjs${SENTRY_WRAPPED_ENTRY}`)) { + const resolution = await this.resolve(source, importer, options); + + // If it cannot be resolved or is external, just return it so that Rollup can display an error + if (!resolution || resolution?.external) return resolution; + + const moduleInfo = await this.load(resolution); + + moduleInfo.moduleSideEffects = true; + + // The key `.` in `exportedBindings` refer to the exports within the file + const exportedFunctions = moduleInfo.exportedBindings?.['.']; + + // The enclosing `if` already checks for the suffix in `source`, but a check in `resolution.id` is needed as well to prevent multiple attachment of the suffix + return resolution.id.includes(`.mjs${SENTRY_WRAPPED_ENTRY}`) + ? resolution.id + : resolution.id + // Concatenates the query params to mark the file (also attaches names of re-exports - this is needed for serverless functions to re-export the handler) + .concat(SENTRY_WRAPPED_ENTRY) + .concat( + exportedFunctions?.length + ? SENTRY_FUNCTIONS_REEXPORT.concat(exportedFunctions.join(',')).concat(QUERY_END_INDICATOR) + : '', + ); + } + return null; + }, + load(id: string) { + if (id.includes(`.mjs${SENTRY_WRAPPED_ENTRY}`)) { + const entryId = removeSentryQueryFromPath(id); + + // Mostly useful for serverless `handler` functions + const reExportedFunctions = id.includes(SENTRY_FUNCTIONS_REEXPORT) + ? constructFunctionReExport(id, entryId) + : ''; + + return ( + // Regular `import` of the Sentry config + `import ${JSON.stringify(resolvedSentryConfigPath)};\n` + + // Dynamic `import()` for the previous, actual entry point. + // `import()` can be used for any code that should be run after the hooks are registered (https://nodejs.org/api/module.html#enabling) + `import(${JSON.stringify(entryId)});\n` + + // By importing "import-in-the-middle/hook.mjs", we can make sure this file wil be included, as not all node builders are including files imported with `module.register()`. + "import 'import-in-the-middle/hook.mjs'\n" + + `${reExportedFunctions}\n` + ); + } + + return null; + }, + }; +} diff --git a/packages/nuxt/src/vite/utils.ts b/packages/nuxt/src/vite/utils.ts index e41d3fb06cab..4d5bc080ba3a 100644 --- a/packages/nuxt/src/vite/utils.ts +++ b/packages/nuxt/src/vite/utils.ts @@ -24,3 +24,59 @@ export function findDefaultSdkInitFile(type: 'server' | 'client'): string | unde return filePaths.find(filename => fs.existsSync(filename)); } + +export const SENTRY_WRAPPED_ENTRY = '?sentry-query-wrapped-entry'; +export const SENTRY_FUNCTIONS_REEXPORT = '?sentry-query-functions-reexport='; +export const QUERY_END_INDICATOR = 'SENTRY-QUERY-END'; + +/** + * Strips the Sentry query part from a path. + * Example: example/path?sentry-query-wrapped-entry?sentry-query-functions-reexport=foo,SENTRY-QUERY-END -> /example/path + * + * Only exported for testing. + */ +export function removeSentryQueryFromPath(url: string): string { + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor + const regex = new RegExp(`\\${SENTRY_WRAPPED_ENTRY}.*?\\${QUERY_END_INDICATOR}`); + return url.replace(regex, ''); +} + +/** + * Extracts and sanitizes function re-export query parameters from a query string. + * If it is a default export, it is not considered for re-exporting. This function is mostly relevant for re-exporting + * serverless `handler` functions. + * + * Only exported for testing. + */ +export function extractFunctionReexportQueryParameters(query: string): string[] { + // Regex matches the comma-separated params between the functions query + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor + const regex = new RegExp(`\\${SENTRY_FUNCTIONS_REEXPORT}(.*?)\\${QUERY_END_INDICATOR}`); + const match = query.match(regex); + + return match && match[1] + ? match[1] + .split(',') + .filter(param => param !== '' && param !== 'default') + // Sanitize, as code could be injected with another rollup plugin + .map((str: string) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) + : []; +} + +/** + * Constructs a code snippet with function reexports (can be used in Rollup plugins) + */ +export function constructFunctionReExport(pathWithQuery: string, entryId: string): string { + const functionNames = extractFunctionReexportQueryParameters(pathWithQuery); + + return functionNames.reduce( + (functionsCode, currFunctionName) => + functionsCode.concat( + `export async function ${currFunctionName}(...args) {\n` + + ` const res = await import(${JSON.stringify(entryId)});\n` + + ` return res.${currFunctionName}.call(this, ...args);\n` + + '}\n', + ), + '', + ); +} diff --git a/packages/nuxt/test/vite/utils.test.ts b/packages/nuxt/test/vite/utils.test.ts index 5115742be0f0..0d7e91b8b83f 100644 --- a/packages/nuxt/test/vite/utils.test.ts +++ b/packages/nuxt/test/vite/utils.test.ts @@ -1,6 +1,14 @@ import * as fs from 'fs'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { findDefaultSdkInitFile } from '../../src/vite/utils'; +import { + QUERY_END_INDICATOR, + SENTRY_FUNCTIONS_REEXPORT, + SENTRY_WRAPPED_ENTRY, + constructFunctionReExport, + extractFunctionReexportQueryParameters, + findDefaultSdkInitFile, + removeSentryQueryFromPath, +} from '../../src/vite/utils'; vi.mock('fs'); @@ -59,3 +67,63 @@ describe('findDefaultSdkInitFile', () => { expect(result).toMatch('packages/nuxt/sentry.server.config.js'); }); }); + +describe('removeSentryQueryFromPath', () => { + it('strips the Sentry query part from the path', () => { + const url = `/example/path${SENTRY_WRAPPED_ENTRY}${SENTRY_FUNCTIONS_REEXPORT}foo,${QUERY_END_INDICATOR}`; + const result = removeSentryQueryFromPath(url); + expect(result).toBe('/example/path'); + }); + + it('returns the same path if the specific query part is not present', () => { + const url = '/example/path?other-query=param'; + const result = removeSentryQueryFromPath(url); + expect(result).toBe(url); + }); +}); + +describe('extractFunctionReexportQueryParameters', () => { + it.each([ + [`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,${QUERY_END_INDICATOR}`, ['foo', 'bar']], + [`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,default${QUERY_END_INDICATOR}`, ['foo', 'bar']], + [ + `${SENTRY_FUNCTIONS_REEXPORT}foo,a.b*c?d[e]f(g)h|i\\\\j(){hello},${QUERY_END_INDICATOR}`, + ['foo', 'a\\.b\\*c\\?d\\[e\\]f\\(g\\)h\\|i\\\\\\\\j\\(\\)\\{hello\\}'], + ], + [`/example/path/${SENTRY_FUNCTIONS_REEXPORT}foo,bar${QUERY_END_INDICATOR}`, ['foo', 'bar']], + [`${SENTRY_FUNCTIONS_REEXPORT}${QUERY_END_INDICATOR}`, []], + ['?other-query=param', []], + ])('extracts parameters from the query string: %s', (query, expected) => { + const result = extractFunctionReexportQueryParameters(query); + expect(result).toEqual(expected); + }); +}); + +describe('constructFunctionReExport', () => { + it('constructs re-export code for given query parameters and entry ID', () => { + const query = `${SENTRY_FUNCTIONS_REEXPORT}foo,bar,${QUERY_END_INDICATOR}}`; + const query2 = `${SENTRY_FUNCTIONS_REEXPORT}foo,bar${QUERY_END_INDICATOR}}`; + const entryId = './module'; + const result = constructFunctionReExport(query, entryId); + const result2 = constructFunctionReExport(query2, entryId); + + const expected = ` +export async function foo(...args) { + const res = await import("./module"); + return res.foo.call(this, ...args); +} +export async function bar(...args) { + const res = await import("./module"); + return res.bar.call(this, ...args); +}`; + expect(result.trim()).toBe(expected.trim()); + expect(result2.trim()).toBe(expected.trim()); + }); + + it('returns an empty string if the query string is empty', () => { + const query = ''; + const entryId = './module'; + const result = constructFunctionReExport(query, entryId); + expect(result).toBe(''); + }); +}); From 5f68d4d13138a3b064ca3ed36d9d72f77500b5f5 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 15 Oct 2024 06:47:27 +0200 Subject: [PATCH 13/34] fix(replay): Fix onError sampling when loading an expired buffered session (#13962) This fixes a bug where an older, saved session (that has a `previousSessionId`, i.e. session was recorded and expired) would cause buffered replays to not send when an error happens. This is because the session start timestamp would never update, causing our flush logic to skip sending the replay due to incorrect replay duration calculation. --- .size-limit.js | 2 +- .../src/util/handleRecordingEmit.ts | 20 ++--- .../test/integration/errorSampleRate.test.ts | 84 +++++++++++++++++++ 3 files changed, 95 insertions(+), 11 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index 8a7670e6d638..a244ccb2a2a3 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -86,7 +86,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'feedbackIntegration'), gzip: true, - limit: '91 KB', + limit: '95 KB', }, { name: '@sentry/browser (incl. Tracing, Replay, Feedback, metrics)', diff --git a/packages/replay-internal/src/util/handleRecordingEmit.ts b/packages/replay-internal/src/util/handleRecordingEmit.ts index 0467edefa9a2..4f4637276116 100644 --- a/packages/replay-internal/src/util/handleRecordingEmit.ts +++ b/packages/replay-internal/src/util/handleRecordingEmit.ts @@ -71,16 +71,6 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa // only be added for checkouts addSettingsEvent(replay, isCheckout); - // If there is a previousSessionId after a full snapshot occurs, then - // the replay session was started due to session expiration. The new session - // is started before triggering a new checkout and contains the id - // of the previous session. Do not immediately flush in this case - // to avoid capturing only the checkout and instead the replay will - // be captured if they perform any follow-up actions. - if (session && session.previousSessionId) { - return true; - } - // When in buffer mode, make sure we adjust the session started date to the current earliest event of the buffer // this should usually be the timestamp of the checkout event, but to be safe... if (replay.recordingMode === 'buffer' && session && replay.eventBuffer) { @@ -97,6 +87,16 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa } } + // If there is a previousSessionId after a full snapshot occurs, then + // the replay session was started due to session expiration. The new session + // is started before triggering a new checkout and contains the id + // of the previous session. Do not immediately flush in this case + // to avoid capturing only the checkout and instead the replay will + // be captured if they perform any follow-up actions. + if (session && session.previousSessionId) { + return true; + } + if (replay.recordingMode === 'session') { // If the full snapshot is due to an initial load, we will not have // a previous session ID. In this case, we want to buffer events diff --git a/packages/replay-internal/test/integration/errorSampleRate.test.ts b/packages/replay-internal/test/integration/errorSampleRate.test.ts index 3763ef83de44..e81d9df1b43d 100644 --- a/packages/replay-internal/test/integration/errorSampleRate.test.ts +++ b/packages/replay-internal/test/integration/errorSampleRate.test.ts @@ -148,6 +148,90 @@ describe('Integration | errorSampleRate', () => { }); }); + it('loads an old session with a previousSessionId set and can send buffered replay', async () => { + vi.mock('../../src/session/fetchSession', () => ({ + fetchSession: () => ({ + id: 'newreplayid', + started: BASE_TIMESTAMP, + lastActivity: BASE_TIMESTAMP, + segmentId: 0, + sampled: 'buffer', + previousSessionId: 'previoussessionid', + }), + })); + + const ADVANCED_TIME = 86400000; + const optionsEvent = createOptionsEvent(replay); + + expect(replay.session.started).toBe(BASE_TIMESTAMP); + + // advance time to make sure replay duration is invalid + vi.advanceTimersByTime(ADVANCED_TIME); + + // full snapshot should update session start time + mockRecord.takeFullSnapshot(true); + expect(replay.session.started).toBe(BASE_TIMESTAMP + ADVANCED_TIME); + expect(replay.recordingMode).toBe('buffer'); + + // advance so we can flush + vi.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + + captureException(new Error('testing')); + await vi.advanceTimersToNextTimerAsync(); + + // Converts to session mode + expect(replay.recordingMode).toBe('session'); + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + ADVANCED_TIME, type: 2 }, + { ...optionsEvent, timestamp: BASE_TIMESTAMP + ADVANCED_TIME }, + ]), + }); + + // capture next event + domHandler({ + name: 'click', + event: new Event('click'), + }); + + vi.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + await vi.advanceTimersToNextTimerAsync(); + + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 1 }, + replayEventPayload: expect.objectContaining({ + // We don't change replay_type as it starts in buffer mode and that's + // what we're interested in, even though recordingMode changes to + // 'session' + replay_type: 'buffer', + }), + recordingData: JSON.stringify([ + // There's a new checkout because we convert to session mode + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + ADVANCED_TIME + DEFAULT_FLUSH_MIN_DELAY, type: 2 }, + { + type: 5, + timestamp: BASE_TIMESTAMP + ADVANCED_TIME + DEFAULT_FLUSH_MIN_DELAY, + data: { + tag: 'breadcrumb', + payload: { + timestamp: (BASE_TIMESTAMP + ADVANCED_TIME + DEFAULT_FLUSH_MIN_DELAY) / 1000, + type: 'default', + category: 'ui.click', + message: '', + data: {}, + }, + }, + }, + ]), + }); + vi.unmock('../../src/session/fetchSession'); + await waitForFlush(); + }); + it('manually flushes replay and does not continue to record', async () => { const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); mockRecord._emitter(TEST_EVENT); From b8d0f2f68d879b418f2aa246ee580adb8361161e Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 15 Oct 2024 11:40:37 +0300 Subject: [PATCH 14/34] chore(node): Bump `@opentelemetry/instrumentation-express` to `0.43.0` (#13948) --- .../multiple-routers/complex-router/test.ts | 74 +++++-------------- .../middle-layer-parameterized/test.ts | 26 ++----- packages/node/package.json | 2 +- yarn.lock | 8 +- 4 files changed, 31 insertions(+), 79 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/multiple-routers/complex-router/test.ts b/dev-packages/node-integration-tests/suites/express/multiple-routers/complex-router/test.ts index 606120248e35..fe065d0dc550 100644 --- a/dev-packages/node-integration-tests/suites/express/multiple-routers/complex-router/test.ts +++ b/dev-packages/node-integration-tests/suites/express/multiple-routers/complex-router/test.ts @@ -1,31 +1,17 @@ -import { conditionalTest } from '../../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; afterAll(() => { cleanupChildProcesses(); }); -// Before Node 16, parametrization is not working properly here -conditionalTest({ min: 16 })('complex-router', () => { +describe('complex-router', () => { test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route and express used multiple middlewares with route', done => { - // parse node.js major version - const [major = 0] = process.versions.node.split('.').map(Number); - // Split test result base on major node version because regex d flag is support from node 16+ - - const EXPECTED_TRANSACTION = - major >= 16 - ? { - transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', - transaction_info: { - source: 'route', - }, - } - : { - transaction: 'GET /api/api/v1/sub-router/users/123/posts/:postId', - transaction_info: { - source: 'route', - }, - }; + const EXPECTED_TRANSACTION = { + transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', + transaction_info: { + source: 'route', + }, + }; createRunner(__dirname, 'server.ts') .ignore('event') @@ -35,23 +21,12 @@ conditionalTest({ min: 16 })('complex-router', () => { }); test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route and express used multiple middlewares with route and original url has query params', done => { - // parse node.js major version - const [major = 0] = process.versions.node.split('.').map(Number); - // Split test result base on major node version because regex d flag is support from node 16+ - const EXPECTED_TRANSACTION = - major >= 16 - ? { - transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', - transaction_info: { - source: 'route', - }, - } - : { - transaction: 'GET /api/api/v1/sub-router/users/123/posts/:postId', - transaction_info: { - source: 'route', - }, - }; + const EXPECTED_TRANSACTION = { + transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', + transaction_info: { + source: 'route', + }, + }; createRunner(__dirname, 'server.ts') .ignore('event') @@ -61,23 +36,12 @@ conditionalTest({ min: 16 })('complex-router', () => { }); test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route and express used multiple middlewares with route and original url ends with trailing slash and has query params', done => { - // parse node.js major version - const [major = 0] = process.versions.node.split('.').map(Number); - // Split test result base on major node version because regex d flag is support from node 16+ - const EXPECTED_TRANSACTION = - major >= 16 - ? { - transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', - transaction_info: { - source: 'route', - }, - } - : { - transaction: 'GET /api/api/v1/sub-router/users/123/posts/:postId', - transaction_info: { - source: 'route', - }, - }; + const EXPECTED_TRANSACTION = { + transaction: 'GET /api/api/v1/sub-router/users/:userId/posts/:postId', + transaction_info: { + source: 'route', + }, + }; createRunner(__dirname, 'server.ts') .ignore('event') diff --git a/dev-packages/node-integration-tests/suites/express/multiple-routers/middle-layer-parameterized/test.ts b/dev-packages/node-integration-tests/suites/express/multiple-routers/middle-layer-parameterized/test.ts index 4a6ae304af14..52a6ce154684 100644 --- a/dev-packages/node-integration-tests/suites/express/multiple-routers/middle-layer-parameterized/test.ts +++ b/dev-packages/node-integration-tests/suites/express/multiple-routers/middle-layer-parameterized/test.ts @@ -1,4 +1,3 @@ -import { conditionalTest } from '../../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; afterAll(() => { @@ -6,25 +5,14 @@ afterAll(() => { }); // Before Node 16, parametrization is not working properly here -conditionalTest({ min: 16 })('middle-layer-parameterized', () => { +describe('middle-layer-parameterized', () => { test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route', done => { - // parse node.js major version - const [major = 0] = process.versions.node.split('.').map(Number); - // Split test result base on major node version because regex d flag is support from node 16+ - const EXPECTED_TRANSACTION = - major >= 16 - ? { - transaction: 'GET /api/v1/users/:userId/posts/:postId', - transaction_info: { - source: 'route', - }, - } - : { - transaction: 'GET /api/v1/users/123/posts/:postId', - transaction_info: { - source: 'route', - }, - }; + const EXPECTED_TRANSACTION = { + transaction: 'GET /api/v1/users/:userId/posts/:postId', + transaction_info: { + source: 'route', + }, + }; createRunner(__dirname, 'server.ts') .ignore('event') diff --git a/packages/node/package.json b/packages/node/package.json index 8145cf18a7d6..52559d93ed27 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -72,7 +72,7 @@ "@opentelemetry/instrumentation-amqplib": "^0.42.0", "@opentelemetry/instrumentation-connect": "0.39.0", "@opentelemetry/instrumentation-dataloader": "0.12.0", - "@opentelemetry/instrumentation-express": "0.42.0", + "@opentelemetry/instrumentation-express": "0.43.0", "@opentelemetry/instrumentation-fastify": "0.39.0", "@opentelemetry/instrumentation-fs": "0.15.0", "@opentelemetry/instrumentation-generic-pool": "0.39.0", diff --git a/yarn.lock b/yarn.lock index 5ab8ce9b9952..2a31856c98dd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7326,10 +7326,10 @@ dependencies: "@opentelemetry/instrumentation" "^0.53.0" -"@opentelemetry/instrumentation-express@0.42.0": - version "0.42.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-express/-/instrumentation-express-0.42.0.tgz#279f195aa66baee2b98623a16666c6229c8e7564" - integrity sha512-YNcy7ZfGnLsVEqGXQPT+S0G1AE46N21ORY7i7yUQyfhGAL4RBjnZUqefMI0NwqIl6nGbr1IpF0rZGoN8Q7x12Q== +"@opentelemetry/instrumentation-express@0.43.0": + version "0.43.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-express/-/instrumentation-express-0.43.0.tgz#35ff5bcf40b816d9a9159d5f7814ed7e5d83f69b" + integrity sha512-bxTIlzn9qPXJgrhz8/Do5Q3jIlqfpoJrSUtVGqH+90eM1v2PkPHc+SdE+zSqe4q9Y1UQJosmZ4N4bm7Zj/++MA== dependencies: "@opentelemetry/core" "^1.8.0" "@opentelemetry/instrumentation" "^0.53.0" From ecf84e0c8ff95f5800efcdcf80ee552964b69175 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 15 Oct 2024 11:44:41 +0300 Subject: [PATCH 15/34] feat(vue): Add Pinia plugin (#13841) Resolves: https://github.com/getsentry/sentry-javascript/issues/13279 Depends on: #13840 [Sample Event](https://sentry-sdks.sentry.io/issues/5939879614/?project=5429219&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D&referrer=issue-stream&sort=date&statsPeriod=1h&stream_index=0) Docs PR: https://github.com/getsentry/sentry-docs/pull/11516 Adds a Pinia plugin with a feature set similar to the Redux integration. - Attaches Pinia state as an attachment to the event (`true` by default) - Provides `actionTransformer` and `stateTransformer` to the user for potentially required PII modifications. - Adds breadcrumbs for Pinia actions - Assigns Pinia state to event contexts. --- .../test-applications/vue-3/package.json | 1 + .../test-applications/vue-3/src/main.ts | 14 +++ .../vue-3/src/router/index.ts | 4 + .../vue-3/src/stores/cart.ts | 43 ++++++++ .../vue-3/src/views/CartView.vue | 88 +++++++++++++++ .../vue-3/tests/pinia.test.ts | 35 ++++++ packages/vue/package.json | 8 +- packages/vue/src/index.ts | 1 + packages/vue/src/pinia.ts | 103 ++++++++++++++++++ yarn.lock | 62 ++++++++++- 10 files changed, 352 insertions(+), 7 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/vue-3/src/stores/cart.ts create mode 100644 dev-packages/e2e-tests/test-applications/vue-3/src/views/CartView.vue create mode 100644 dev-packages/e2e-tests/test-applications/vue-3/tests/pinia.test.ts create mode 100644 packages/vue/src/pinia.ts diff --git a/dev-packages/e2e-tests/test-applications/vue-3/package.json b/dev-packages/e2e-tests/test-applications/vue-3/package.json index 6b837910fc02..aab0c6f3b744 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/package.json +++ b/dev-packages/e2e-tests/test-applications/vue-3/package.json @@ -16,6 +16,7 @@ }, "dependencies": { "@sentry/vue": "latest || *", + "pinia": "^2.2.3", "vue": "^3.4.15", "vue-router": "^4.2.5" }, diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts index 13064ce04080..b940023b3153 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts @@ -4,10 +4,13 @@ import { createApp } from 'vue'; import App from './App.vue'; import router from './router'; +import { createPinia } from 'pinia'; + import * as Sentry from '@sentry/vue'; import { browserTracingIntegration } from '@sentry/vue'; const app = createApp(App); +const pinia = createPinia(); Sentry.init({ app, @@ -22,5 +25,16 @@ Sentry.init({ trackComponents: ['ComponentMainView', ''], }); +pinia.use( + Sentry.createSentryPiniaPlugin({ + actionTransformer: action => `Transformed: ${action}`, + stateTransformer: state => ({ + transformed: true, + ...state, + }), + }), +); + +app.use(pinia); app.use(router); app.mount('#app'); diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/router/index.ts b/dev-packages/e2e-tests/test-applications/vue-3/src/router/index.ts index 3a05e4f1055a..c81a662c61e2 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/src/router/index.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/router/index.ts @@ -34,6 +34,10 @@ const router = createRouter({ path: '/components', component: () => import('../views/ComponentMainView.vue'), }, + { + path: '/cart', + component: () => import('../views/CartView.vue'), + }, ], }); diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/stores/cart.ts b/dev-packages/e2e-tests/test-applications/vue-3/src/stores/cart.ts new file mode 100644 index 000000000000..7786c7f27cd9 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/stores/cart.ts @@ -0,0 +1,43 @@ +import { acceptHMRUpdate, defineStore } from 'pinia'; + +export const useCartStore = defineStore({ + id: 'cart', + state: () => ({ + rawItems: [] as string[], + }), + getters: { + items: (state): Array<{ name: string; amount: number }> => + state.rawItems.reduce( + (items, item) => { + const existingItem = items.find(it => it.name === item); + + if (!existingItem) { + items.push({ name: item, amount: 1 }); + } else { + existingItem.amount++; + } + + return items; + }, + [] as Array<{ name: string; amount: number }>, + ), + }, + actions: { + addItem(name: string) { + this.rawItems.push(name); + }, + + removeItem(name: string) { + const i = this.rawItems.lastIndexOf(name); + if (i > -1) this.rawItems.splice(i, 1); + }, + + throwError() { + throw new Error('error'); + }, + }, +}); + +if (import.meta.hot) { + import.meta.hot.accept(acceptHMRUpdate(useCartStore, import.meta.hot)); +} diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/views/CartView.vue b/dev-packages/e2e-tests/test-applications/vue-3/src/views/CartView.vue new file mode 100644 index 000000000000..ba7037e68bfe --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/views/CartView.vue @@ -0,0 +1,88 @@ + + + + + diff --git a/dev-packages/e2e-tests/test-applications/vue-3/tests/pinia.test.ts b/dev-packages/e2e-tests/test-applications/vue-3/tests/pinia.test.ts new file mode 100644 index 000000000000..5699ebc24b7c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/vue-3/tests/pinia.test.ts @@ -0,0 +1,35 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test('sends pinia action breadcrumbs and state context', async ({ page }) => { + await page.goto('/cart'); + + await page.locator('#item-input').fill('item'); + await page.locator('#item-add').click(); + + const errorPromise = waitForError('vue-3', async errorEvent => { + return errorEvent?.exception?.values?.[0].value === 'This is an error'; + }); + + await page.locator('#throw-error').click(); + + const error = await errorPromise; + + expect(error).toBeTruthy(); + expect(error.breadcrumbs?.length).toBeGreaterThan(0); + + const actionBreadcrumb = error.breadcrumbs?.find(breadcrumb => breadcrumb.category === 'action'); + + expect(actionBreadcrumb).toBeDefined(); + expect(actionBreadcrumb?.message).toBe('Transformed: addItem'); + expect(actionBreadcrumb?.level).toBe('info'); + + const stateContext = error.contexts?.state?.state; + + expect(stateContext).toBeDefined(); + expect(stateContext?.type).toBe('pinia'); + expect(stateContext?.value).toEqual({ + transformed: true, + rawItems: ['item'], + }); +}); diff --git a/packages/vue/package.json b/packages/vue/package.json index 1090d0d4292e..7d1978551baf 100644 --- a/packages/vue/package.json +++ b/packages/vue/package.json @@ -45,7 +45,13 @@ "@sentry/utils": "8.34.0" }, "peerDependencies": { - "vue": "2.x || 3.x" + "vue": "2.x || 3.x", + "pinia": "2.x" + }, + "peerDependenciesMeta": { + "pinia": { + "optional": true + } }, "devDependencies": { "vue": "~3.2.41" diff --git a/packages/vue/src/index.ts b/packages/vue/src/index.ts index 110b80d270a9..096b7a2144e5 100644 --- a/packages/vue/src/index.ts +++ b/packages/vue/src/index.ts @@ -5,3 +5,4 @@ export { browserTracingIntegration } from './browserTracingIntegration'; export { attachErrorHandler } from './errorhandler'; export { createTracingMixins } from './tracing'; export { vueIntegration } from './integration'; +export { createSentryPiniaPlugin } from './pinia'; diff --git a/packages/vue/src/pinia.ts b/packages/vue/src/pinia.ts new file mode 100644 index 000000000000..a21273a7d54b --- /dev/null +++ b/packages/vue/src/pinia.ts @@ -0,0 +1,103 @@ +import { addBreadcrumb, getClient, getCurrentScope, getGlobalScope } from '@sentry/core'; +import { addNonEnumerableProperty } from '@sentry/utils'; + +// Inline PiniaPlugin type +type PiniaPlugin = (context: { + store: { + $id: string; + $state: unknown; + $onAction: (callback: (context: { name: string; after: (callback: () => void) => void }) => void) => void; + }; +}) => void; + +type SentryPiniaPluginOptions = { + attachPiniaState?: boolean; + addBreadcrumbs?: boolean; + actionTransformer?: (action: any) => any; + stateTransformer?: (state: any) => any; +}; + +export const createSentryPiniaPlugin: (options?: SentryPiniaPluginOptions) => PiniaPlugin = ( + options: SentryPiniaPluginOptions = { + attachPiniaState: true, + addBreadcrumbs: true, + actionTransformer: action => action, + stateTransformer: state => state, + }, +) => { + const plugin: PiniaPlugin = ({ store }) => { + options.attachPiniaState !== false && + getGlobalScope().addEventProcessor((event, hint) => { + try { + // Get current timestamp in hh:mm:ss + const timestamp = new Date().toTimeString().split(' ')[0]; + const filename = `pinia_state_${store.$id}_${timestamp}.json`; + + hint.attachments = [ + ...(hint.attachments || []), + { + filename, + data: JSON.stringify(store.$state), + }, + ]; + } catch (_) { + // empty + } + + return event; + }); + + store.$onAction(context => { + context.after(() => { + const transformedActionName = options.actionTransformer + ? options.actionTransformer(context.name) + : context.name; + + if ( + typeof transformedActionName !== 'undefined' && + transformedActionName !== null && + options.addBreadcrumbs !== false + ) { + addBreadcrumb({ + category: 'action', + message: transformedActionName, + level: 'info', + }); + } + + /* Set latest state to scope */ + const transformedState = options.stateTransformer ? options.stateTransformer(store.$state) : store.$state; + const scope = getCurrentScope(); + const currentState = scope.getScopeData().contexts.state; + + if (typeof transformedState !== 'undefined' && transformedState !== null) { + const client = getClient(); + const options = client && client.getOptions(); + const normalizationDepth = (options && options.normalizeDepth) || 3; // default state normalization depth to 3 + const piniaStateContext = { type: 'pinia', value: transformedState }; + + const newState = { + ...(currentState || {}), + state: piniaStateContext, + }; + + addNonEnumerableProperty( + newState, + '__sentry_override_normalization_depth__', + 3 + // 3 layers for `state.value.transformedState + normalizationDepth, // rest for the actual state + ); + + scope.setContext('state', newState); + } else { + scope.setContext('state', { + ...(currentState || {}), + state: { type: 'pinia', value: 'undefined' }, + }); + } + }); + }); + }; + + return plugin; +}; diff --git a/yarn.lock b/yarn.lock index 2a31856c98dd..db64fa00fc6c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9826,7 +9826,17 @@ dependencies: "@types/unist" "*" -"@types/history-4@npm:@types/history@4.7.8", "@types/history-5@npm:@types/history@4.7.8", "@types/history@*": +"@types/history-4@npm:@types/history@4.7.8": + version "4.7.8" + resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" + integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== + +"@types/history-5@npm:@types/history@4.7.8": + version "4.7.8" + resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" + integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== + +"@types/history@*": version "4.7.8" resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== @@ -10154,7 +10164,15 @@ "@types/history" "^3" "@types/react" "*" -"@types/react-router-4@npm:@types/react-router@5.1.14", "@types/react-router-5@npm:@types/react-router@5.1.14": +"@types/react-router-4@npm:@types/react-router@5.1.14": + version "5.1.14" + resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.14.tgz#e0442f4eb4c446541ad7435d44a97f8fe6df40da" + integrity sha512-LAJpqYUaCTMT2anZheoidiIymt8MuX286zoVFPM3DVb23aQBH0mAkFvzpd4LKqiolV8bBtZWT5Qp7hClCNDENw== + dependencies: + "@types/history" "*" + "@types/react" "*" + +"@types/react-router-5@npm:@types/react-router@5.1.14": version "5.1.14" resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.14.tgz#e0442f4eb4c446541ad7435d44a97f8fe6df40da" integrity sha512-LAJpqYUaCTMT2anZheoidiIymt8MuX286zoVFPM3DVb23aQBH0mAkFvzpd4LKqiolV8bBtZWT5Qp7hClCNDENw== @@ -28626,7 +28644,7 @@ react-is@^18.0.0: dependencies: "@remix-run/router" "1.0.2" -"react-router-6@npm:react-router@6.3.0", react-router@6.3.0: +"react-router-6@npm:react-router@6.3.0": version "6.3.0" resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== @@ -28641,6 +28659,13 @@ react-router-dom@^6.2.2: history "^5.2.0" react-router "6.3.0" +react-router@6.3.0: + version "6.3.0" + resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" + integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== + dependencies: + history "^5.2.0" + react@^18.0.0: version "18.0.0" resolved "https://registry.yarnpkg.com/react/-/react-18.0.0.tgz#b468736d1f4a5891f38585ba8e8fb29f91c3cb96" @@ -31089,7 +31114,16 @@ string-template@~0.2.1: resolved "https://registry.yarnpkg.com/string-template/-/string-template-0.2.1.tgz#42932e598a352d01fc22ec3367d9d84eec6c9add" integrity sha1-QpMuWYo1LQH8IuwzZ9nYTuxsmt0= -"string-width-cjs@npm:string-width@^4.2.0", string-width@4.2.3, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0": + version "4.2.3" + resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" + integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== + dependencies: + emoji-regex "^8.0.0" + is-fullwidth-code-point "^3.0.0" + strip-ansi "^6.0.1" + +string-width@4.2.3, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -31201,7 +31235,14 @@ stringify-object@^3.2.1: is-obj "^1.0.1" is-regexp "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1": + version "6.0.1" + resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" + integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== + dependencies: + ansi-regex "^5.0.1" + +strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -34218,7 +34259,16 @@ wrangler@^3.67.1: optionalDependencies: fsevents "~2.3.2" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@7.0.0, wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": + version "7.0.0" + resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" + integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== + dependencies: + ansi-styles "^4.0.0" + string-width "^4.1.0" + strip-ansi "^6.0.0" + +wrap-ansi@7.0.0, wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== From 4c0c25cbfbbbb9c71fcf1a7a5a62cf82c086457e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 15 Oct 2024 11:06:34 +0200 Subject: [PATCH 16/34] test(node): Add tests for current DSC transaction name updating behavior (#13961) Add tests for DSC updating behaviour in the Node SDK, analogously to #13953 From b86c182fb845c82daab2858d86318a398868e8df Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 15 Oct 2024 16:15:10 +0200 Subject: [PATCH 17/34] feat(replay): Do not log "timeout while trying to read resp body" as exception (#13965) Change this to be a warn level log instead of an exception (which gets captured to Sentry). Since this is an error we are throwing ourselves and it is to be expected, no need to treat as an exception. This also adds a new meta string to differentiate from an error while parsing. This means we can show a more specific error message on the frontend. --- .../replay-internal/src/coreHandlers/util/fetchUtils.ts | 7 +++++-- packages/replay-internal/src/types/request.ts | 1 + .../test/unit/coreHandlers/util/fetchUtils.test.ts | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/replay-internal/src/coreHandlers/util/fetchUtils.ts b/packages/replay-internal/src/coreHandlers/util/fetchUtils.ts index 6502206b58b6..f218f4ab9b35 100644 --- a/packages/replay-internal/src/coreHandlers/util/fetchUtils.ts +++ b/packages/replay-internal/src/coreHandlers/util/fetchUtils.ts @@ -209,6 +209,11 @@ async function _parseFetchResponseBody(response: Response): Promise<[string | un const text = await _tryGetResponseText(res); return [text]; } catch (error) { + if (error instanceof Error && error.message.indexOf('Timeout') > -1) { + DEBUG_BUILD && logger.warn('Parsing text body from response timed out'); + return [undefined, 'BODY_PARSE_TIMEOUT']; + } + DEBUG_BUILD && logger.exception(error, 'Failed to get text body from response'); return [undefined, 'BODY_PARSE_ERROR']; } @@ -299,8 +304,6 @@ function _tryGetResponseText(response: Response): Promise { ) .finally(() => clearTimeout(timeout)); }); - - return _getResponseText(response); } async function _getResponseText(response: Response): Promise { diff --git a/packages/replay-internal/src/types/request.ts b/packages/replay-internal/src/types/request.ts index 60c25a55ce44..c04b57409d0c 100644 --- a/packages/replay-internal/src/types/request.ts +++ b/packages/replay-internal/src/types/request.ts @@ -8,6 +8,7 @@ export type NetworkMetaWarning = | 'TEXT_TRUNCATED' | 'URL_SKIPPED' | 'BODY_PARSE_ERROR' + | 'BODY_PARSE_TIMEOUT' | 'UNPARSEABLE_BODY_TYPE'; interface NetworkMeta { diff --git a/packages/replay-internal/test/unit/coreHandlers/util/fetchUtils.test.ts b/packages/replay-internal/test/unit/coreHandlers/util/fetchUtils.test.ts index ffd665471975..4da9ecab639e 100644 --- a/packages/replay-internal/test/unit/coreHandlers/util/fetchUtils.test.ts +++ b/packages/replay-internal/test/unit/coreHandlers/util/fetchUtils.test.ts @@ -132,7 +132,7 @@ describe('Unit | coreHandlers | util | fetchUtils', () => { ]); expect(res).toEqual({ - _meta: { warnings: ['BODY_PARSE_ERROR'] }, + _meta: { warnings: ['BODY_PARSE_TIMEOUT'] }, headers: {}, size: undefined, }); From 41515a6a2b2b9380507e269d540cd70952d6aba2 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 16 Oct 2024 10:04:06 +0200 Subject: [PATCH 18/34] fix(node): Ensure `ignoreOutgoingRequests` of `httpIntegration` applies to breadcrumbs (#13970) Fix a bug which caused the `httpIntegration`'s `ignoreOutgoingRequests` option to not be applied to breadcrumb creation for outgoing requests. As a result, despite spans being correctly filtered by the option, breadcrumbs would still be created which contradicts the function'S JSDoc and [our docs](https://docs.sentry.io/platforms/javascript/guides/node/configuration/integrations/http/#ignoreoutgoingrequests). This patch fixes the bug by: - correctly passing in `ignoreOutgoingRequests` to `SentryHttpIntegration` (same signature as `httpIntegration`) - reconstructing the `url` and `request` parameter like in the [OpenTelemetry instrumentation](https://github.com/open-telemetry/opentelemetry-js/blob/7293e69c1e55ca62e15d0724d22605e61bd58952/experimental/packages/opentelemetry-instrumentation-http/src/http.ts#L756-L789) - adding/adjusting a regression test so that we properly test agains `ignoreOutgoingRequests` with both params. Now not just for filtered spans but also for breadcrumbs. --- .../server-ignoreOutgoingRequests.js | 50 ++++++++------ .../suites/tracing/httpIntegration/test.ts | 66 +++++++------------ .../http/SentryHttpInstrumentation.ts | 49 ++++++++++++-- packages/node/src/integrations/http/index.ts | 21 +++--- 4 files changed, 110 insertions(+), 76 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js index 9d7d2ed069d1..b42fa97fab08 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js @@ -11,10 +11,11 @@ Sentry.init({ integrations: [ Sentry.httpIntegration({ ignoreOutgoingRequests: (url, request) => { - if (url.includes('example.com')) { + if (url === 'http://example.com/blockUrl') { return true; } - if (request.method === 'POST' && request.path === '/path') { + + if (request.hostname === 'example.com' && request.path === '/blockRequest') { return true; } return false; @@ -32,28 +33,37 @@ const app = express(); app.use(cors()); -app.get('/test', (_req, response) => { - http - .request('http://example.com/', res => { - res.on('data', () => {}); - res.on('end', () => { - response.send({ response: 'done' }); - }); - }) - .end(); +app.get('/testUrl', (_req, response) => { + makeHttpRequest('http://example.com/blockUrl').then(() => { + makeHttpRequest('http://example.com/pass').then(() => { + response.send({ response: 'done' }); + }); + }); }); -app.post('/testPath', (_req, response) => { - http - .request('http://example.com/path', res => { - res.on('data', () => {}); - res.on('end', () => { - response.send({ response: 'done' }); - }); - }) - .end(); +app.get('/testRequest', (_req, response) => { + makeHttpRequest('http://example.com/blockRequest').then(() => { + makeHttpRequest('http://example.com/pass').then(() => { + response.send({ response: 'done' }); + }); + }); }); Sentry.setupExpressErrorHandler(app); startExpressServerAndSendPortToRunner(app); + +function makeHttpRequest(url) { + return new Promise((resolve, reject) => { + http + .get(url, res => { + res.on('data', () => {}); + res.on('end', () => { + resolve(); + }); + }) + .on('error', error => { + reject(error); + }); + }); +} diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts index 972ba30eab43..016ad078d34e 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts @@ -128,65 +128,45 @@ describe('httpIntegration', () => { }); }); - describe("doesn't create child spans for outgoing requests ignored via `ignoreOutgoingRequests`", () => { + describe("doesn't create child spans or breadcrumbs for outgoing requests ignored via `ignoreOutgoingRequests`", () => { test('via the url param', done => { const runner = createRunner(__dirname, 'server-ignoreOutgoingRequests.js') .expect({ - transaction: { - contexts: { - trace: { - span_id: expect.any(String), - trace_id: expect.any(String), - data: { - url: expect.stringMatching(/\/test$/), - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - }, - }, - transaction: 'GET /test', - spans: [ - expect.objectContaining({ op: 'middleware.express', description: 'query' }), - expect.objectContaining({ op: 'middleware.express', description: 'expressInit' }), - expect.objectContaining({ op: 'middleware.express', description: 'corsMiddleware' }), - expect.objectContaining({ op: 'request_handler.express', description: '/test' }), - ], + transaction: event => { + expect(event.transaction).toBe('GET /testUrl'); + + const requestSpans = event.spans?.filter(span => span.op === 'http.client'); + expect(requestSpans).toHaveLength(1); + expect(requestSpans![0]?.description).toBe('GET http://example.com/pass'); + + const breadcrumbs = event.breadcrumbs?.filter(b => b.category === 'http'); + expect(breadcrumbs).toHaveLength(1); + expect(breadcrumbs![0]?.data?.url).toEqual('http://example.com/pass'); }, }) .start(done); - runner.makeRequest('get', '/test'); + runner.makeRequest('get', '/testUrl'); }); test('via the request param', done => { const runner = createRunner(__dirname, 'server-ignoreOutgoingRequests.js') .expect({ - transaction: { - contexts: { - trace: { - span_id: expect.any(String), - trace_id: expect.any(String), - data: { - url: expect.stringMatching(/\/testPath$/), - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - }, - }, - transaction: 'POST /testPath', - spans: [ - expect.objectContaining({ op: 'middleware.express', description: 'query' }), - expect.objectContaining({ op: 'middleware.express', description: 'expressInit' }), - expect.objectContaining({ op: 'middleware.express', description: 'corsMiddleware' }), - expect.objectContaining({ op: 'request_handler.express', description: '/testPath' }), - ], + transaction: event => { + expect(event.transaction).toBe('GET /testRequest'); + + const requestSpans = event.spans?.filter(span => span.op === 'http.client'); + expect(requestSpans).toHaveLength(1); + expect(requestSpans![0]?.description).toBe('GET http://example.com/pass'); + + const breadcrumbs = event.breadcrumbs?.filter(b => b.category === 'http'); + expect(breadcrumbs).toHaveLength(1); + expect(breadcrumbs![0]?.data?.url).toEqual('http://example.com/pass'); }, }) .start(done); - runner.makeRequest('post', '/testPath'); + runner.makeRequest('get', '/testRequest'); }); }); }); diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index 62922b1b3921..090c0783507a 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -1,8 +1,10 @@ import type * as http from 'node:http'; +import type { RequestOptions } from 'node:http'; import type * as https from 'node:https'; import { VERSION } from '@opentelemetry/core'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; +import { getRequestInfo } from '@opentelemetry/instrumentation-http'; import { addBreadcrumb, getClient, getIsolationScope, withIsolationScope } from '@sentry/core'; import type { SanitizedRequestData } from '@sentry/types'; import { @@ -12,12 +14,29 @@ import { stripUrlQueryAndFragment, } from '@sentry/utils'; import type { NodeClient } from '../../sdk/client'; +import { getRequestUrl } from '../../utils/getRequestUrl'; type Http = typeof http; type Https = typeof https; type SentryHttpInstrumentationOptions = InstrumentationConfig & { + /** + * Whether breadcrumbs should be recorded for requests. + * + * @default `true` + */ breadcrumbs?: boolean; + + /** + * Do not capture breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`. + * For the scope of this instrumentation, this callback only controls breadcrumb creation. + * The same option can be passed to the top-level httpIntegration where it controls both, breadcrumb and + * span creation. + * + * @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request. + * @param request Contains the {@type RequestOptions} object used to make the outgoing request. + */ + ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; }; /** @@ -140,7 +159,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase http.ClientRequest, - ) => (...args: unknown[]) => http.ClientRequest { + ) => (options: URL | http.RequestOptions | string, ...args: unknown[]) => http.ClientRequest { // eslint-disable-next-line @typescript-eslint/no-this-alias const instrumentation = this; @@ -148,12 +167,34 @@ export class SentryHttpInstrumentation extends InstrumentationBase; request.prependListener('response', (response: http.IncomingMessage) => { - const breadcrumbs = instrumentation.getConfig().breadcrumbs; - const _breadcrumbs = typeof breadcrumbs === 'undefined' ? true : breadcrumbs; - if (_breadcrumbs) { + const _breadcrumbs = instrumentation.getConfig().breadcrumbs; + const breadCrumbsEnabled = typeof _breadcrumbs === 'undefined' ? true : _breadcrumbs; + + const _ignoreOutgoingRequests = instrumentation.getConfig().ignoreOutgoingRequests; + const shouldCreateBreadcrumb = + typeof _ignoreOutgoingRequests === 'function' + ? !_ignoreOutgoingRequests(getRequestUrl(request), optionsParsed) + : true; + + if (breadCrumbsEnabled && shouldCreateBreadcrumb) { addRequestBreadcrumb(request, response); } }); diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index 9c8d13b58127..975503956f21 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -20,7 +20,7 @@ const INSTRUMENTATION_NAME = '@opentelemetry_sentry-patched/instrumentation-http interface HttpOptions { /** - * Whether breadcrumbs should be recorded for requests. + * Whether breadcrumbs should be recorded for outgoing requests. * Defaults to true */ breadcrumbs?: boolean; @@ -45,8 +45,8 @@ interface HttpOptions { ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; /** - * Do not capture spans or breadcrumbs for incoming HTTP requests to URLs where the given callback returns `true`. - * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. + * Do not capture spans for incoming HTTP requests to URLs where the given callback returns `true`. + * Spans will be non recording if tracing is disabled. * * The `urlPath` param consists of the URL path and query string (if any) of the incoming request. * For example: `'/users/details?id=123'` @@ -82,12 +82,15 @@ interface HttpOptions { }; } -export const instrumentSentryHttp = generateInstrumentOnce<{ breadcrumbs?: boolean }>( - `${INTEGRATION_NAME}.sentry`, - options => { - return new SentryHttpInstrumentation({ breadcrumbs: options?.breadcrumbs }); - }, -); +export const instrumentSentryHttp = generateInstrumentOnce<{ + breadcrumbs?: HttpOptions['breadcrumbs']; + ignoreOutgoingRequests?: HttpOptions['ignoreOutgoingRequests']; +}>(`${INTEGRATION_NAME}.sentry`, options => { + return new SentryHttpInstrumentation({ + breadcrumbs: options?.breadcrumbs, + ignoreOutgoingRequests: options?.ignoreOutgoingRequests, + }); +}); export const instrumentOtelHttp = generateInstrumentOnce(INTEGRATION_NAME, config => { const instrumentation = new HttpInstrumentation(config); From 7fbe7447ce55dfe0b92e98675f9b89b467c8b30f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 16 Oct 2024 11:44:12 +0200 Subject: [PATCH 19/34] test(nextjs): Use RC1 as latest for Next.js canary tests (#13995) --- .../e2e-tests/test-applications/nextjs-15/package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/package.json b/dev-packages/e2e-tests/test-applications/nextjs-15/package.json index 54d672523253..04033e0362b2 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-15/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/package.json @@ -9,7 +9,8 @@ "test:dev": "TEST_ENV=development playwright test", "test:build": "pnpm install && npx playwright install && pnpm build", "test:build-canary": "pnpm install && pnpm add next@canary && pnpm add react@beta && pnpm add react-dom@beta && npx playwright install && pnpm build", - "test:build-latest": "pnpm install && pnpm add next@rc && pnpm add react@beta && pnpm add react-dom@beta && npx playwright install && pnpm build", + "//": "15.0.0-canary.194 is the canary release attached to Next.js RC 1. We need to use the canary version instead of the RC because PPR will not work without. The specific react version is also attached to RC 1.", + "test:build-latest": "pnpm install && pnpm add next@15.0.0-canary.194 && pnpm add react@19.0.0-rc-cd22717c-20241013 && pnpm add react-dom@19.0.0-rc-cd22717c-20241013 && npx playwright install && pnpm build", "test:assert": "pnpm test:prod && pnpm test:dev" }, "dependencies": { From 8d5a08459c896541bbc3684039d38d103a3ab0ea Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 16 Oct 2024 11:57:38 +0200 Subject: [PATCH 20/34] fix(core): `.set` the `sentry-trace` header instead of `.append`ing in fetch instrumentation (#13907) --- .size-limit.js | 4 +- .../trace-header-merging/init.js | 10 +++ .../trace-header-merging/subject.js | 53 +++++++++++++++ .../trace-header-merging/template.html | 11 ++++ .../trace-header-merging/test.ts | 64 +++++++++++++++++++ packages/core/src/fetch.ts | 63 +++++++++++++++--- 6 files changed, 193 insertions(+), 12 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts diff --git a/.size-limit.js b/.size-limit.js index a244ccb2a2a3..bdfe8a4397e2 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -79,7 +79,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'), gzip: true, - limit: '78 KB', + limit: '78.1 KB', }, { name: '@sentry/browser (incl. Tracing, Replay, Feedback)', @@ -138,7 +138,7 @@ module.exports = [ import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'), ignore: ['react/jsx-runtime'], gzip: true, - limit: '39 KB', + limit: '39.05 KB', }, // Vue SDK (ESM) { diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js new file mode 100644 index 000000000000..7cd076a052e5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracePropagationTargets: ['http://example.com'], + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js new file mode 100644 index 000000000000..5951d6d33411 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js @@ -0,0 +1,53 @@ +fetchPojo.addEventListener('click', () => { + const fetchOptions = { + headers: { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-release=4.2.0', + }, + }; + + // Make two fetch requests that reuse the same fetch object + Sentry.startSpan({ name: 'does-not-matter-1' }, () => + fetch('http://example.com/fetch-pojo', fetchOptions) + .then(res => res.text()) + .then(() => + Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)), + ), + ); +}); + +fetchArray.addEventListener('click', () => { + const fetchOptions = { + headers: [ + ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'], + ['baggage', 'sentry-release=4.2.0'], + ], + }; + + // Make two fetch requests that reuse the same fetch object + Sentry.startSpan({ name: 'does-not-matter-1' }, () => + fetch('http://example.com/fetch-array', fetchOptions) + .then(res => res.text()) + .then(() => + Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)), + ), + ); +}); + +fetchHeaders.addEventListener('click', () => { + const fetchOptions = { + headers: new Headers({ + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-release=4.2.0', + }), + }; + + // Make two fetch requests that reuse the same fetch object + Sentry.startSpan({ name: 'does-not-matter-1' }, () => + fetch('http://example.com/fetch-headers', fetchOptions) + .then(res => res.text()) + .then(() => + Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)), + ), + ); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html new file mode 100644 index 000000000000..dc60d6d83808 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts new file mode 100644 index 000000000000..5ae2f83924d0 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts @@ -0,0 +1,64 @@ +import type { Page, Request } from '@playwright/test'; +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +async function assertRequests({ + page, + buttonSelector, + requestMatcher, +}: { page: Page; buttonSelector: string; requestMatcher: string }) { + const requests = await new Promise(resolve => { + const requests: Request[] = []; + page + .route(requestMatcher, (route, request) => { + requests.push(request); + if (requests.length === 2) { + resolve(requests); + } + + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({}), + }); + }) + .then(() => { + page.click(buttonSelector); + }); + }); + + requests.forEach(request => { + const headers = request.headers(); + + // No merged sentry trace headers + expect(headers['sentry-trace']).not.toContain(','); + + // No multiple baggage entries + expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1); + }); +} + +sentryTest( + 'Ensure the SDK does not infinitely append tracing headers to outgoing requests', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + await page.goto(url); + + await sentryTest.step('fetch with POJO', () => + assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }), + ); + + await sentryTest.step('fetch with array', () => + assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }), + ); + + await sentryTest.step('fetch with Headers instance', () => + assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }), + ); + }, +); diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index 9b9d2ece836b..26a993ff0e12 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -1,6 +1,7 @@ import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types'; import { BAGGAGE_HEADER_NAME, + SENTRY_BAGGAGE_KEY_PREFIX, dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, isInstanceOf, @@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest( request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package, client: Client, scope: Scope, - options: { + fetchOptionsObj: { headers?: | { [key: string]: string[] | string | undefined; @@ -145,7 +146,7 @@ export function addTracingHeadersToFetchRequest( ); const headers = - options.headers || + fetchOptionsObj.headers || (typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined); if (!headers) { @@ -153,17 +154,45 @@ export function addTracingHeadersToFetchRequest( } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) { const newHeaders = new Headers(headers as Headers); - newHeaders.append('sentry-trace', sentryTraceHeader); + newHeaders.set('sentry-trace', sentryTraceHeader); if (sentryBaggageHeader) { - // If the same header is appended multiple times the browser will merge the values into a single request header. - // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. - newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader); + const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME); + if (prevBaggageHeader) { + const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader); + newHeaders.set( + BAGGAGE_HEADER_NAME, + // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header + // otherwise just set the sentry baggage header + prevHeaderStrippedFromSentryBaggage + ? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}` + : sentryBaggageHeader, + ); + } else { + newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader); + } } return newHeaders as PolymorphicRequestHeaders; } else if (Array.isArray(headers)) { - const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]]; + const newHeaders = [ + ...headers + // Remove any existing sentry-trace headers + .filter(header => { + return !(Array.isArray(header) && header[0] === 'sentry-trace'); + }) + // Get rid of previous sentry baggage values in baggage header + .map(header => { + if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') { + const [headerName, headerValue, ...rest] = header; + return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest]; + } else { + return header; + } + }), + // Attach the new sentry-trace header + ['sentry-trace', sentryTraceHeader], + ]; if (sentryBaggageHeader) { // If there are multiple entries with the same key, the browser will merge the values into a single request header. @@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest( return newHeaders as PolymorphicRequestHeaders; } else { const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined; - const newBaggageHeaders: string[] = []; + let newBaggageHeaders: string[] = []; if (Array.isArray(existingBaggageHeader)) { - newBaggageHeaders.push(...existingBaggageHeader); + newBaggageHeaders = existingBaggageHeader + .map(headerItem => + typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem, + ) + .filter(headerItem => headerItem === ''); } else if (existingBaggageHeader) { - newBaggageHeaders.push(existingBaggageHeader); + newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader)); } if (sentryBaggageHeader) { @@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void { } span.end(); } + +function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string { + return ( + baggageHeader + .split(',') + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)) + .join(',') + ); +} From ab292f4921f9f7aa4e2a9cf13594b0064e297f74 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 16 Oct 2024 13:19:19 +0000 Subject: [PATCH 21/34] chore(deps): bump astro from 4.1.1 to 4.16.1 in /dev-packages/e2e-tests/test-applications/cloudflare-astro (#13974) --- .../e2e-tests/test-applications/cloudflare-astro/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-astro/package.json b/dev-packages/e2e-tests/test-applications/cloudflare-astro/package.json index a69841091fb1..c8109962ebd0 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-astro/package.json +++ b/dev-packages/e2e-tests/test-applications/cloudflare-astro/package.json @@ -19,7 +19,7 @@ "dependencies": { "@astrojs/cloudflare": "8.1.0", "@sentry/astro": "latest || *", - "astro": "4.1.1" + "astro": "4.16.1" }, "volta": { "extends": "../../package.json" From 471cf9aec741f71a8b49b51642fd81d730ddeb8e Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 16 Oct 2024 16:27:41 +0300 Subject: [PATCH 22/34] chore(node): Bump `@opentelemetry/instrumentation-fastify` to `0.40.0` (#13983) Resolves: #13774 Fastify v5 is enabled with release: https://github.com/open-telemetry/opentelemetry-js-contrib/releases/tag/instrumentation-fastify-v0.40.0 --- .github/workflows/build.yml | 1 + .../node-fastify-5/.gitignore | 1 + .../test-applications/node-fastify-5/.npmrc | 2 + .../node-fastify-5/package.json | 31 ++ .../node-fastify-5/playwright.config.mjs | 7 + .../node-fastify-5/src/app.ts | 153 ++++++++ .../node-fastify-5/start-event-proxy.mjs | 6 + .../node-fastify-5/tests/errors.test.ts | 29 ++ .../node-fastify-5/tests/propagation.test.ts | 356 ++++++++++++++++++ .../node-fastify-5/tests/transactions.test.ts | 126 +++++++ .../node-fastify-5/tsconfig.json | 10 + packages/node/package.json | 2 +- yarn.lock | 8 +- 13 files changed, 727 insertions(+), 5 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/.gitignore create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/.npmrc create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/package.json create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/playwright.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/src/app.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/start-event-proxy.mjs create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/tests/transactions.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/tsconfig.json diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 59ab696758c2..ce139e9e7787 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -921,6 +921,7 @@ jobs: 'tanstack-router', 'generic-ts3.8', 'node-fastify', + 'node-fastify-5', 'node-hapi', 'node-nestjs-basic', 'node-nestjs-distributed-tracing', diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/.gitignore b/dev-packages/e2e-tests/test-applications/node-fastify-5/.gitignore new file mode 100644 index 000000000000..1521c8b7652b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/.gitignore @@ -0,0 +1 @@ +dist diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/.npmrc b/dev-packages/e2e-tests/test-applications/node-fastify-5/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json b/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json new file mode 100644 index 000000000000..4d1385df12b8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json @@ -0,0 +1,31 @@ +{ + "name": "node-fastify-5", + "version": "1.0.0", + "private": true, + "scripts": { + "start": "ts-node src/app.ts", + "test": "playwright test", + "clean": "npx rimraf node_modules pnpm-lock.yaml", + "typecheck": "tsc", + "test:build": "pnpm install && pnpm run typecheck", + "test:assert": "pnpm test" + }, + "dependencies": { + "@sentry/node": "latest || *", + "@sentry/types": "latest || *", + "@sentry/core": "latest || *", + "@sentry/utils": "latest || *", + "@sentry/opentelemetry": "latest || *", + "@types/node": "22.7.5", + "fastify": "5.0.0", + "typescript": "5.6.3", + "ts-node": "10.9.2" + }, + "devDependencies": { + "@playwright/test": "^1.44.1", + "@sentry-internal/test-utils": "link:../../../test-utils" + }, + "volta": { + "extends": "../../package.json" + } +} diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/node-fastify-5/playwright.config.mjs new file mode 100644 index 000000000000..31f2b913b58b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/playwright.config.mjs @@ -0,0 +1,7 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: `pnpm start`, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app.ts b/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app.ts new file mode 100644 index 000000000000..275dfa786ca3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app.ts @@ -0,0 +1,153 @@ +import type * as S from '@sentry/node'; +const Sentry = require('@sentry/node') as typeof S; + +// We wrap console.warn to find out if a warning is incorrectly logged +console.warn = new Proxy(console.warn, { + apply: function (target, thisArg, argumentsList) { + const msg = argumentsList[0]; + if (typeof msg === 'string' && msg.startsWith('[Sentry]')) { + console.error(`Sentry warning was triggered: ${msg}`); + process.exit(1); + } + + return target.apply(thisArg, argumentsList); + }, +}); + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.E2E_TEST_DSN, + integrations: [], + tracesSampleRate: 1, + tunnel: 'http://localhost:3031/', // proxy server + tracePropagationTargets: ['http://localhost:3030', '/external-allowed'], +}); + +import type * as H from 'http'; +import type * as F from 'fastify'; + +// Make sure fastify is imported after Sentry is initialized +const { fastify } = require('fastify') as typeof F; +const http = require('http') as typeof H; + +const app = fastify(); +const port = 3030; +const port2 = 3040; + +Sentry.setupFastifyErrorHandler(app); + +app.get('/test-success', function (_req, res) { + res.send({ version: 'v1' }); +}); + +app.get<{ Params: { param: string } }>('/test-param/:param', function (req, res) { + res.send({ paramWas: req.params.param }); +}); + +app.get<{ Params: { id: string } }>('/test-inbound-headers/:id', function (req, res) { + const headers = req.headers; + + res.send({ headers, id: req.params.id }); +}); + +app.get<{ Params: { id: string } }>('/test-outgoing-http/:id', async function (req, res) { + const id = req.params.id; + const data = await makeHttpRequest(`http://localhost:3030/test-inbound-headers/${id}`); + + res.send(data); +}); + +app.get<{ Params: { id: string } }>('/test-outgoing-fetch/:id', async function (req, res) { + const id = req.params.id; + const response = await fetch(`http://localhost:3030/test-inbound-headers/${id}`); + const data = await response.json(); + + res.send(data); +}); + +app.get('/test-transaction', async function (req, res) { + Sentry.startSpan({ name: 'test-span' }, () => { + Sentry.startSpan({ name: 'child-span' }, () => {}); + }); + + res.send({}); +}); + +app.get('/test-error', async function (req, res) { + const exceptionId = Sentry.captureException(new Error('This is an error')); + + await Sentry.flush(2000); + + res.send({ exceptionId }); +}); + +app.get<{ Params: { id: string } }>('/test-exception/:id', async function (req, res) { + throw new Error(`This is an exception with id ${req.params.id}`); +}); + +app.get('/test-outgoing-fetch-external-allowed', async function (req, res) { + const fetchResponse = await fetch(`http://localhost:${port2}/external-allowed`); + const data = await fetchResponse.json(); + + res.send(data); +}); + +app.get('/test-outgoing-fetch-external-disallowed', async function (req, res) { + const fetchResponse = await fetch(`http://localhost:${port2}/external-disallowed`); + const data = await fetchResponse.json(); + + res.send(data); +}); + +app.get('/test-outgoing-http-external-allowed', async function (req, res) { + const data = await makeHttpRequest(`http://localhost:${port2}/external-allowed`); + res.send(data); +}); + +app.get('/test-outgoing-http-external-disallowed', async function (req, res) { + const data = await makeHttpRequest(`http://localhost:${port2}/external-disallowed`); + res.send(data); +}); + +app.listen({ port: port }); + +// A second app so we can test header propagation between external URLs +const app2 = fastify(); +app2.get('/external-allowed', function (req, res) { + const headers = req.headers; + + res.send({ headers, route: '/external-allowed' }); +}); + +app2.get('/external-disallowed', function (req, res) { + const headers = req.headers; + + res.send({ headers, route: '/external-disallowed' }); +}); + +app2.listen({ port: port2 }); + +function makeHttpRequest(url: string) { + return new Promise(resolve => { + const data: any[] = []; + + http + .request(url, httpRes => { + httpRes.on('data', chunk => { + data.push(chunk); + }); + httpRes.on('error', error => { + resolve({ error: error.message, url }); + }); + httpRes.on('end', () => { + try { + const json = JSON.parse(Buffer.concat(data).toString()); + resolve(json); + } catch { + resolve({ data: Buffer.concat(data).toString(), url }); + } + }); + }) + .end(); + }); +} diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/node-fastify-5/start-event-proxy.mjs new file mode 100644 index 000000000000..0de44700dd11 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'node-fastify-5', +}); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts new file mode 100644 index 000000000000..67b27a6c0e5e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts @@ -0,0 +1,29 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test('Sends correct error event', async ({ baseURL }) => { + const errorEventPromise = waitForError('node-fastify-5', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123'; + }); + + await fetch(`${baseURL}/test-exception/123`); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123'); + + expect(errorEvent.request).toEqual({ + method: 'GET', + cookies: {}, + headers: expect.any(Object), + url: 'http://localhost:3030/test-exception/123', + }); + + expect(errorEvent.transaction).toEqual('GET /test-exception/:id'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts new file mode 100644 index 000000000000..050ce19c6a3c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/propagation.test.ts @@ -0,0 +1,356 @@ +import crypto from 'crypto'; +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { SpanJSON } from '@sentry/types'; + +test('Propagates trace for outgoing http requests', async ({ baseURL }) => { + const id = crypto.randomUUID(); + + const inboundTransactionPromise = waitForTransaction('node-fastify-5', transactionEvent => { + return ( + transactionEvent.contexts?.trace?.op === 'http.server' && + transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-inbound-headers/${id}` + ); + }); + + const outboundTransactionPromise = waitForTransaction('node-fastify-5', transactionEvent => { + return ( + transactionEvent.contexts?.trace?.op === 'http.server' && + transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-http/${id}` + ); + }); + + const response = await fetch(`${baseURL}/test-outgoing-http/${id}`); + const data = await response.json(); + + const inboundTransaction = await inboundTransactionPromise; + const outboundTransaction = await outboundTransactionPromise; + + const traceId = outboundTransaction?.contexts?.trace?.trace_id; + const outgoingHttpSpan = outboundTransaction?.spans?.find(span => span.op === 'http.client') as SpanJSON | undefined; + + expect(outgoingHttpSpan).toBeDefined(); + + const outgoingHttpSpanId = outgoingHttpSpan?.span_id; + + expect(traceId).toEqual(expect.any(String)); + + // data is passed through from the inbound request, to verify we have the correct headers set + const inboundHeaderSentryTrace = data.headers?.['sentry-trace']; + const inboundHeaderBaggage = data.headers?.['baggage']; + + expect(inboundHeaderSentryTrace).toEqual(`${traceId}-${outgoingHttpSpanId}-1`); + expect(inboundHeaderBaggage).toBeDefined(); + + const baggage = (inboundHeaderBaggage || '').split(','); + expect(baggage).toEqual( + expect.arrayContaining([ + 'sentry-environment=qa', + `sentry-trace_id=${traceId}`, + expect.stringMatching(/sentry-public_key=/), + ]), + ); + + expect(outboundTransaction.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: `http://localhost:3030/test-outgoing-http/${id}`, + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': `http://localhost:3030/test-outgoing-http/${id}`, + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': `/test-outgoing-http/${id}`, + 'http.user_agent': 'node', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': expect.any(String), + 'net.host.port': expect.any(Number), + 'net.peer.ip': expect.any(String), + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-outgoing-http/:id', + }, + op: 'http.server', + span_id: expect.any(String), + status: 'ok', + trace_id: traceId, + origin: 'auto.http.otel.http', + }); + + expect(inboundTransaction.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: `http://localhost:3030/test-inbound-headers/${id}`, + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': `http://localhost:3030/test-inbound-headers/${id}`, + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': `/test-inbound-headers/${id}`, + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': expect.any(String), + 'net.host.port': expect.any(Number), + 'net.peer.ip': expect.any(String), + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-inbound-headers/:id', + }, + op: 'http.server', + parent_span_id: outgoingHttpSpanId, + span_id: expect.any(String), + status: 'ok', + trace_id: traceId, + origin: 'auto.http.otel.http', + }); +}); + +test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { + const id = crypto.randomUUID(); + + const inboundTransactionPromise = waitForTransaction('node-fastify-5', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-inbound-headers/${id}` + ); + }); + + const outboundTransactionPromise = waitForTransaction('node-fastify-5', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-fetch/${id}` + ); + }); + + const response = await fetch(`${baseURL}/test-outgoing-fetch/${id}`); + const data = await response.json(); + + const inboundTransaction = await inboundTransactionPromise; + const outboundTransaction = await outboundTransactionPromise; + + const traceId = outboundTransaction?.contexts?.trace?.trace_id; + const outgoingHttpSpan = outboundTransaction?.spans?.find(span => span.op === 'http.client') as SpanJSON | undefined; + + expect(outgoingHttpSpan).toBeDefined(); + + const outgoingHttpSpanId = outgoingHttpSpan?.span_id; + + expect(traceId).toEqual(expect.any(String)); + + // data is passed through from the inbound request, to verify we have the correct headers set + const inboundHeaderSentryTrace = data.headers?.['sentry-trace']; + const inboundHeaderBaggage = data.headers?.['baggage']; + + expect(inboundHeaderSentryTrace).toEqual(`${traceId}-${outgoingHttpSpanId}-1`); + expect(inboundHeaderBaggage).toBeDefined(); + + const baggage = (inboundHeaderBaggage || '').split(','); + expect(baggage).toEqual( + expect.arrayContaining([ + 'sentry-environment=qa', + `sentry-trace_id=${traceId}`, + expect.stringMatching(/sentry-public_key=/), + ]), + ); + + expect(outboundTransaction.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: `http://localhost:3030/test-outgoing-fetch/${id}`, + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': `http://localhost:3030/test-outgoing-fetch/${id}`, + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': `/test-outgoing-fetch/${id}`, + 'http.user_agent': 'node', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': expect.any(String), + 'net.host.port': expect.any(Number), + 'net.peer.ip': expect.any(String), + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-outgoing-fetch/:id', + }, + op: 'http.server', + span_id: expect.any(String), + status: 'ok', + trace_id: traceId, + origin: 'auto.http.otel.http', + }); + + expect(inboundTransaction.contexts?.trace).toEqual({ + data: expect.objectContaining({ + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: `http://localhost:3030/test-inbound-headers/${id}`, + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': `http://localhost:3030/test-inbound-headers/${id}`, + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': `/test-inbound-headers/${id}`, + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': expect.any(String), + 'net.host.port': expect.any(Number), + 'net.peer.ip': expect.any(String), + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-inbound-headers/:id', + }), + op: 'http.server', + parent_span_id: outgoingHttpSpanId, + span_id: expect.any(String), + status: 'ok', + trace_id: traceId, + origin: 'auto.http.otel.http', + }); +}); + +test('Propagates trace for outgoing external http requests', async ({ baseURL }) => { + const inboundTransactionPromise = waitForTransaction('node-fastify-5', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-http-external-allowed` + ); + }); + + const response = await fetch(`${baseURL}/test-outgoing-http-external-allowed`); + const data = await response.json(); + + const inboundTransaction = await inboundTransactionPromise; + + const traceId = inboundTransaction?.contexts?.trace?.trace_id; + const spanId = inboundTransaction?.spans?.find(span => span.op === 'http.client')?.span_id; + + expect(traceId).toEqual(expect.any(String)); + expect(spanId).toEqual(expect.any(String)); + + expect(data).toEqual({ + route: '/external-allowed', + headers: expect.objectContaining({ + 'sentry-trace': `${traceId}-${spanId}-1`, + baggage: expect.any(String), + }), + }); + + const baggage = (data.headers.baggage || '').split(','); + expect(baggage).toEqual( + expect.arrayContaining([ + 'sentry-environment=qa', + `sentry-trace_id=${traceId}`, + expect.stringMatching(/sentry-public_key=/), + ]), + ); +}); + +test('Does not propagate outgoing http requests not covered by tracePropagationTargets', async ({ baseURL }) => { + const inboundTransactionPromise = waitForTransaction('node-fastify-5', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-http-external-disallowed` + ); + }); + + const response = await fetch(`${baseURL}/test-outgoing-http-external-disallowed`); + const data = await response.json(); + + const inboundTransaction = await inboundTransactionPromise; + + const traceId = inboundTransaction?.contexts?.trace?.trace_id; + const spanId = inboundTransaction?.spans?.find(span => span.op === 'http.client')?.span_id; + + expect(traceId).toEqual(expect.any(String)); + expect(spanId).toEqual(expect.any(String)); + + expect(data.route).toBe('/external-disallowed'); + expect(data.headers?.['sentry-trace']).toBeUndefined(); + expect(data.headers?.baggage).toBeUndefined(); +}); + +test('Propagates trace for outgoing external fetch requests', async ({ baseURL }) => { + const inboundTransactionPromise = waitForTransaction('node-fastify-5', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-fetch-external-allowed` + ); + }); + + const response = await fetch(`${baseURL}/test-outgoing-fetch-external-allowed`); + const data = await response.json(); + + const inboundTransaction = await inboundTransactionPromise; + + const traceId = inboundTransaction?.contexts?.trace?.trace_id; + const spanId = inboundTransaction?.spans?.find(span => span.op === 'http.client')?.span_id; + + expect(traceId).toEqual(expect.any(String)); + expect(spanId).toEqual(expect.any(String)); + + expect(data).toEqual({ + route: '/external-allowed', + headers: expect.objectContaining({ + 'sentry-trace': `${traceId}-${spanId}-1`, + baggage: expect.any(String), + }), + }); + + const baggage = (data.headers.baggage || '').split(','); + expect(baggage).toEqual( + expect.arrayContaining([ + 'sentry-environment=qa', + `sentry-trace_id=${traceId}`, + expect.stringMatching(/sentry-public_key=/), + ]), + ); +}); + +test('Does not propagate outgoing fetch requests not covered by tracePropagationTargets', async ({ baseURL }) => { + const inboundTransactionPromise = waitForTransaction('node-fastify-5', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-fetch-external-disallowed` + ); + }); + + const response = await fetch(`${baseURL}/test-outgoing-fetch-external-disallowed`); + const data = await response.json(); + + const inboundTransaction = await inboundTransactionPromise; + + const traceId = inboundTransaction?.contexts?.trace?.trace_id; + const spanId = inboundTransaction?.spans?.find(span => span.op === 'http.client')?.span_id; + + expect(traceId).toEqual(expect.any(String)); + expect(spanId).toEqual(expect.any(String)); + + expect(data.route).toBe('/external-disallowed'); + expect(data.headers?.['sentry-trace']).toBeUndefined(); + expect(data.headers?.baggage).toBeUndefined(); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/transactions.test.ts new file mode 100644 index 000000000000..6ca2d19f3b32 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/transactions.test.ts @@ -0,0 +1,126 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test('Sends an API route transaction', async ({ baseURL }) => { + const pageloadTransactionEventPromise = waitForTransaction('node-fastify-5', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /test-transaction' + ); + }); + + await fetch(`${baseURL}/test-transaction`); + + const transactionEvent = await pageloadTransactionEventPromise; + + expect(transactionEvent.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: 'http://localhost:3030/test-transaction', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': 'http://localhost:3030/test-transaction', + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': '/test-transaction', + 'http.user_agent': 'node', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': expect.any(String), + 'net.host.port': expect.any(Number), + 'net.peer.ip': expect.any(String), + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-transaction', + }, + op: 'http.server', + span_id: expect.any(String), + status: 'ok', + trace_id: expect.any(String), + origin: 'auto.http.otel.http', + }); + + expect(transactionEvent).toEqual( + expect.objectContaining({ + transaction: 'GET /test-transaction', + type: 'transaction', + transaction_info: { + source: 'route', + }, + }), + ); + + const spans = transactionEvent.spans || []; + + expect(spans).toContainEqual({ + data: { + 'plugin.name': 'fastify -> sentry-fastify-error-handler', + 'fastify.type': 'middleware', + 'hook.name': 'onRequest', + 'sentry.origin': 'auto.http.otel.fastify', + 'sentry.op': 'middleware.fastify', + }, + description: 'sentry-fastify-error-handler', + op: 'middleware.fastify', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'auto.http.otel.fastify', + }); + + expect(spans).toContainEqual({ + data: { + 'plugin.name': 'fastify -> sentry-fastify-error-handler', + 'fastify.type': 'request_handler', + 'http.route': '/test-transaction', + 'sentry.op': 'request_handler.fastify', + 'sentry.origin': 'auto.http.otel.fastify', + }, + description: 'sentry-fastify-error-handler', + op: 'request_handler.fastify', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'auto.http.otel.fastify', + }); + + expect(spans).toContainEqual({ + data: { + 'sentry.origin': 'manual', + }, + description: 'test-span', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'manual', + }); + + expect(spans).toContainEqual({ + data: { + 'sentry.origin': 'manual', + }, + description: 'child-span', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'manual', + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/tsconfig.json b/dev-packages/e2e-tests/test-applications/node-fastify-5/tsconfig.json new file mode 100644 index 000000000000..6b69bfaa593b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/tsconfig.json @@ -0,0 +1,10 @@ +{ + "compilerOptions": { + "types": ["node"], + "esModuleInterop": true, + "lib": ["dom", "dom.iterable", "esnext"], + "strict": true, + "outDir": "dist" + }, + "include": ["./src/*.ts"] +} diff --git a/packages/node/package.json b/packages/node/package.json index 52559d93ed27..03362092b86a 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -73,7 +73,7 @@ "@opentelemetry/instrumentation-connect": "0.39.0", "@opentelemetry/instrumentation-dataloader": "0.12.0", "@opentelemetry/instrumentation-express": "0.43.0", - "@opentelemetry/instrumentation-fastify": "0.39.0", + "@opentelemetry/instrumentation-fastify": "0.40.0", "@opentelemetry/instrumentation-fs": "0.15.0", "@opentelemetry/instrumentation-generic-pool": "0.39.0", "@opentelemetry/instrumentation-graphql": "0.43.0", diff --git a/yarn.lock b/yarn.lock index db64fa00fc6c..48991d296050 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7335,10 +7335,10 @@ "@opentelemetry/instrumentation" "^0.53.0" "@opentelemetry/semantic-conventions" "^1.27.0" -"@opentelemetry/instrumentation-fastify@0.39.0": - version "0.39.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-fastify/-/instrumentation-fastify-0.39.0.tgz#96a040e4944daf77c53a8fe5a128bc3b2568e4aa" - integrity sha512-SS9uSlKcsWZabhBp2szErkeuuBDgxOUlllwkS92dVaWRnMmwysPhcEgHKB8rUe3BHg/GnZC1eo1hbTZv4YhfoA== +"@opentelemetry/instrumentation-fastify@0.40.0": + version "0.40.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-fastify/-/instrumentation-fastify-0.40.0.tgz#0c57608ac202337d56b53338f1fc9369d224306b" + integrity sha512-74qj4nG3zPtU7g2x4sm2T4R3/pBMyrYstTsqSZwdlhQk1SD4l8OSY9sPRX1qkhfxOuW3U4KZQAV/Cymb3fB6hg== dependencies: "@opentelemetry/core" "^1.8.0" "@opentelemetry/instrumentation" "^0.53.0" From 8782af8dc7d9addf32a80dece5f14072af9cdc56 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Wed, 16 Oct 2024 17:07:03 +0200 Subject: [PATCH 23/34] test(nuxt): Use `sentry.server.config.ts` in E2E tests (#13999) The E2E tests still used an `instrument` file in the `public` folder (which is not the way to setup the SDK anymore) because `import-in-the-middle/hook.mjs` was not available and threw an error. To be able to properly test the setup with `sentry.server.config.ts`, a function was added that copies the `import-in-the-middle` folder to the build output to include all files. --- .../e2e-tests/test-applications/nuxt-3/copyIITM.bash | 7 +++++++ .../e2e-tests/test-applications/nuxt-3/nuxt.config.ts | 6 ++++++ .../e2e-tests/test-applications/nuxt-3/package.json | 8 ++++++-- .../test-applications/nuxt-3/playwright.config.ts | 2 +- .../instrument.server.mjs => sentry.server.config.ts} | 0 .../e2e-tests/test-applications/nuxt-4/copyIITM.bash | 7 +++++++ .../e2e-tests/test-applications/nuxt-4/nuxt.config.ts | 6 ++++++ .../e2e-tests/test-applications/nuxt-4/package.json | 8 ++++++-- .../test-applications/nuxt-4/playwright.config.ts | 2 +- .../instrument.server.mjs => sentry.server.config.ts} | 0 .../test-applications/nuxt-4/tests/errors.client.test.ts | 2 +- .../test-applications/nuxt-4/tests/tracing.client.test.ts | 2 +- 12 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nuxt-3/copyIITM.bash rename dev-packages/e2e-tests/test-applications/nuxt-3/{public/instrument.server.mjs => sentry.server.config.ts} (100%) create mode 100644 dev-packages/e2e-tests/test-applications/nuxt-4/copyIITM.bash rename dev-packages/e2e-tests/test-applications/nuxt-4/{public/instrument.server.mjs => sentry.server.config.ts} (100%) diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/copyIITM.bash b/dev-packages/e2e-tests/test-applications/nuxt-3/copyIITM.bash new file mode 100644 index 000000000000..0e04d001c968 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/copyIITM.bash @@ -0,0 +1,7 @@ +# This script copies the `import-in-the-middle` content of the E2E test project root `node_modules` to the build output `node_modules` +# For some reason, some files are missing in the output (like `hook.mjs`) and this is not reproducible in external, standalone projects. +# +# Things we tried (that did not fix the problem): +# - Adding a resolution for `@vercel/nft` v0.27.0 (this worked in the standalone project) +# - Also adding `@vercel/nft` v0.27.0 to pnpm `peerDependencyRules` +cp -r node_modules/.pnpm/import-in-the-middle@1.*/node_modules/import-in-the-middle .output/server/node_modules/import-in-the-middle diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/nuxt.config.ts b/dev-packages/e2e-tests/test-applications/nuxt-3/nuxt.config.ts index 0fcccd560af9..87e046ed39e9 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/nuxt.config.ts +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/nuxt.config.ts @@ -11,4 +11,10 @@ export default defineNuxtConfig({ }, }, }, + nitro: { + rollupConfig: { + // @sentry/... is set external to prevent bundling all of Sentry into the `runtime.mjs` file in the build output + external: [/@sentry\/.*/], + }, + }, }); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/package.json b/dev-packages/e2e-tests/test-applications/nuxt-3/package.json index 7173aeaa4ce5..079beb8bfc86 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/package.json +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/package.json @@ -3,10 +3,11 @@ "private": true, "type": "module", "scripts": { - "build": "nuxt build", + "build": "nuxt build && bash ./copyIITM.bash", "dev": "nuxt dev", "generate": "nuxt generate", - "preview": "NODE_OPTIONS='--import ./public/instrument.server.mjs' nuxt preview", + "preview": "nuxt preview", + "start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs", "clean": "npx nuxi cleanup", "test": "playwright test", "test:build": "pnpm install && npx playwright install && pnpm build", @@ -20,5 +21,8 @@ "@nuxt/test-utils": "^3.14.1", "@playwright/test": "^1.44.1", "@sentry-internal/test-utils": "link:../../../test-utils" + }, + "overrides": { + "@vercel/nft": "0.27.4" } } diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/playwright.config.ts b/dev-packages/e2e-tests/test-applications/nuxt-3/playwright.config.ts index d1094993131d..aa1ff8e9743c 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/playwright.config.ts +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/playwright.config.ts @@ -12,7 +12,7 @@ const nuxtConfigOptions: ConfigOptions = { * Like this: import { expect, test } from '@nuxt/test-utils/playwright' */ const config = getPlaywrightConfig({ - startCommand: `pnpm preview`, + startCommand: `pnpm start`, use: { ...nuxtConfigOptions }, }); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/public/instrument.server.mjs b/dev-packages/e2e-tests/test-applications/nuxt-3/sentry.server.config.ts similarity index 100% rename from dev-packages/e2e-tests/test-applications/nuxt-3/public/instrument.server.mjs rename to dev-packages/e2e-tests/test-applications/nuxt-3/sentry.server.config.ts diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4/copyIITM.bash b/dev-packages/e2e-tests/test-applications/nuxt-4/copyIITM.bash new file mode 100644 index 000000000000..0e04d001c968 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nuxt-4/copyIITM.bash @@ -0,0 +1,7 @@ +# This script copies the `import-in-the-middle` content of the E2E test project root `node_modules` to the build output `node_modules` +# For some reason, some files are missing in the output (like `hook.mjs`) and this is not reproducible in external, standalone projects. +# +# Things we tried (that did not fix the problem): +# - Adding a resolution for `@vercel/nft` v0.27.0 (this worked in the standalone project) +# - Also adding `@vercel/nft` v0.27.0 to pnpm `peerDependencyRules` +cp -r node_modules/.pnpm/import-in-the-middle@1.*/node_modules/import-in-the-middle .output/server/node_modules/import-in-the-middle diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4/nuxt.config.ts b/dev-packages/e2e-tests/test-applications/nuxt-4/nuxt.config.ts index 90f26a8ea6b6..c00ba0d5d9ed 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-4/nuxt.config.ts +++ b/dev-packages/e2e-tests/test-applications/nuxt-4/nuxt.config.ts @@ -13,4 +13,10 @@ export default defineNuxtConfig({ }, }, }, + nitro: { + rollupConfig: { + // @sentry/... is set external to prevent bundling all of Sentry into the `runtime.mjs` file in the build output + external: [/@sentry\/.*/], + }, + }, }); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4/package.json b/dev-packages/e2e-tests/test-applications/nuxt-4/package.json index 14e28d852c88..5e7bcd57af0e 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-4/package.json +++ b/dev-packages/e2e-tests/test-applications/nuxt-4/package.json @@ -3,10 +3,11 @@ "private": true, "type": "module", "scripts": { - "build": "nuxt build", + "build": "nuxt build && bash ./copyIITM.bash", "dev": "nuxt dev", "generate": "nuxt generate", - "preview": "NODE_OPTIONS='--import ./public/instrument.server.mjs' nuxt preview", + "preview": "nuxt preview", + "start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs", "clean": "npx nuxi cleanup", "test": "playwright test", "test:build": "pnpm install && npx playwright install && pnpm build", @@ -20,5 +21,8 @@ "@nuxt/test-utils": "^3.14.2", "@playwright/test": "^1.44.1", "@sentry-internal/test-utils": "link:../../../test-utils" + }, + "overrides": { + "@vercel/nft": "0.27.4" } } diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4/playwright.config.ts b/dev-packages/e2e-tests/test-applications/nuxt-4/playwright.config.ts index d1094993131d..aa1ff8e9743c 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-4/playwright.config.ts +++ b/dev-packages/e2e-tests/test-applications/nuxt-4/playwright.config.ts @@ -12,7 +12,7 @@ const nuxtConfigOptions: ConfigOptions = { * Like this: import { expect, test } from '@nuxt/test-utils/playwright' */ const config = getPlaywrightConfig({ - startCommand: `pnpm preview`, + startCommand: `pnpm start`, use: { ...nuxtConfigOptions }, }); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4/public/instrument.server.mjs b/dev-packages/e2e-tests/test-applications/nuxt-4/sentry.server.config.ts similarity index 100% rename from dev-packages/e2e-tests/test-applications/nuxt-4/public/instrument.server.mjs rename to dev-packages/e2e-tests/test-applications/nuxt-4/sentry.server.config.ts diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4/tests/errors.client.test.ts b/dev-packages/e2e-tests/test-applications/nuxt-4/tests/errors.client.test.ts index 1177218aebec..c887e255fe57 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-4/tests/errors.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/nuxt-4/tests/errors.client.test.ts @@ -1,4 +1,4 @@ -import { expect, test } from '@nuxt/test-utils/playwright'; +import { expect, test } from '@playwright/test'; import { waitForError } from '@sentry-internal/test-utils'; test.describe('client-side errors', async () => { diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4/tests/tracing.client.test.ts b/dev-packages/e2e-tests/test-applications/nuxt-4/tests/tracing.client.test.ts index 871d8b19513c..9119e279e491 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-4/tests/tracing.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/nuxt-4/tests/tracing.client.test.ts @@ -1,4 +1,4 @@ -import { expect, test } from '@nuxt/test-utils/playwright'; +import { expect, test } from '@playwright/test'; import { waitForTransaction } from '@sentry-internal/test-utils'; import type { Span } from '@sentry/nuxt'; From 6bc37f09d7c32aab01f2064c024a728807277e8a Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 17 Oct 2024 09:37:40 +0200 Subject: [PATCH 24/34] feat(nuxt): Make dynamic import() wrapping default (#13958) BREAKING CHANGE: The `--import` flag must not be added anymore. If it is still set, the server-side will be initialized twice and this leads to unexpected errors. --- First merge this: https://github.com/getsentry/sentry-javascript/pull/13945 Part of this: https://github.com/getsentry/sentry-javascript/issues/13943 This PR makes it the default to include a rollup plugin that wraps the server entry file with a dynamic import (`import()`). This is a replacement for the node `--import` CLI flag. If you still want to manually add the CLI flag you can use this option in the `nuxt.config.ts` file: ```js sentry: { dynamicImportForServerEntry: false, } ``` (option name is up for discussion) --- .../test-applications/nuxt-3/package.json | 2 +- .../test-applications/nuxt-4/package.json | 2 +- packages/nuxt/src/common/types.ts | 15 +++-- packages/nuxt/src/module.ts | 46 ++++++++++----- packages/nuxt/src/vite/addServerConfig.ts | 57 +------------------ packages/nuxt/src/vite/utils.ts | 7 ++- packages/nuxt/test/vite/utils.test.ts | 29 ++++++++-- 7 files changed, 74 insertions(+), 84 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/package.json b/dev-packages/e2e-tests/test-applications/nuxt-3/package.json index 079beb8bfc86..6c8eb1fcdd95 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/package.json +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/package.json @@ -7,7 +7,7 @@ "dev": "nuxt dev", "generate": "nuxt generate", "preview": "nuxt preview", - "start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs", + "start": "node .output/server/index.mjs", "clean": "npx nuxi cleanup", "test": "playwright test", "test:build": "pnpm install && npx playwright install && pnpm build", diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4/package.json b/dev-packages/e2e-tests/test-applications/nuxt-4/package.json index 5e7bcd57af0e..db56273a7493 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-4/package.json +++ b/dev-packages/e2e-tests/test-applications/nuxt-4/package.json @@ -7,7 +7,7 @@ "dev": "nuxt dev", "generate": "nuxt generate", "preview": "nuxt preview", - "start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs", + "start": "node .output/server/index.mjs", "clean": "npx nuxi cleanup", "test": "playwright test", "test:build": "pnpm install && npx playwright install && pnpm build", diff --git a/packages/nuxt/src/common/types.ts b/packages/nuxt/src/common/types.ts index bcc14ad1d307..6ba29752a308 100644 --- a/packages/nuxt/src/common/types.ts +++ b/packages/nuxt/src/common/types.ts @@ -103,16 +103,19 @@ export type SentryNuxtModuleOptions = { debug?: boolean; /** - * Enabling basic server tracing can be used for environments where modifying the node option `--import` is not possible. - * However, enabling this option only supports limited tracing instrumentation. Only http traces will be collected (but no database-specific traces etc.). + * Wraps the server entry file with a dynamic `import()`. This will make it possible to preload Sentry and register + * necessary hooks before other code runs. (Node docs: https://nodejs.org/api/module.html#enabling) * - * If this option is `true`, the Sentry SDK will import the Sentry server config at the top of the server entry file to load the SDK on the server. + * If this option is `false`, the Sentry SDK won't wrap the server entry file with `import()`. Not wrapping the + * server entry file will disable Sentry on the server-side. When you set this option to `false`, make sure + * to add the Sentry server config with the node `--import` CLI flag to enable Sentry on the server-side. * - * **DO NOT** enable this option if you've already added the node option `--import` in your node start script. This would initialize Sentry twice on the server-side and leads to unexpected issues. + * **DO NOT** add the node CLI flag `--import` in your node start script, when `dynamicImportForServerEntry` is set to `true` (default). + * This would initialize Sentry twice on the server-side and this leads to unexpected issues. * - * @default false + * @default true */ - experimental_basicServerTracing?: boolean; + dynamicImportForServerEntry?: boolean; /** * Options to be passed directly to the Sentry Rollup Plugin (`@sentry/rollup-plugin`) and Sentry Vite Plugin (`@sentry/vite-plugin`) that ship with the Sentry Nuxt SDK. diff --git a/packages/nuxt/src/module.ts b/packages/nuxt/src/module.ts index c74fe32b93fe..77bf8edd77de 100644 --- a/packages/nuxt/src/module.ts +++ b/packages/nuxt/src/module.ts @@ -2,7 +2,7 @@ import * as path from 'path'; import { addPlugin, addPluginTemplate, addServerPlugin, createResolver, defineNuxtModule } from '@nuxt/kit'; import { consoleSandbox } from '@sentry/utils'; import type { SentryNuxtModuleOptions } from './common/types'; -import { addSentryTopImport, addServerConfigToBuild } from './vite/addServerConfig'; +import { addDynamicImportEntryFileWrapper, addServerConfigToBuild } from './vite/addServerConfig'; import { setupSourceMaps } from './vite/sourceMaps'; import { findDefaultSdkInitFile } from './vite/utils'; @@ -17,7 +17,12 @@ export default defineNuxtModule({ }, }, defaults: {}, - setup(moduleOptions, nuxt) { + setup(moduleOptionsParam, nuxt) { + const moduleOptions = { + ...moduleOptionsParam, + dynamicImportForServerEntry: moduleOptionsParam.dynamicImportForServerEntry !== false, // default: true + }; + const moduleDirResolver = createResolver(import.meta.url); const buildDirResolver = createResolver(nuxt.options.buildDir); @@ -48,15 +53,17 @@ export default defineNuxtModule({ const serverConfigFile = findDefaultSdkInitFile('server'); if (serverConfigFile) { - // Inject the server-side Sentry config file with a side effect import - addPluginTemplate({ - mode: 'server', - filename: 'sentry-server-config.mjs', - getContents: () => - `import "${buildDirResolver.resolve(`/${serverConfigFile}`)}"\n` + - 'import { defineNuxtPlugin } from "#imports"\n' + - 'export default defineNuxtPlugin(() => {})', - }); + if (moduleOptions.dynamicImportForServerEntry === false) { + // Inject the server-side Sentry config file with a side effect import + addPluginTemplate({ + mode: 'server', + filename: 'sentry-server-config.mjs', + getContents: () => + `import "${buildDirResolver.resolve(`/${serverConfigFile}`)}"\n` + + 'import { defineNuxtPlugin } from "#imports"\n' + + 'export default defineNuxtPlugin(() => {})', + }); + } addServerPlugin(moduleDirResolver.resolve('./runtime/plugins/sentry.server')); } @@ -67,11 +74,9 @@ export default defineNuxtModule({ nuxt.hooks.hook('nitro:init', nitro => { if (serverConfigFile && serverConfigFile.includes('.server.config')) { - addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile); + if (moduleOptions.dynamicImportForServerEntry === false) { + addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile); - if (moduleOptions.experimental_basicServerTracing) { - addSentryTopImport(moduleOptions, nitro); - } else { if (moduleOptions.debug) { const serverDirResolver = createResolver(nitro.options.output.serverDir); const serverConfigPath = serverDirResolver.resolve('sentry.server.config.mjs'); @@ -86,6 +91,17 @@ export default defineNuxtModule({ ); }); } + } else { + addDynamicImportEntryFileWrapper(nitro, serverConfigFile); + + if (moduleOptions.debug) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + '[Sentry] Wrapping the server entry file with a dynamic `import()`, so Sentry can be preloaded before the server initializes.', + ); + }); + } } } }); diff --git a/packages/nuxt/src/vite/addServerConfig.ts b/packages/nuxt/src/vite/addServerConfig.ts index 5a5d2e5bd627..4c12c6fd7dc2 100644 --- a/packages/nuxt/src/vite/addServerConfig.ts +++ b/packages/nuxt/src/vite/addServerConfig.ts @@ -74,54 +74,6 @@ export function addServerConfigToBuild( }); } -/** - * Adds the Sentry server config import at the top of the server entry file to load the SDK on the server. - * This is necessary for environments where modifying the node option `--import` is not possible. - * However, only limited tracing instrumentation is supported when doing this. - */ -export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nitro: Nitro): void { - nitro.hooks.hook('close', () => { - // other presets ('node-server' or 'vercel') have an index.mjs - const presetsWithServerFile = ['netlify']; - const entryFileName = - typeof nitro.options.rollupConfig?.output.entryFileNames === 'string' - ? nitro.options.rollupConfig?.output.entryFileNames - : presetsWithServerFile.includes(nitro.options.preset) - ? 'server.mjs' - : 'index.mjs'; - - const serverDirResolver = createResolver(nitro.options.output.serverDir); - const entryFilePath = serverDirResolver.resolve(entryFileName); - - try { - fs.readFile(entryFilePath, 'utf8', (err, data) => { - const updatedContent = `import './${SERVER_CONFIG_FILENAME}.mjs';\n${data}`; - - fs.writeFile(entryFilePath, updatedContent, 'utf8', () => { - if (moduleOptions.debug) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.log( - `[Sentry] Successfully added the Sentry import to the server entry file "\`${entryFilePath}\`"`, - ); - }); - } - }); - }); - } catch (err) { - if (moduleOptions.debug) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - `[Sentry] An error occurred when trying to add the Sentry import to the server entry file "\`${entryFilePath}\`":`, - err, - ); - }); - } - } - }); -} - /** * This function modifies the Rollup configuration to include a plugin that wraps the entry file with a dynamic import (`import()`) * and adds the Sentry server config with the static `import` declaration. @@ -187,11 +139,8 @@ function wrapEntryWithDynamicImport(resolvedSentryConfigPath: string): InputPlug : resolution.id // Concatenates the query params to mark the file (also attaches names of re-exports - this is needed for serverless functions to re-export the handler) .concat(SENTRY_WRAPPED_ENTRY) - .concat( - exportedFunctions?.length - ? SENTRY_FUNCTIONS_REEXPORT.concat(exportedFunctions.join(',')).concat(QUERY_END_INDICATOR) - : '', - ); + .concat(exportedFunctions?.length ? SENTRY_FUNCTIONS_REEXPORT.concat(exportedFunctions.join(',')) : '') + .concat(QUERY_END_INDICATOR); } return null; }, @@ -211,7 +160,7 @@ function wrapEntryWithDynamicImport(resolvedSentryConfigPath: string): InputPlug // `import()` can be used for any code that should be run after the hooks are registered (https://nodejs.org/api/module.html#enabling) `import(${JSON.stringify(entryId)});\n` + // By importing "import-in-the-middle/hook.mjs", we can make sure this file wil be included, as not all node builders are including files imported with `module.register()`. - "import 'import-in-the-middle/hook.mjs'\n" + + "import 'import-in-the-middle/hook.mjs';\n" + `${reExportedFunctions}\n` ); } diff --git a/packages/nuxt/src/vite/utils.ts b/packages/nuxt/src/vite/utils.ts index 4d5bc080ba3a..1737a47e8062 100644 --- a/packages/nuxt/src/vite/utils.ts +++ b/packages/nuxt/src/vite/utils.ts @@ -57,7 +57,7 @@ export function extractFunctionReexportQueryParameters(query: string): string[] return match && match[1] ? match[1] .split(',') - .filter(param => param !== '' && param !== 'default') + .filter(param => param !== '') // Sanitize, as code could be injected with another rollup plugin .map((str: string) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) : []; @@ -72,10 +72,11 @@ export function constructFunctionReExport(pathWithQuery: string, entryId: string return functionNames.reduce( (functionsCode, currFunctionName) => functionsCode.concat( - `export async function ${currFunctionName}(...args) {\n` + + 'async function reExport(...args) {\n' + ` const res = await import(${JSON.stringify(entryId)});\n` + ` return res.${currFunctionName}.call(this, ...args);\n` + - '}\n', + '}\n' + + `export { reExport as ${currFunctionName} };\n`, ), '', ); diff --git a/packages/nuxt/test/vite/utils.test.ts b/packages/nuxt/test/vite/utils.test.ts index 0d7e91b8b83f..a38dbdc44793 100644 --- a/packages/nuxt/test/vite/utils.test.ts +++ b/packages/nuxt/test/vite/utils.test.ts @@ -71,8 +71,11 @@ describe('findDefaultSdkInitFile', () => { describe('removeSentryQueryFromPath', () => { it('strips the Sentry query part from the path', () => { const url = `/example/path${SENTRY_WRAPPED_ENTRY}${SENTRY_FUNCTIONS_REEXPORT}foo,${QUERY_END_INDICATOR}`; + const url2 = `/example/path${SENTRY_WRAPPED_ENTRY}${QUERY_END_INDICATOR}`; const result = removeSentryQueryFromPath(url); + const result2 = removeSentryQueryFromPath(url2); expect(result).toBe('/example/path'); + expect(result2).toBe('/example/path'); }); it('returns the same path if the specific query part is not present', () => { @@ -85,7 +88,7 @@ describe('removeSentryQueryFromPath', () => { describe('extractFunctionReexportQueryParameters', () => { it.each([ [`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,${QUERY_END_INDICATOR}`, ['foo', 'bar']], - [`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,default${QUERY_END_INDICATOR}`, ['foo', 'bar']], + [`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,default${QUERY_END_INDICATOR}`, ['foo', 'bar', 'default']], [ `${SENTRY_FUNCTIONS_REEXPORT}foo,a.b*c?d[e]f(g)h|i\\\\j(){hello},${QUERY_END_INDICATOR}`, ['foo', 'a\\.b\\*c\\?d\\[e\\]f\\(g\\)h\\|i\\\\\\\\j\\(\\)\\{hello\\}'], @@ -108,18 +111,36 @@ describe('constructFunctionReExport', () => { const result2 = constructFunctionReExport(query2, entryId); const expected = ` -export async function foo(...args) { +async function reExport(...args) { const res = await import("./module"); return res.foo.call(this, ...args); } -export async function bar(...args) { +export { reExport as foo }; +async function reExport(...args) { const res = await import("./module"); return res.bar.call(this, ...args); -}`; +} +export { reExport as bar }; +`; expect(result.trim()).toBe(expected.trim()); expect(result2.trim()).toBe(expected.trim()); }); + it('constructs re-export code for a "default" query parameters and entry ID', () => { + const query = `${SENTRY_FUNCTIONS_REEXPORT}default${QUERY_END_INDICATOR}}`; + const entryId = './index'; + const result = constructFunctionReExport(query, entryId); + + const expected = ` +async function reExport(...args) { + const res = await import("./index"); + return res.default.call(this, ...args); +} +export { reExport as default }; +`; + expect(result.trim()).toBe(expected.trim()); + }); + it('returns an empty string if the query string is empty', () => { const query = ''; const entryId = './module'; From ff7a07dc09730c3ec816d7daea153c4692e1df26 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 17 Oct 2024 16:49:45 +0200 Subject: [PATCH 25/34] feat(nuxt): Add Http `responseHook` with `waitUntil` (#13986) With [waitUntil](https://vercel.com/docs/functions/functions-api-reference#waituntil) the lambda execution continues until all async tasks (like sending data to Sentry) are done. Timing-wise it should work like this: `span.end()` -> `waitUntil()` -> Nitro/Node `response.end()` The problem in [this PR](https://github.com/getsentry/sentry-javascript/pull/13895) was that the Nitro hook `afterResponse` is called to late (after `response.end()`), so `waitUntil()` could not be added to this hook. --- Just for reference how this is done in Nitro (and h3, the underlying http framework): 1. The Nitro `afterResponse` hook is called in `onAfterResponse` https://github.com/unjs/nitro/blob/359af68d2b3d51d740cf869d0f13aec0c5e9f565/src/runtime/internal/app.ts#L71-L77 2. h3 `onAfterResponse` is called after the Node response was sent (and `onBeforeResponse` is called too early for calling `waitUntil`, as the span just starts at this point): https://github.com/unjs/h3/blob/7324eeec854eecc37422074ef9f2aec8a5e4a816/src/adapters/node/index.ts#L38-L47 - `endNodeResponse` calls `response.end()`: https://github.com/unjs/h3/blob/7324eeec854eecc37422074ef9f2aec8a5e4a816/src/adapters/node/internal/utils.ts#L58 --- packages/nuxt/src/server/sdk.ts | 42 +++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/packages/nuxt/src/server/sdk.ts b/packages/nuxt/src/server/sdk.ts index a6599b4ac088..59832bbb2a39 100644 --- a/packages/nuxt/src/server/sdk.ts +++ b/packages/nuxt/src/server/sdk.ts @@ -1,7 +1,12 @@ -import { applySdkMetadata, getGlobalScope } from '@sentry/core'; -import { init as initNode } from '@sentry/node'; -import type { Client, EventProcessor } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { applySdkMetadata, flush, getGlobalScope } from '@sentry/core'; +import { + type NodeOptions, + getDefaultIntegrations as getDefaultNodeIntegrations, + httpIntegration, + init as initNode, +} from '@sentry/node'; +import type { Client, EventProcessor, Integration } from '@sentry/types'; +import { logger, vercelWaitUntil } from '@sentry/utils'; import { DEBUG_BUILD } from '../common/debug-build'; import type { SentryNuxtServerOptions } from '../common/types'; @@ -14,6 +19,7 @@ export function init(options: SentryNuxtServerOptions): Client | undefined { const sentryOptions = { ...options, registerEsmLoaderHooks: mergeRegisterEsmLoaderHooks(options), + defaultIntegrations: getNuxtDefaultIntegrations(options), }; applySdkMetadata(sentryOptions, 'nuxt', ['nuxt', 'node']); @@ -46,6 +52,21 @@ export function init(options: SentryNuxtServerOptions): Client | undefined { return client; } +function getNuxtDefaultIntegrations(options: NodeOptions): Integration[] { + return [ + ...getDefaultNodeIntegrations(options).filter(integration => integration.name !== 'Http'), + // The httpIntegration is added as defaultIntegration, so users can still overwrite it + httpIntegration({ + instrumentation: { + responseHook: () => { + // Makes it possible to end the tracing span before closing the Vercel lambda (https://vercel.com/docs/functions/functions-api-reference#waituntil) + vercelWaitUntil(flushSafelyWithTimeout()); + }, + }, + }), + ]; +} + /** * Adds /vue/ to the registerEsmLoaderHooks options and merges it with the old values in the array if one is defined. * If the registerEsmLoaderHooks option is already a boolean, nothing is changed. @@ -64,3 +85,16 @@ export function mergeRegisterEsmLoaderHooks( } return options.registerEsmLoaderHooks ?? { exclude: [/vue/] }; } + +/** + * Flushes pending Sentry events with a 2-second timeout and in a way that cannot create unhandled promise rejections. + */ +export async function flushSafelyWithTimeout(): Promise { + try { + DEBUG_BUILD && logger.log('Flushing events...'); + await flush(2000); + DEBUG_BUILD && logger.log('Done flushing events'); + } catch (e) { + DEBUG_BUILD && logger.log('Error while flushing events:\n', e); + } +} From bece3e508604644f07604931bcca327078a93556 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 17 Oct 2024 10:30:45 -0700 Subject: [PATCH 26/34] ci: Ensure release comments are not added for prereleases (#13991) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Noticed here: https://github.com/getsentry/sentry-javascript/issues/13932#issuecomment-2414900079 we should not add comments for prereleases 😅 --- .github/workflows/release-comment-issues.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release-comment-issues.yml b/.github/workflows/release-comment-issues.yml index e9d1e81b75ea..4bbcb29aba21 100644 --- a/.github/workflows/release-comment-issues.yml +++ b/.github/workflows/release-comment-issues.yml @@ -20,7 +20,12 @@ jobs: run: echo "version=${{ github.event.inputs.version || github.event.release.tag_name }}" >> $GITHUB_OUTPUT - name: Comment on linked issues that are mentioned in release - if: steps.get_version.outputs.version != '' + if: | + steps.get_version.outputs.version != '' + && !contains(steps.get_version.outputs.version, '-beta.') + && !contains(steps.get_version.outputs.version, '-alpha.') + && !contains(steps.get_version.outputs.version, '-rc.') + uses: getsentry/release-comment-issues-gh-action@v1 with: github_token: ${{ secrets.GITHUB_TOKEN }} From 77b3355b2f2327ce5b761855d877b2e88e9215d7 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 17 Oct 2024 12:40:26 -0700 Subject: [PATCH 27/34] fix(replay): Ignore older performance entries when starting manually (#13969) For replay, performance entries are captured separately, and added to the replay before we send it. Generally, this works just fine, because when we buffer, we clear the captured performance events whenever the buffer is cleared. However, when we manually start the replay, it can happen that we capture performane entries from way before we started the replay manually, which will in turn extend the timeline of the replay potentially drastically. To fix this, we now drop any performance events, when starting manually, that happened more than a second before we manually started the replay. --- .../suites/replay/bufferModeManual/test.ts | 119 ++++++++++++++++++ packages/replay-internal/src/replay.ts | 13 +- 2 files changed, 131 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/replay/bufferModeManual/test.ts b/dev-packages/browser-integration-tests/suites/replay/bufferModeManual/test.ts index 09fc602d92b7..4ac7b9ea9cf1 100644 --- a/dev-packages/browser-integration-tests/suites/replay/bufferModeManual/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/bufferModeManual/test.ts @@ -287,6 +287,125 @@ sentryTest( }, ); +sentryTest( + '[buffer-mode] manually starting replay ignores earlier performance entries', + async ({ getLocalTestUrl, page, browserName }) => { + // This was sometimes flaky on webkit, so skipping for now + if (shouldSkipReplayTest() || browserName === 'webkit') { + sentryTest.skip(); + } + + const reqPromise0 = waitForReplayRequest(page, 0); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + + // Wait for everything to be initialized - Replay is not running yet + await page.waitForFunction('!!window.Replay'); + + // Wait for a second, then start replay + await new Promise(resolve => setTimeout(resolve, 1000)); + await page.evaluate('window.Replay.start()'); + + const req0 = await reqPromise0; + + const event0 = getReplayEvent(req0); + const content0 = getReplayRecordingContent(req0); + + expect(event0).toEqual( + getExpectedReplayEvent({ + replay_type: 'session', + }), + ); + + const { performanceSpans } = content0; + + // Here, we test that this does not contain any web-vital etc. performance spans + // as these have been emitted _before_ the replay was manually started + expect(performanceSpans).toEqual([ + { + op: 'memory', + description: 'memory', + startTimestamp: expect.any(Number), + endTimestamp: expect.any(Number), + data: { + memory: { + jsHeapSizeLimit: expect.any(Number), + totalJSHeapSize: expect.any(Number), + usedJSHeapSize: expect.any(Number), + }, + }, + }, + ]); + }, +); + +sentryTest( + '[buffer-mode] manually starting replay ignores earlier performance entries when starting immediately', + async ({ getLocalTestUrl, page, browserName }) => { + // This was sometimes flaky on webkit, so skipping for now + if (shouldSkipReplayTest() || browserName === 'webkit') { + sentryTest.skip(); + } + + const reqPromise0 = waitForReplayRequest(page, 0); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + page.goto(url); + + // Wait for everything to be initialized, then start replay as soon as possible + await page.waitForFunction('!!window.Replay'); + await page.evaluate('window.Replay.start()'); + + const req0 = await reqPromise0; + + const event0 = getReplayEvent(req0); + const content0 = getReplayRecordingContent(req0); + + expect(event0).toEqual( + getExpectedReplayEvent({ + replay_type: 'session', + }), + ); + + const { performanceSpans } = content0; + + expect(performanceSpans).toEqual([ + { + op: 'memory', + description: 'memory', + startTimestamp: expect.any(Number), + endTimestamp: expect.any(Number), + data: { + memory: { + jsHeapSizeLimit: expect.any(Number), + totalJSHeapSize: expect.any(Number), + usedJSHeapSize: expect.any(Number), + }, + }, + }, + ]); + }, +); + // Doing this in buffer mode to test changing error sample rate after first // error happens. sentryTest( diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index 563938fe4d43..cfd8a2a45c33 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -1046,11 +1046,22 @@ export class ReplayContainer implements ReplayContainerInterface { * are included in the replay event before it is finished and sent to Sentry. */ private _addPerformanceEntries(): Promise> { - const performanceEntries = createPerformanceEntries(this.performanceEntries).concat(this.replayPerformanceEntries); + let performanceEntries = createPerformanceEntries(this.performanceEntries).concat(this.replayPerformanceEntries); this.performanceEntries = []; this.replayPerformanceEntries = []; + // If we are manually starting, we want to ensure we only include performance entries + // that are after the initial timestamp + // The reason for this is that we may have performance entries from the page load, but may decide to start + // the replay later on, in which case we do not want to include these entries. + // without this, manually started replays can have events long before the actual replay recording starts, + // which messes with the timeline etc. + if (this._requiresManualStart) { + const initialTimestampInSeconds = this._context.initialTimestamp / 1000; + performanceEntries = performanceEntries.filter(entry => entry.start >= initialTimestampInSeconds); + } + return Promise.all(createPerformanceSpans(this, performanceEntries)); } From d3847b482eb606c2cb2251748f0f8198f3cfda02 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 18 Oct 2024 12:05:18 +0200 Subject: [PATCH 28/34] perf(node): Truncate breadcrumb messages created by console integration (#14006) --- packages/node/src/integrations/console.ts | 17 +++--- .../node/test/integration/console.test.ts | 60 +++++++++++++++++++ 2 files changed, 67 insertions(+), 10 deletions(-) create mode 100644 packages/node/test/integration/console.test.ts diff --git a/packages/node/src/integrations/console.ts b/packages/node/src/integrations/console.ts index 5cd2eb2ce98a..4a7cea53ec90 100644 --- a/packages/node/src/integrations/console.ts +++ b/packages/node/src/integrations/console.ts @@ -1,11 +1,13 @@ import * as util from 'node:util'; import { addBreadcrumb, defineIntegration, getClient } from '@sentry/core'; -import type { IntegrationFn } from '@sentry/types'; -import { addConsoleInstrumentationHandler, severityLevelFromString } from '@sentry/utils'; +import { addConsoleInstrumentationHandler, severityLevelFromString, truncate } from '@sentry/utils'; const INTEGRATION_NAME = 'Console'; -const _consoleIntegration = (() => { +/** + * Capture console logs as breadcrumbs. + */ +export const consoleIntegration = defineIntegration(() => { return { name: INTEGRATION_NAME, setup(client) { @@ -18,7 +20,7 @@ const _consoleIntegration = (() => { { category: 'console', level: severityLevelFromString(level), - message: util.format.apply(undefined, args), + message: truncate(util.format.apply(undefined, args), 2048), // 2KB }, { input: [...args], @@ -28,9 +30,4 @@ const _consoleIntegration = (() => { }); }, }; -}) satisfies IntegrationFn; - -/** - * Capture console logs as breadcrumbs. - */ -export const consoleIntegration = defineIntegration(_consoleIntegration); +}); diff --git a/packages/node/test/integration/console.test.ts b/packages/node/test/integration/console.test.ts new file mode 100644 index 000000000000..d869959ebdfc --- /dev/null +++ b/packages/node/test/integration/console.test.ts @@ -0,0 +1,60 @@ +import * as SentryCore from '@sentry/core'; +import { resetInstrumentationHandlers } from '@sentry/utils'; +import { getClient } from '../../src'; +import type { NodeClient } from '../../src'; +import { consoleIntegration } from '../../src/integrations/console'; + +const addBreadcrumbSpy = jest.spyOn(SentryCore, 'addBreadcrumb'); + +jest.spyOn(console, 'log').mockImplementation(() => { + // noop so that we don't spam the logs +}); + +afterEach(() => { + jest.clearAllMocks(); + resetInstrumentationHandlers(); +}); + +describe('Console integration', () => { + it('should add a breadcrumb on console.log', () => { + consoleIntegration().setup?.(getClient() as NodeClient); + + // eslint-disable-next-line no-console + console.log('test'); + + expect(addBreadcrumbSpy).toHaveBeenCalledTimes(1); + expect(addBreadcrumbSpy).toHaveBeenCalledWith( + { + category: 'console', + level: 'log', + message: 'test', + }, + { + input: ['test'], + level: 'log', + }, + ); + }); + + it('should truncate breadcrumbs with more than 2 KB message size', () => { + consoleIntegration().setup?.(getClient() as NodeClient); + + const longMsg = 'A'.repeat(10_000); + + // eslint-disable-next-line no-console + console.log(longMsg); + + expect(addBreadcrumbSpy).toHaveBeenCalledTimes(1); + expect(addBreadcrumbSpy).toHaveBeenCalledWith( + { + category: 'console', + level: 'log', + message: `${'A'.repeat(2048)}...`, + }, + { + input: [longMsg], + level: 'log', + }, + ); + }); +}); From 96526c2c5b22725623d47a3ec0b4f489b4b2d36e Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:30:01 +0200 Subject: [PATCH 29/34] docs(nuxt): Add beta readme (#13979) Removes code snippets with `--import` and adds a "Troubleshooting" section. --- packages/nuxt/README.md | 97 +++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 58 deletions(-) diff --git a/packages/nuxt/README.md b/packages/nuxt/README.md index b16d555a3648..abf7f0d7c594 100644 --- a/packages/nuxt/README.md +++ b/packages/nuxt/README.md @@ -4,20 +4,20 @@

-# Official Sentry SDK for Nuxt (EXPERIMENTAL) +# Official Sentry SDK for Nuxt (BETA) [![npm version](https://img.shields.io/npm/v/@sentry/nuxt.svg)](https://www.npmjs.com/package/@sentry/nuxt) [![npm dm](https://img.shields.io/npm/dm/@sentry/nuxt.svg)](https://www.npmjs.com/package/@sentry/nuxt) [![npm dt](https://img.shields.io/npm/dt/@sentry/nuxt.svg)](https://www.npmjs.com/package/@sentry/nuxt) -**This SDK is under active development! Feel free to already try it but expect breaking changes** +This SDK is in **Beta**. The API is stable but updates may include minor changes in behavior. Please reach out on +[GitHub](https://github.com/getsentry/sentry-javascript/issues/new/choose) if you have any feedback or concerns. This +SDK is for [Nuxt](https://nuxt.com/). If you're using [Vue](https://vuejs.org/) see our +[Vue SDK here](https://github.com/getsentry/sentry-javascript/tree/develop/packages/vue). ## Links -todo: link official SDK docs - -- [Official Browser SDK Docs](https://docs.sentry.io/platforms/javascript/) -- [Official Node SDK Docs](https://docs.sentry.io/platforms/node/) +- [Official Nuxt SDK Docs](https://docs.sentry.io/platforms/javascript/guides/nuxt/) ## Compatibility @@ -28,43 +28,12 @@ The minimum supported version of Nuxt is `3.0.0`. This package is a wrapper around `@sentry/node` for the server and `@sentry/vue` for the client side, with added functionality related to Nuxt. -**What is working:** - -- Error Reporting - - Vue - - Node - - Nitro - -**What is partly working:** - -- Source Maps -- Connected Tracing (Frontend & Backend) -- Tracing by setting `tracesSampleRate` - - UI (Vue) traces - - HTTP (Node) traces - -**Known Issues:** - -- When adding `sentry.server.config.(ts/js)`, you get an error like this: - "`Failed to register ESM hook (import-in-the-middle/hook.mjs)`". You can add a resolution for `@vercel/nft` to fix - this. This will add the `hook.mjs` file to your build output - ([issue here](https://github.com/unjs/nitro/issues/2703)). - ```json - "resolutions": { - "@vercel/nft": "^0.27.4" - } - ``` - -## Automatic Setup +**Limitations:** -todo: add wizard instructions - -Take a look at the sections below if you want to customize your SDK configuration. +- Server monitoring is not available during development mode (`nuxt dev`) ## Manual Setup -If the setup through the wizard doesn't work for you, you can also set up the SDK manually. - ### 1. Prerequisites & Installation 1. Install the Sentry Nuxt SDK: @@ -92,7 +61,7 @@ export default defineNuxtConfig({ ### 3. Client-side setup -Add a `sentry.client.config.(js|ts)` file to the root of your project: +Add a `sentry.client.config.ts` file to the root of your project: ```javascript import { useRuntimeConfig } from '#imports'; @@ -106,7 +75,7 @@ Sentry.init({ ### 4. Server-side setup -Add an `sentry.client.config.(js|ts)` file to the root of your project: +Add an `sentry.client.config.ts` file to the root of your project: ```javascript import * as Sentry from '@sentry/nuxt'; @@ -119,11 +88,12 @@ if (process.env.SENTRY_DSN) { } ``` -The Nuxt runtime config does not work in the Sentry server to technical reasons (it has to be loaded before Nuxt is -loaded). To be able to use `process.env` you either have to add `--env-file=.env` to your node command +Using `useRuntimeConfig` does not work in the Sentry server config file due to technical reasons (the file has to be +loaded before Nuxt is loaded). To be able to use `process.env` you either have to add `--env-file=.env` to your node +command ```bash -node --env-file=.env --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs +node --env-file=.env .output/server/index.mjs ``` or use the `dotenv` package: @@ -139,28 +109,18 @@ Sentry.init({ }); ``` -Add an import flag to the Node options of your `node` command (not `nuxt preview`), so the file loads before any other -imports (keep in mind the `.mjs` file ending): - -```json -{ - "scripts": { - "start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs" - } -} -``` - ## Uploading Source Maps -To upload source maps, you can use the `sourceMapsUploadOptions` option inside the `sentry` options of your -`nuxt.config.ts`: +To upload source maps, you have to enable client source maps in your `nuxt.config.ts`. Then, you add your project +settings to the `sentry.sourceMapsUploadOptions` of your `nuxt.config.ts`: ```javascript // nuxt.config.ts export default defineNuxtConfig({ + sourcemap: { client: true }, + modules: ['@sentry/nuxt/module'], sentry: { - debug: true, sourceMapsUploadOptions: { org: 'your-org-slug', project: 'your-project-slug', @@ -169,3 +129,24 @@ export default defineNuxtConfig({ }, }); ``` + +## Troubleshooting + +When adding `sentry.server.config.ts`, you might get an error like this: +"`Failed to register ESM hook import-in-the-middle/hook.mjs`". You can add an override (npm/pnpm) or a resolution (yarn) +for `@vercel/nft` to fix this. This will add the `hook.mjs` file to your build output +([Nitro issue here](https://github.com/unjs/nitro/issues/2703)). + +```json +"overrides": { + "@vercel/nft": "^0.27.4" +} +``` + +or in `yarn`: + +```json +"resolutions": { + "@vercel/nft": "^0.27.4" +} +``` From 54d286b89694f857806e442727b97e6fab52fb48 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:45:15 +0200 Subject: [PATCH 30/34] feat(nuxt): Respect user-provided source map generation settings (#14020) Nuxt implementation for: https://github.com/getsentry/sentry-javascript/issues/13993 Fixes: https://github.com/getsentry/sentry-javascript/issues/13997 In Nuxt, there are 3 places to set source maps (and all need to be enabled): - `sourcemap` - `nitro.rollupConfig.output.sourcemap` - `vite.build.sourcemap` As Nuxt sets `sourcemap.client: false` ([docs here](https://nuxt.com/docs/api/nuxt-config#sourcemap)), it's not possible to determine whether this setting was set by the user or the framework. Users have to set this manually like this: ```js export default defineNuxtConfig({ sourcemap: { client: true, }, }) ``` With this PR, all source maps are set to `'hidden'` if they were undefined before and keep the setting otherwise. This is done in 3 separate functions (one for vite, rollup and nuxt) to better distinguish between the settings. --- packages/nuxt/src/vite/sourceMaps.ts | 235 ++++++++++++++++++--- packages/nuxt/test/vite/sourceMaps.test.ts | 159 +++++++++++++- 2 files changed, 362 insertions(+), 32 deletions(-) diff --git a/packages/nuxt/src/vite/sourceMaps.ts b/packages/nuxt/src/vite/sourceMaps.ts index 9abbfe8eaf08..8fd6f0a381fd 100644 --- a/packages/nuxt/src/vite/sourceMaps.ts +++ b/packages/nuxt/src/vite/sourceMaps.ts @@ -1,9 +1,16 @@ import type { Nuxt } from '@nuxt/schema'; import { type SentryRollupPluginOptions, sentryRollupPlugin } from '@sentry/rollup-plugin'; +import { consoleSandbox } from '@sentry/utils'; import { type SentryVitePluginOptions, sentryVitePlugin } from '@sentry/vite-plugin'; import type { NitroConfig } from 'nitropack'; +import type { OutputOptions } from 'rollup'; import type { SentryNuxtModuleOptions } from '../common/types'; +/** + * Whether the user enabled (true, 'hidden', 'inline') or disabled (false) source maps + */ +export type UserSourceMapSetting = 'enabled' | 'disabled' | 'unset' | undefined; + /** * Setup source maps for Sentry inside the Nuxt module during build time (in Vite for Nuxt and Rollup for Nitro). */ @@ -11,17 +18,21 @@ export function setupSourceMaps(moduleOptions: SentryNuxtModuleOptions, nuxt: Nu const sourceMapsUploadOptions = moduleOptions.sourceMapsUploadOptions || {}; const sourceMapsEnabled = sourceMapsUploadOptions.enabled ?? true; - nuxt.hook('vite:extendConfig', async (viteInlineConfig, _env) => { - if (sourceMapsEnabled && viteInlineConfig.mode !== 'development') { - // Add Sentry plugin - viteInlineConfig.plugins = viteInlineConfig.plugins || []; - viteInlineConfig.plugins.push(sentryVitePlugin(getPluginOptions(moduleOptions))); + nuxt.hook('modules:done', () => { + if (sourceMapsEnabled && !nuxt.options.dev) { + changeNuxtSourceMapSettings(nuxt, moduleOptions); + } + }); - // Enable source maps - viteInlineConfig.build = viteInlineConfig.build || {}; - viteInlineConfig.build.sourcemap = true; + nuxt.hook('vite:extendConfig', async (viteConfig, _env) => { + if (sourceMapsEnabled && viteConfig.mode !== 'development') { + const previousUserSourceMapSetting = changeViteSourceMapSettings(viteConfig, moduleOptions); - logDebugInfo(moduleOptions, viteInlineConfig.build?.sourcemap); + // Add Sentry plugin + viteConfig.plugins = viteConfig.plugins || []; + viteConfig.plugins.push( + sentryVitePlugin(getPluginOptions(moduleOptions, previousUserSourceMapSetting === 'unset')), + ); } }); @@ -38,15 +49,12 @@ export function setupSourceMaps(moduleOptions: SentryNuxtModuleOptions, nuxt: Nu nitroConfig.rollupConfig.plugins = [nitroConfig.rollupConfig.plugins]; } - // Add Sentry plugin - nitroConfig.rollupConfig.plugins.push(sentryRollupPlugin(getPluginOptions(moduleOptions))); - - // Enable source maps - nitroConfig.rollupConfig.output = nitroConfig?.rollupConfig?.output || {}; - nitroConfig.rollupConfig.output.sourcemap = true; - nitroConfig.rollupConfig.output.sourcemapExcludeSources = false; // Adding "sourcesContent" to the source map (Nitro sets this eto `true`) + const previousUserSourceMapSetting = changeRollupSourceMapSettings(nitroConfig, moduleOptions); - logDebugInfo(moduleOptions, nitroConfig.rollupConfig.output?.sourcemap); + // Add Sentry plugin + nitroConfig.rollupConfig.plugins.push( + sentryRollupPlugin(getPluginOptions(moduleOptions, previousUserSourceMapSetting === 'unset')), + ); } }); } @@ -65,9 +73,19 @@ function normalizePath(path: string): string { */ export function getPluginOptions( moduleOptions: SentryNuxtModuleOptions, + deleteFilesAfterUpload: boolean, ): SentryVitePluginOptions | SentryRollupPluginOptions { const sourceMapsUploadOptions = moduleOptions.sourceMapsUploadOptions || {}; + if (typeof sourceMapsUploadOptions.sourcemaps?.filesToDeleteAfterUpload === 'undefined' && deleteFilesAfterUpload) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + '[Sentry] Setting `sentry.sourceMapsUploadOptions.sourcemaps.filesToDeleteAfterUpload: [".*/**/*.map"]` to delete generated source maps after they were uploaded to Sentry.', + ); + }); + } + return { org: sourceMapsUploadOptions.org ?? process.env.SENTRY_ORG, project: sourceMapsUploadOptions.project ?? process.env.SENTRY_PROJECT, @@ -83,29 +101,192 @@ export function getPluginOptions( sourcemaps: { // The server/client files are in different places depending on the nitro preset (e.g. '.output/server' or '.netlify/functions-internal/server') - // We cannot determine automatically how the build folder looks like (depends on the preset), so we have to accept that sourcemaps are uploaded multiple times (with the vitePlugin for Nuxt and the rollupPlugin for Nitro). + // We cannot determine automatically how the build folder looks like (depends on the preset), so we have to accept that source maps are uploaded multiple times (with the vitePlugin for Nuxt and the rollupPlugin for Nitro). // If we could know where the server/client assets are located, we could do something like this (based on the Nitro preset): isNitro ? ['./.output/server/**/*'] : ['./.output/public/**/*'], assets: sourceMapsUploadOptions.sourcemaps?.assets ?? undefined, ignore: sourceMapsUploadOptions.sourcemaps?.ignore ?? undefined, - filesToDeleteAfterUpload: sourceMapsUploadOptions.sourcemaps?.filesToDeleteAfterUpload ?? undefined, + filesToDeleteAfterUpload: sourceMapsUploadOptions.sourcemaps?.filesToDeleteAfterUpload + ? sourceMapsUploadOptions.sourcemaps?.filesToDeleteAfterUpload + : deleteFilesAfterUpload + ? ['.*/**/*.map'] + : undefined, rewriteSources: (source: string) => normalizePath(source), ...moduleOptions?.unstable_sentryBundlerPluginOptions?.sourcemaps, }, }; } -function logDebugInfo(moduleOptions: SentryNuxtModuleOptions, sourceMapsPreviouslyEnabled: boolean): void { - if (moduleOptions.debug && !sourceMapsPreviouslyEnabled) { +/* There are 3 ways to set up source maps (https://github.com/getsentry/sentry-javascript/issues/13993) + 1. User explicitly disabled source maps + - keep this setting (emit a warning that errors won't be unminified in Sentry) + - We will not upload anything + 2. users enabled source map generation (true, hidden, inline). + - keep this setting (don't do anything - like deletion - besides uploading) + 3. users did not set source maps generation + - we enable 'hidden' source maps generation + - configure `filesToDeleteAfterUpload` to delete all .map files (we emit a log about this) + + Nuxt has 3 places to set source maps: vite options, rollup options, nuxt itself + Ideally, all 3 are enabled to get all source maps. + */ + +/** only exported for testing */ +export function changeNuxtSourceMapSettings( + nuxt: Nuxt, + sentryModuleOptions: SentryNuxtModuleOptions, +): { client: UserSourceMapSetting; server: UserSourceMapSetting } { + nuxt.options = nuxt.options || {}; + nuxt.options.sourcemap = nuxt.options.sourcemap ?? { server: undefined, client: undefined }; + + let previousUserSourceMapSetting: { client: UserSourceMapSetting; server: UserSourceMapSetting } = { + client: undefined, + server: undefined, + }; + + const nuxtSourceMap = nuxt.options.sourcemap; + + if (typeof nuxtSourceMap === 'string' || typeof nuxtSourceMap === 'boolean' || typeof nuxtSourceMap === 'undefined') { + switch (nuxtSourceMap) { + case false: + warnExplicitlyDisabledSourceMap('sourcemap'); + previousUserSourceMapSetting = { client: 'disabled', server: 'disabled' }; + break; + + case 'hidden': + case true: + logKeepSourceMapSetting(sentryModuleOptions, 'sourcemap', (nuxtSourceMap as true).toString()); + previousUserSourceMapSetting = { client: 'enabled', server: 'enabled' }; + break; + case undefined: + nuxt.options.sourcemap = { server: 'hidden', client: 'hidden' }; + logSentryEnablesSourceMap('sourcemap.client', 'hidden'); + logSentryEnablesSourceMap('sourcemap.server', 'hidden'); + previousUserSourceMapSetting = { client: 'unset', server: 'unset' }; + break; + } + } else { + if (nuxtSourceMap.client === false) { + warnExplicitlyDisabledSourceMap('sourcemap.client'); + previousUserSourceMapSetting.client = 'disabled'; + } else if (['hidden', true].includes(nuxtSourceMap.client)) { + logKeepSourceMapSetting(sentryModuleOptions, 'sourcemap.client', nuxtSourceMap.client.toString()); + previousUserSourceMapSetting.client = 'enabled'; + } else { + nuxt.options.sourcemap.client = 'hidden'; + logSentryEnablesSourceMap('sourcemap.client', 'hidden'); + previousUserSourceMapSetting.client = 'unset'; + } + + if (nuxtSourceMap.server === false) { + warnExplicitlyDisabledSourceMap('sourcemap.server'); + previousUserSourceMapSetting.server = 'disabled'; + } else if (['hidden', true].includes(nuxtSourceMap.server)) { + logKeepSourceMapSetting(sentryModuleOptions, 'sourcemap.server', nuxtSourceMap.server.toString()); + previousUserSourceMapSetting.server = 'enabled'; + } else { + nuxt.options.sourcemap.server = 'hidden'; + logSentryEnablesSourceMap('sourcemap.server', 'hidden'); + previousUserSourceMapSetting.server = 'unset'; + } + } + + return previousUserSourceMapSetting; +} + +/** only exported for testing */ +export function changeViteSourceMapSettings( + viteConfig: { build?: { sourcemap?: boolean | 'inline' | 'hidden' } }, + sentryModuleOptions: SentryNuxtModuleOptions, +): UserSourceMapSetting { + viteConfig.build = viteConfig.build || {}; + const viteSourceMap = viteConfig.build.sourcemap; + + let previousUserSourceMapSetting: UserSourceMapSetting; + + if (viteSourceMap === false) { + warnExplicitlyDisabledSourceMap('vite.build.sourcemap'); + previousUserSourceMapSetting = 'disabled'; + } else if (viteSourceMap && ['hidden', 'inline', true].includes(viteSourceMap)) { + logKeepSourceMapSetting(sentryModuleOptions, 'vite.build.sourcemap', viteSourceMap.toString()); + previousUserSourceMapSetting = 'enabled'; + } else { + viteConfig.build.sourcemap = 'hidden'; + logSentryEnablesSourceMap('vite.build.sourcemap', 'hidden'); + previousUserSourceMapSetting = 'unset'; + } + + return previousUserSourceMapSetting; +} + +/** only exported for testing */ +export function changeRollupSourceMapSettings( + nitroConfig: { + rollupConfig?: { + output?: { + sourcemap?: OutputOptions['sourcemap']; + sourcemapExcludeSources?: OutputOptions['sourcemapExcludeSources']; + }; + }; + }, + sentryModuleOptions: SentryNuxtModuleOptions, +): UserSourceMapSetting { + nitroConfig.rollupConfig = nitroConfig.rollupConfig || {}; + nitroConfig.rollupConfig.output = nitroConfig.rollupConfig.output || { sourcemap: undefined }; + + let previousUserSourceMapSetting: UserSourceMapSetting; + + const nitroSourceMap = nitroConfig.rollupConfig.output.sourcemap; + + if (nitroSourceMap === false) { + warnExplicitlyDisabledSourceMap('nitro.rollupConfig.output.sourcemap'); + previousUserSourceMapSetting = 'disabled'; + } else if (nitroSourceMap && ['hidden', 'inline', true].includes(nitroSourceMap)) { + logKeepSourceMapSetting(sentryModuleOptions, 'nitro.rollupConfig.output.sourcemap', nitroSourceMap.toString()); + previousUserSourceMapSetting = 'enabled'; + } else { + nitroConfig.rollupConfig.output.sourcemap = 'hidden'; + logSentryEnablesSourceMap('nitro.rollupConfig.output.sourcemap', 'hidden'); + previousUserSourceMapSetting = 'unset'; + } + + nitroConfig.rollupConfig.output.sourcemapExcludeSources = false; + consoleSandbox(() => { // eslint-disable-next-line no-console - console.log('[Sentry]: Enabled source maps generation in the Vite build options.'); + console.log( + '[Sentry] Disabled source map setting in the Nuxt config: `nitro.rollupConfig.output.sourcemapExcludeSources`. Source maps will include the actual code to be able to un-minify code snippets in Sentry.', + ); + }); - const sourceMapsUploadOptions = moduleOptions.sourceMapsUploadOptions || {}; + return previousUserSourceMapSetting; +} - if (!sourceMapsUploadOptions.sourcemaps?.filesToDeleteAfterUpload) { +function logKeepSourceMapSetting( + sentryNuxtModuleOptions: SentryNuxtModuleOptions, + settingKey: string, + settingValue: string, +): void { + if (sentryNuxtModuleOptions.debug) { + consoleSandbox(() => { // eslint-disable-next-line no-console - console.warn( - '[Sentry] We recommend setting the `sourceMapsUploadOptions.sourcemaps.filesToDeleteAfterUpload` option to clean up source maps after uploading. Otherwise, source maps might be deployed to production, depending on your configuration', + console.log( + `[Sentry] We discovered \`${settingKey}\` is set to \`${settingValue}\`. Sentry will keep this source map setting. This will un-minify the code snippet on the Sentry Issue page.`, ); - } + }); } } + +function warnExplicitlyDisabledSourceMap(settingKey: string): void { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + `[Sentry] Parts of source map generation are currently disabled in your Nuxt configuration (\`${settingKey}: false\`). This setting is either a default setting or was explicitly set in your configuration. Sentry won't override this setting. Without source maps, code snippets on the Sentry Issues page will remain minified. To show unminified code, enable source maps in \`${settingKey}\`.`, + ); + }); +} + +function logSentryEnablesSourceMap(settingKey: string, settingValue: string): void { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log(`[Sentry] Enabled source map generation in the build options with \`${settingKey}: ${settingValue}\`.`); + }); +} diff --git a/packages/nuxt/test/vite/sourceMaps.test.ts b/packages/nuxt/test/vite/sourceMaps.test.ts index 34c520b96d83..0c90429fa8d5 100644 --- a/packages/nuxt/test/vite/sourceMaps.test.ts +++ b/packages/nuxt/test/vite/sourceMaps.test.ts @@ -1,6 +1,13 @@ +import type { Nuxt } from '@nuxt/schema'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { SentryNuxtModuleOptions } from '../../src/common/types'; -import { getPluginOptions } from '../../src/vite/sourceMaps'; +import type { UserSourceMapSetting } from '../../src/vite/sourceMaps'; +import { + changeNuxtSourceMapSettings, + changeRollupSourceMapSettings, + changeViteSourceMapSettings, + getPluginOptions, +} from '../../src/vite/sourceMaps'; describe('getPluginOptions', () => { beforeEach(() => { @@ -17,7 +24,7 @@ describe('getPluginOptions', () => { process.env = { ...defaultEnv }; - const options = getPluginOptions({} as SentryNuxtModuleOptions); + const options = getPluginOptions({} as SentryNuxtModuleOptions, false); expect(options).toEqual( expect.objectContaining({ @@ -39,7 +46,7 @@ describe('getPluginOptions', () => { }); it('returns default options when no moduleOptions are provided', () => { - const options = getPluginOptions({} as SentryNuxtModuleOptions); + const options = getPluginOptions({} as SentryNuxtModuleOptions, false); expect(options.org).toBeUndefined(); expect(options.project).toBeUndefined(); @@ -75,7 +82,7 @@ describe('getPluginOptions', () => { }, debug: true, }; - const options = getPluginOptions(customOptions); + const options = getPluginOptions(customOptions, true); expect(options).toEqual( expect.objectContaining({ org: 'custom-org', @@ -119,7 +126,7 @@ describe('getPluginOptions', () => { }, }, }; - const options = getPluginOptions(customOptions); + const options = getPluginOptions(customOptions, false); expect(options).toEqual( expect.objectContaining({ debug: true, @@ -137,3 +144,145 @@ describe('getPluginOptions', () => { ); }); }); + +describe('change sourcemap settings', () => { + describe('changeViteSourcemapSettings', () => { + let viteConfig: { build?: { sourcemap?: boolean | 'inline' | 'hidden' } }; + let sentryModuleOptions: SentryNuxtModuleOptions; + + beforeEach(() => { + viteConfig = {}; + sentryModuleOptions = {}; + }); + + it('should handle viteConfig.build.sourcemap settings', () => { + const cases: { + sourcemap?: boolean | 'hidden' | 'inline'; + expectedSourcemap: boolean | string; + expectedReturn: UserSourceMapSetting; + }[] = [ + { sourcemap: false, expectedSourcemap: false, expectedReturn: 'disabled' }, + { sourcemap: 'hidden', expectedSourcemap: 'hidden', expectedReturn: 'enabled' }, + { sourcemap: 'inline', expectedSourcemap: 'inline', expectedReturn: 'enabled' }, + { sourcemap: true, expectedSourcemap: true, expectedReturn: 'enabled' }, + { sourcemap: undefined, expectedSourcemap: 'hidden', expectedReturn: 'unset' }, + ]; + + cases.forEach(({ sourcemap, expectedSourcemap, expectedReturn }) => { + viteConfig.build = { sourcemap }; + const previousUserSourcemapSetting = changeViteSourceMapSettings(viteConfig, sentryModuleOptions); + expect(viteConfig.build.sourcemap).toBe(expectedSourcemap); + expect(previousUserSourcemapSetting).toBe(expectedReturn); + }); + }); + }); + + describe('changeRollupSourcemapSettings', () => { + let nitroConfig: { + rollupConfig?: { output?: { sourcemap?: boolean | 'hidden' | 'inline'; sourcemapExcludeSources?: boolean } }; + }; + let sentryModuleOptions: SentryNuxtModuleOptions; + + beforeEach(() => { + nitroConfig = {}; + sentryModuleOptions = {}; + }); + + it('should handle nitroConfig.rollupConfig.output.sourcemap settings', () => { + const cases: { + output?: { sourcemap?: boolean | 'hidden' | 'inline' }; + expectedSourcemap: boolean | string; + expectedReturn: UserSourceMapSetting; + }[] = [ + { output: { sourcemap: false }, expectedSourcemap: false, expectedReturn: 'disabled' }, + { output: { sourcemap: 'hidden' }, expectedSourcemap: 'hidden', expectedReturn: 'enabled' }, + { output: { sourcemap: 'inline' }, expectedSourcemap: 'inline', expectedReturn: 'enabled' }, + { output: { sourcemap: true }, expectedSourcemap: true, expectedReturn: 'enabled' }, + { output: { sourcemap: undefined }, expectedSourcemap: 'hidden', expectedReturn: 'unset' }, + { output: undefined, expectedSourcemap: 'hidden', expectedReturn: 'unset' }, + ]; + + cases.forEach(({ output, expectedSourcemap, expectedReturn }) => { + nitroConfig.rollupConfig = { output }; + const previousUserSourceMapSetting = changeRollupSourceMapSettings(nitroConfig, sentryModuleOptions); + expect(nitroConfig.rollupConfig?.output?.sourcemap).toBe(expectedSourcemap); + expect(previousUserSourceMapSetting).toBe(expectedReturn); + expect(nitroConfig.rollupConfig?.output?.sourcemapExcludeSources).toBe(false); + }); + }); + }); + + describe('changeNuxtSourcemapSettings', () => { + let nuxt: { options: { sourcemap: { client: boolean | 'hidden'; server: boolean | 'hidden' } } }; + let sentryModuleOptions: SentryNuxtModuleOptions; + + beforeEach(() => { + // @ts-expect-error - Nuxt types don't accept `undefined` but we want to test this case + nuxt = { options: { sourcemap: { client: undefined } } }; + sentryModuleOptions = {}; + }); + + it('should handle nuxt.options.sourcemap.client settings', () => { + const cases = [ + // { clientSourcemap: false, expectedSourcemap: false, expectedReturn: 'disabled' }, + // { clientSourcemap: 'hidden', expectedSourcemap: 'hidden', expectedReturn: 'enabled' }, + { clientSourcemap: true, expectedSourcemap: true, expectedReturn: 'enabled' }, + { clientSourcemap: undefined, expectedSourcemap: 'hidden', expectedReturn: 'unset' }, + ]; + + cases.forEach(({ clientSourcemap, expectedSourcemap, expectedReturn }) => { + // @ts-expect-error - Nuxt types don't accept `undefined` but we want to test this case + nuxt.options.sourcemap.client = clientSourcemap; + const previousUserSourcemapSetting = changeNuxtSourceMapSettings(nuxt as Nuxt, sentryModuleOptions); + expect(nuxt.options.sourcemap.client).toBe(expectedSourcemap); + expect(previousUserSourcemapSetting.client).toBe(expectedReturn); + }); + }); + + it('should handle nuxt.options.sourcemap.server settings', () => { + const cases = [ + { serverSourcemap: false, expectedSourcemap: false, expectedReturn: 'disabled' }, + { serverSourcemap: 'hidden', expectedSourcemap: 'hidden', expectedReturn: 'enabled' }, + { serverSourcemap: true, expectedSourcemap: true, expectedReturn: 'enabled' }, + { serverSourcemap: undefined, expectedSourcemap: 'hidden', expectedReturn: 'unset' }, + ]; + + cases.forEach(({ serverSourcemap, expectedSourcemap, expectedReturn }) => { + // @ts-expect-error server available + nuxt.options.sourcemap.server = serverSourcemap; + const previousUserSourcemapSetting = changeNuxtSourceMapSettings(nuxt as Nuxt, sentryModuleOptions); + expect(nuxt.options.sourcemap.server).toBe(expectedSourcemap); + expect(previousUserSourcemapSetting.server).toBe(expectedReturn); + }); + }); + + describe('should handle nuxt.options.sourcemap as a boolean', () => { + it('keeps setting of nuxt.options.sourcemap if it is set', () => { + const cases = [ + { sourcemap: false, expectedSourcemap: false, expectedReturn: 'disabled' }, + { sourcemap: true, expectedSourcemap: true, expectedReturn: 'enabled' }, + { sourcemap: 'hidden', expectedSourcemap: 'hidden', expectedReturn: 'enabled' }, + ]; + + cases.forEach(({ sourcemap, expectedSourcemap, expectedReturn }) => { + // @ts-expect-error string type is possible in Nuxt (but type says differently) + nuxt.options.sourcemap = sourcemap; + const previousUserSourcemapSetting = changeNuxtSourceMapSettings(nuxt as Nuxt, sentryModuleOptions); + expect(nuxt.options.sourcemap).toBe(expectedSourcemap); + expect(previousUserSourcemapSetting.client).toBe(expectedReturn); + expect(previousUserSourcemapSetting.server).toBe(expectedReturn); + }); + }); + + it("sets client and server to 'hidden' if nuxt.options.sourcemap not set", () => { + // @ts-expect-error - Nuxt types don't accept `undefined` but we want to test this case + nuxt.options.sourcemap = undefined; + const previousUserSourcemapSetting = changeNuxtSourceMapSettings(nuxt as Nuxt, sentryModuleOptions); + expect(nuxt.options.sourcemap.client).toBe('hidden'); + expect(nuxt.options.sourcemap.server).toBe('hidden'); + expect(previousUserSourcemapSetting.client).toBe('unset'); + expect(previousUserSourcemapSetting.server).toBe('unset'); + }); + }); + }); +}); From ab28544daa9ba2eae1cff41078f25c28e1bdcc4d Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 18 Oct 2024 14:19:14 +0200 Subject: [PATCH 31/34] feat(nuxt): Log server instrumentation might not work in dev (#14021) Sentry is initialized too late in development mode and tracing does not work. This is a note to inform users about that. Will be fixed in the stable version. --- packages/nuxt/src/module.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/nuxt/src/module.ts b/packages/nuxt/src/module.ts index 77bf8edd77de..56fa71ad95a3 100644 --- a/packages/nuxt/src/module.ts +++ b/packages/nuxt/src/module.ts @@ -74,6 +74,15 @@ export default defineNuxtModule({ nuxt.hooks.hook('nitro:init', nitro => { if (serverConfigFile && serverConfigFile.includes('.server.config')) { + if (nitro.options.dev) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + '[Sentry] Your application is running in development mode. Note: @sentry/nuxt is in beta and may not work as expected on the server-side (Nitro). Errors are reported, but tracing does not work.', + ); + }); + } + if (moduleOptions.dynamicImportForServerEntry === false) { addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile); From 2648ef63b71cbdfd03359c188470ae14cd481a4e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 21 Oct 2024 10:22:59 +0200 Subject: [PATCH 32/34] test(e2e): Add event proxy option to allow for event dumps (#13998) --- .eslintrc.js | 1 + .github/workflows/build.yml | 9 ++ .../test-applications/nextjs-15/.gitignore | 1 + .../test-applications/nextjs-15/next-env.d.ts | 2 +- .../nextjs-15/start-event-proxy.mjs | 8 ++ .../test-utils/src/event-proxy-server.ts | 10 ++ ...malize-e2e-test-dump-transaction-events.js | 109 ++++++++++++++++++ 7 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 scripts/normalize-e2e-test-dump-transaction-events.js diff --git a/.eslintrc.js b/.eslintrc.js index a9fb5f421af5..53944e16d9dc 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -21,6 +21,7 @@ module.exports = { 'examples/**', 'test/manual/**', 'types/**', + 'scripts/*.js', ], reportUnusedDisableDirectives: true, overrides: [ diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ce139e9e7787..299ed59cd220 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1035,6 +1035,15 @@ jobs: overwrite: true retention-days: 7 + - name: Upload E2E Test Event Dumps + uses: actions/upload-artifact@v4 + if: always() + with: + name: playwright-event-dumps-job_e2e_playwright_tests-${{ matrix.test-application }} + path: dev-packages/e2e-tests/test-applications/${{ matrix.test-application }}/event-dumps + overwrite: true + retention-days: 7 + - name: Upload test results to Codecov if: cancelled() == false continue-on-error: true diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/.gitignore b/dev-packages/e2e-tests/test-applications/nextjs-15/.gitignore index e799cc33c4e7..ebdbfc025b6a 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-15/.gitignore +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/.gitignore @@ -43,3 +43,4 @@ next-env.d.ts .vscode test-results +event-dumps diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/next-env.d.ts b/dev-packages/e2e-tests/test-applications/nextjs-15/next-env.d.ts index 4f11a03dc6cc..40c3d68096c2 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-15/next-env.d.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/next-env.d.ts @@ -2,4 +2,4 @@ /// // NOTE: This file should not be edited -// see https://nextjs.org/docs/basic-features/typescript for more information. +// see https://nextjs.org/docs/app/building-your-application/configuring/typescript for more information. diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/nextjs-15/start-event-proxy.mjs index 90d736790faa..959b40d253e8 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-15/start-event-proxy.mjs +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/start-event-proxy.mjs @@ -1,6 +1,14 @@ +import * as fs from 'fs'; +import * as path from 'path'; import { startEventProxyServer } from '@sentry-internal/test-utils'; +const packageJson = JSON.parse(fs.readFileSync(path.join(process.cwd(), 'package.json'))); + startEventProxyServer({ port: 3031, proxyServerName: 'nextjs-15', + envelopeDumpPath: path.join( + process.cwd(), + `event-dumps/next-${packageJson.dependencies.next}-${process.env.TEST_ENV}.dump`, + ), }); diff --git a/dev-packages/test-utils/src/event-proxy-server.ts b/dev-packages/test-utils/src/event-proxy-server.ts index 17922a4f90aa..448dd6e34ef0 100644 --- a/dev-packages/test-utils/src/event-proxy-server.ts +++ b/dev-packages/test-utils/src/event-proxy-server.ts @@ -17,6 +17,8 @@ interface EventProxyServerOptions { port: number; /** The name for the proxy server used for referencing it with listener functions */ proxyServerName: string; + /** A path to optionally output all Envelopes to. Can be used to compare event payloads before and after changes. */ + envelopeDumpPath?: string; } interface SentryRequestCallbackData { @@ -167,6 +169,10 @@ export async function startProxyServer( * option to this server (like this `tunnel: http://localhost:${port option}/`). */ export async function startEventProxyServer(options: EventProxyServerOptions): Promise { + if (options.envelopeDumpPath) { + await fs.promises.mkdir(path.dirname(path.resolve(options.envelopeDumpPath)), { recursive: true }); + } + await startProxyServer(options, async (eventCallbackListeners, proxyRequest, proxyRequestBody, eventBuffer) => { const data: SentryRequestCallbackData = { envelope: parseEnvelope(proxyRequestBody), @@ -183,6 +189,10 @@ export async function startEventProxyServer(options: EventProxyServerOptions): P listener(dataString); }); + if (options.envelopeDumpPath) { + fs.appendFileSync(path.resolve(options.envelopeDumpPath), `${JSON.stringify(data.envelope)}\n`, 'utf-8'); + } + return [ 200, '{}', diff --git a/scripts/normalize-e2e-test-dump-transaction-events.js b/scripts/normalize-e2e-test-dump-transaction-events.js new file mode 100644 index 000000000000..ba06a63fa020 --- /dev/null +++ b/scripts/normalize-e2e-test-dump-transaction-events.js @@ -0,0 +1,109 @@ +/* eslint-disable no-console */ + +const fs = require('fs'); +const path = require('path'); + +if (process.argv.length < 4) { + throw new Error('Please provide an input and output file path as an argument.'); +} + +const resolvedInputPath = path.resolve(process.argv[2]); +const resolvedOutputPath = path.resolve(process.argv[3]); + +const fileContents = fs.readFileSync(resolvedInputPath, 'utf8'); + +const transactionNodes = []; + +fileContents.split('\n').forEach(serializedEnvelope => { + let envelope; + try { + envelope = JSON.parse(serializedEnvelope); + } catch (e) { + return; + // noop + } + + const envelopeItems = envelope[1]; + + envelopeItems.forEach(([envelopeItemHeader, transaction]) => { + if (envelopeItemHeader.type === 'transaction') { + const rootNode = { + runtime: transaction.contexts.runtime?.name, + op: transaction.contexts.trace.op, + name: transaction.transaction, + children: [], + }; + + const spanMap = new Map(); + spanMap.set(transaction.contexts.trace.span_id, rootNode); + + transaction.spans.forEach(span => { + const node = { + op: span.data['sentry.op'], + name: span.description, + parent_span_id: span.parent_span_id, + children: [], + }; + spanMap.set(span.span_id, node); + }); + + transaction.spans.forEach(span => { + const node = spanMap.get(span.span_id); + if (node && node.parent_span_id) { + const parentNode = spanMap.get(node.parent_span_id); + parentNode.children.push(node); + } + }); + + transactionNodes.push(rootNode); + } + }); +}); + +const output = transactionNodes + .sort((a, b) => { + const aSerialized = serializeNode(a); + const bSerialized = serializeNode(b); + if (aSerialized < bSerialized) { + return -1; + } else if (aSerialized > bSerialized) { + return 1; + } else { + return 0; + } + }) + .map(node => buildDeterministicStringFromNode(node)) + .join('\n\n-----------------------\n\n'); + +fs.writeFileSync(resolvedOutputPath, output, 'utf-8'); + +// ------- utility fns ---------- + +function buildDeterministicStringFromNode(node, depth = 0) { + const mainParts = []; + if (node.runtime) { + mainParts.push(`(${node.runtime})`); + } + mainParts.push(`${node.op ?? 'default'} -`); + mainParts.push(node.name); + const main = mainParts.join(' '); + const children = node.children + .sort((a, b) => { + const aSerialized = serializeNode(a); + const bSerialized = serializeNode(b); + if (aSerialized < bSerialized) { + return -1; + } else if (aSerialized > bSerialized) { + return 1; + } else { + return 0; + } + }) + .map(child => '\n' + buildDeterministicStringFromNode(child, depth + 1)) + .join(''); + return `${main}${children}`.split('\n').join('\n '); +} + +function serializeNode(node) { + return [node.op, node.name, node.runtime].join('---'); +} From f358790964a55e6b0aa73c9d4b48deb04a666a12 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 21 Oct 2024 11:24:50 +0200 Subject: [PATCH 33/34] test(e2e): Pin deps in Astro Cloudflare E2E test (#14030) --- .../e2e-tests/test-applications/astro-4/package.json | 8 ++++---- .../test-applications/cloudflare-astro/package.json | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/astro-4/package.json b/dev-packages/e2e-tests/test-applications/astro-4/package.json index 5bc5233ef7cc..f20c10f25448 100644 --- a/dev-packages/e2e-tests/test-applications/astro-4/package.json +++ b/dev-packages/e2e-tests/test-applications/astro-4/package.json @@ -12,13 +12,13 @@ "test:assert": "TEST_ENV=production playwright test" }, "dependencies": { - "@astrojs/check": "^0.9.2", - "@astrojs/node": "^8.3.2", + "@astrojs/check": "0.9.2", + "@astrojs/node": "8.3.2", "@playwright/test": "^1.46.0", "@sentry/astro": "* || latest", "@sentry-internal/test-utils": "link:../../../test-utils", - "@spotlightjs/astro": "^2.1.6", - "astro": "^4.13.3", + "@spotlightjs/astro": "2.1.6", + "astro": "4.13.3", "typescript": "^5.5.4" }, "devDependencies": { diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-astro/package.json b/dev-packages/e2e-tests/test-applications/cloudflare-astro/package.json index c8109962ebd0..fd636122d590 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-astro/package.json +++ b/dev-packages/e2e-tests/test-applications/cloudflare-astro/package.json @@ -21,6 +21,9 @@ "@sentry/astro": "latest || *", "astro": "4.16.1" }, + "devDependencies": { + "@astrojs/internal-helpers": "0.4.1" + }, "volta": { "extends": "../../package.json" } From f4ba0391e0ef083ed9b0dd8eca4e56a94932230d Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 18 Oct 2024 14:53:32 +0200 Subject: [PATCH 34/34] meta(changelog): Update changelog for 8.35.0 --- CHANGELOG.md | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b614cc109c2e..4bb9478862cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,102 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 8.35.0 + +### Beta release of the official Nuxt Sentry SDK + +This release marks the beta release of the `@sentry/nuxt` Sentry SDK. For details on how to use it, check out the +[Sentry Nuxt SDK README](https://github.com/getsentry/sentry-javascript/tree/develop/packages/nuxt). Please reach out on +[GitHub](https://github.com/getsentry/sentry-javascript/issues/new/choose) if you have any feedback or concerns. + +- **feat(nuxt): Make dynamic import() wrapping default + ([#13958](https://github.com/getsentry/sentry-javascript/pull/13958))** (BREAKING) +- **feat(nuxt): Add Rollup plugin to wrap server entry with `import()` + ([#13945](https://github.com/getsentry/sentry-javascript/pull/13945))** + +**It is no longer required to add a Node `--import` flag. Please update your start command to avoid initializing Sentry +twice (BREAKING CHANGE).** The SDK will now apply modifications during the build of your application to allow for +patching of libraries during runtime. If run into issues with this change, you can disable this behavior in your +`nuxt.config.ts` and use the `--import` flag instead: + +```js +sentry: { + dynamicImportForServerEntry: false; +} +``` + +- **feat(nuxt): Respect user-provided source map generation settings + ([#14020](https://github.com/getsentry/sentry-javascript/pull/14020))** + +We now require you to explicitly enable sourcemaps for the clientside so that Sentry can un-minify your errors. We made +this change so source maps aren't accidentally leaked to the public. Enable source maps on the client as follows: + +```js +export default defineNuxtConfig({ + sourcemap: { + client: true, + }, +}); +``` + +- feat(nuxt): Log server instrumentation might not work in dev + ([#14021](https://github.com/getsentry/sentry-javascript/pull/14021)) +- feat(nuxt): Add Http `responseHook` with `waitUntil` + ([#13986](https://github.com/getsentry/sentry-javascript/pull/13986)) + +### Important Changes + +- **feat(vue): Add Pinia plugin ([#13841](https://github.com/getsentry/sentry-javascript/pull/13841))** + +Support for [Pinia](https://pinia.vuejs.org/) is added in this release for `@sentry/vue`. To capture Pinia state data, +add `createSentryPiniaPlugin()` to your Pinia store: + +```javascript +import { createPinia } from 'pinia'; +import { createSentryPiniaPlugin } from '@sentry/vue'; + +const pinia = createPinia(); + +pinia.use(createSentryPiniaPlugin()); +``` + +- **feat(node): Implement Sentry-specific http instrumentation + ([#13763](https://github.com/getsentry/sentry-javascript/pull/13763))** + +This change introduces a new `SentryHttpInstrumentation` to handle non-span related HTTP instrumentation, allowing it to +run side-by-side with OTel's `HttpInstrumentation`. This improves support for custom OTel setups and avoids conflicts +with Sentry's instrumentation. Additionally, the `spans: false` option is reintroduced for `httpIntegration` to disable +span emission while still allowing custom `HttpInstrumentation` instances (`httpIntegration({ spans: false })`). + - **feat(core): Make stream instrumentation opt-in ([#13951](https://github.com/getsentry/sentry-javascript/pull/13951))** This change adds a new option `trackFetchStreamPerformance` to the browser tracing integration. Only when set to `true`, Sentry will instrument streams via fetch. +### Other Changes + +- feat(node): Expose `suppressTracing` API ([#13875](https://github.com/getsentry/sentry-javascript/pull/13875)) +- feat(replay): Do not log "timeout while trying to read resp body" as exception + ([#13965](https://github.com/getsentry/sentry-javascript/pull/13965)) +- chore(node): Bump `@opentelemetry/instrumentation-express` to `0.43.0` + ([#13948](https://github.com/getsentry/sentry-javascript/pull/13948)) +- chore(node): Bump `@opentelemetry/instrumentation-fastify` to `0.40.0` + ([#13983](https://github.com/getsentry/sentry-javascript/pull/13983)) +- fix: Ensure type for `init` is correct in meta frameworks + ([#13938](https://github.com/getsentry/sentry-javascript/pull/13938)) +- fix(core): `.set` the `sentry-trace` header instead of `.append`ing in fetch instrumentation + ([#13907](https://github.com/getsentry/sentry-javascript/pull/13907)) +- fix(module): keep version for node ESM package ([#13922](https://github.com/getsentry/sentry-javascript/pull/13922)) +- fix(node): Ensure `ignoreOutgoingRequests` of `httpIntegration` applies to breadcrumbs + ([#13970](https://github.com/getsentry/sentry-javascript/pull/13970)) +- fix(replay): Fix onError sampling when loading an expired buffered session + ([#13962](https://github.com/getsentry/sentry-javascript/pull/13962)) +- fix(replay): Ignore older performance entries when starting manually + ([#13969](https://github.com/getsentry/sentry-javascript/pull/13969)) +- perf(node): Truncate breadcrumb messages created by console integration + ([#14006](https://github.com/getsentry/sentry-javascript/pull/14006)) + Work in this release was contributed by @ZakrepaShe and @zhiyan114. Thank you for your contributions! ## 8.34.0