From 1d87cecda01c19b840010bdac78754bcd9767817 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 11 Oct 2024 11:10:33 +0200 Subject: [PATCH 01/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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 214cdf0c8eba3b4bcda2275c135fd01ecfaeb1af Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 15 Oct 2024 13:29:27 -0400 Subject: [PATCH 18/19] meta: Prepare prelease of 8.35.0-beta.0 --- CHANGELOG.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b614cc109c2e..e39be4ffd0b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 8.35.0-beta.0 + +### Important Changes + - **feat(core): Make stream instrumentation opt-in ([#13951](https://github.com/getsentry/sentry-javascript/pull/13951))** @@ -18,6 +22,25 @@ Sentry will instrument streams via fetch. Work in this release was contributed by @ZakrepaShe and @zhiyan114. Thank you for your contributions! +### Other Changes + +- chore(node): Bump `@opentelemetry/instrumentation-express` to `0.43.0` ([#13948](https://github.com/getsentry/sentry-javascript/pull/13948)) +- ci: Ensure we check correctly for bot users ([#13955](https://github.com/getsentry/sentry-javascript/pull/13955)) +- dev(e2e): Fix nestjs version constraint ([#13964](https://github.com/getsentry/sentry-javascript/pull/13964)) +- feat(node): Expose `suppressTracing` API ([#13875](https://github.com/getsentry/sentry-javascript/pull/13875)) +- feat(node): Implement Sentry-specific http instrumentation ([#13763](https://github.com/getsentry/sentry-javascript/pull/13763)) +- feat(nuxt): Add Rollup plugin to wrap server entry with `import()` ([#13945](https://github.com/getsentry/sentry-javascript/pull/13945)) +- feat(replay): Do not log "timeout while trying to read resp body" as exception ([#13965](https://github.com/getsentry/sentry-javascript/pull/13965)) +- feat(vue): Add Pinia plugin ([#13841](https://github.com/getsentry/sentry-javascript/pull/13841)) +- fix: Ensure type for `init` is correct in meta frameworks ([#13938](https://github.com/getsentry/sentry-javascript/pull/13938)) +- fix(module): keep version for node ESM package ([#13922](https://github.com/getsentry/sentry-javascript/pull/13922)) +- fix(replay): Fix onError sampling when loading an expired buffered session ([#13962](https://github.com/getsentry/sentry-javascript/pull/13962)) +- ref: Add external contributor to CHANGELOG.md ([#13956](https://github.com/getsentry/sentry-javascript/pull/13956)) +- ref: Add external contributor to CHANGELOG.md ([#13959](https://github.com/getsentry/sentry-javascript/pull/13959)) +- test(browser): Add test for current DSC transaction name updating behavior ([#13953](https://github.com/getsentry/sentry-javascript/pull/13953)) +- test(loader): Update Loader Script & test error in `sentryOnLoad` ([#13952](https://github.com/getsentry/sentry-javascript/pull/13952)) +- test(node): Add tests for current DSC transaction name updating behavior ([#13961](https://github.com/getsentry/sentry-javascript/pull/13961)) + ## 8.34.0 ### Important Changes From 97aff96a8127c22d08d71ed52d3e757ea1ac76d5 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 15 Oct 2024 13:29:49 -0400 Subject: [PATCH 19/19] prettier --- CHANGELOG.md | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e39be4ffd0b7..ab7e5ab2ee1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,22 +24,31 @@ Work in this release was contributed by @ZakrepaShe and @zhiyan114. Thank you fo ### Other Changes -- chore(node): Bump `@opentelemetry/instrumentation-express` to `0.43.0` ([#13948](https://github.com/getsentry/sentry-javascript/pull/13948)) +- chore(node): Bump `@opentelemetry/instrumentation-express` to `0.43.0` + ([#13948](https://github.com/getsentry/sentry-javascript/pull/13948)) - ci: Ensure we check correctly for bot users ([#13955](https://github.com/getsentry/sentry-javascript/pull/13955)) - dev(e2e): Fix nestjs version constraint ([#13964](https://github.com/getsentry/sentry-javascript/pull/13964)) - feat(node): Expose `suppressTracing` API ([#13875](https://github.com/getsentry/sentry-javascript/pull/13875)) -- feat(node): Implement Sentry-specific http instrumentation ([#13763](https://github.com/getsentry/sentry-javascript/pull/13763)) -- feat(nuxt): Add Rollup plugin to wrap server entry with `import()` ([#13945](https://github.com/getsentry/sentry-javascript/pull/13945)) -- feat(replay): Do not log "timeout while trying to read resp body" as exception ([#13965](https://github.com/getsentry/sentry-javascript/pull/13965)) +- feat(node): Implement Sentry-specific http instrumentation + ([#13763](https://github.com/getsentry/sentry-javascript/pull/13763)) +- feat(nuxt): Add Rollup plugin to wrap server entry with `import()` + ([#13945](https://github.com/getsentry/sentry-javascript/pull/13945)) +- feat(replay): Do not log "timeout while trying to read resp body" as exception + ([#13965](https://github.com/getsentry/sentry-javascript/pull/13965)) - feat(vue): Add Pinia plugin ([#13841](https://github.com/getsentry/sentry-javascript/pull/13841)) -- fix: Ensure type for `init` is correct in meta frameworks ([#13938](https://github.com/getsentry/sentry-javascript/pull/13938)) +- fix: Ensure type for `init` is correct in meta frameworks + ([#13938](https://github.com/getsentry/sentry-javascript/pull/13938)) - fix(module): keep version for node ESM package ([#13922](https://github.com/getsentry/sentry-javascript/pull/13922)) -- fix(replay): Fix onError sampling when loading an expired buffered session ([#13962](https://github.com/getsentry/sentry-javascript/pull/13962)) +- fix(replay): Fix onError sampling when loading an expired buffered session + ([#13962](https://github.com/getsentry/sentry-javascript/pull/13962)) - ref: Add external contributor to CHANGELOG.md ([#13956](https://github.com/getsentry/sentry-javascript/pull/13956)) - ref: Add external contributor to CHANGELOG.md ([#13959](https://github.com/getsentry/sentry-javascript/pull/13959)) -- test(browser): Add test for current DSC transaction name updating behavior ([#13953](https://github.com/getsentry/sentry-javascript/pull/13953)) -- test(loader): Update Loader Script & test error in `sentryOnLoad` ([#13952](https://github.com/getsentry/sentry-javascript/pull/13952)) -- test(node): Add tests for current DSC transaction name updating behavior ([#13961](https://github.com/getsentry/sentry-javascript/pull/13961)) +- test(browser): Add test for current DSC transaction name updating behavior + ([#13953](https://github.com/getsentry/sentry-javascript/pull/13953)) +- test(loader): Update Loader Script & test error in `sentryOnLoad` + ([#13952](https://github.com/getsentry/sentry-javascript/pull/13952)) +- test(node): Add tests for current DSC transaction name updating behavior + ([#13961](https://github.com/getsentry/sentry-javascript/pull/13961)) ## 8.34.0