From 16996bbb62b5c5dd90bb662f09089a3f06ddf0f0 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Aug 2024 12:47:27 +0200 Subject: [PATCH 01/17] fix(remix): Ensure `origin` is correctly set for remix server spans (#13305) Noticed here: https://github.com/getsentry/sentry-javascript/pull/13282 that we are not correctly setting origin for remix spans. --- .../src/utils/integrations/opentelemetry.ts | 16 ++++++++-------- .../server/instrumentation-otel/loader.test.ts | 6 ++++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/remix/src/utils/integrations/opentelemetry.ts b/packages/remix/src/utils/integrations/opentelemetry.ts index 24648bb8db22..fa1d8fd1b749 100644 --- a/packages/remix/src/utils/integrations/opentelemetry.ts +++ b/packages/remix/src/utils/integrations/opentelemetry.ts @@ -1,7 +1,7 @@ import { RemixInstrumentation } from 'opentelemetry-instrumentation-remix'; -import { defineIntegration } from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_OP, generateInstrumentOnce, getClient, spanToJSON } from '@sentry/node'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration } from '@sentry/core'; +import { generateInstrumentOnce, getClient, spanToJSON } from '@sentry/node'; import type { Client, IntegrationFn, Span } from '@sentry/types'; import type { RemixOptions } from '../remixOptions'; @@ -47,13 +47,13 @@ const addRemixSpanAttributes = (span: Span): void => { // `requestHandler` span from `opentelemetry-instrumentation-remix` is the main server span. // It should be marked as the `http.server` operation. // The incoming requests are skipped by the custom `RemixHttpIntegration` package. - if (type === 'requestHandler') { - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server'); - return; - } - // All other spans are marked as `remix` operations with their specific type [loader, action] - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, `${type}.remix`); + const op = type === 'requestHandler' ? 'http.server' : `${type}.remix`; + + span.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.remix', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op, + }); }; /** diff --git a/packages/remix/test/integration/test/server/instrumentation-otel/loader.test.ts b/packages/remix/test/integration/test/server/instrumentation-otel/loader.test.ts index 49b0fa7665fd..dcf45ed617f1 100644 --- a/packages/remix/test/integration/test/server/instrumentation-otel/loader.test.ts +++ b/packages/remix/test/integration/test/server/instrumentation-otel/loader.test.ts @@ -103,15 +103,17 @@ describe('Remix API Loaders', () => { data: { 'code.function': 'loader', 'sentry.op': 'loader.remix', + 'sentry.origin': 'auto.http.otel.remix', }, - origin: 'manual', + origin: 'auto.http.otel.remix', }, { data: { 'code.function': 'loader', 'sentry.op': 'loader.remix', + 'sentry.origin': 'auto.http.otel.remix', }, - origin: 'manual', + origin: 'auto.http.otel.remix', }, ], }); From 51bbf327f02d2d2e4bb391f49f8285518c25d43e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Aug 2024 13:05:08 +0200 Subject: [PATCH 02/17] fix(opentelemetry): Do not overwrite http span name if kind is internal (#13282) If the kind of a http span is neither client nor server, it implies it is most likely being started with `startSpan()` manually, in which case we rather not want to overwrite the name. --- .../suites/tracing/nestjs/scenario.ts | 5 ++++- .../suites/tracing/nestjs/test.ts | 7 ++++++- .../src/utils/parseSpanDescription.ts | 18 +++++++++++++++--- .../test/utils/parseSpanDescription.test.ts | 19 +++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts index b75ff4d8a9ef..953619d8d437 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts @@ -13,7 +13,7 @@ Sentry.init({ }); import { Controller, Get, Injectable, Module } from '@nestjs/common'; -import { NestFactory } from '@nestjs/core'; +import { BaseExceptionFilter, HttpAdapterHost, NestFactory } from '@nestjs/core'; const port = 3450; @@ -48,6 +48,9 @@ class AppModule {} async function run(): Promise { const app = await NestFactory.create(AppModule); await app.listen(port); + + const { httpAdapter } = app.get(HttpAdapterHost); + Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter)); sendPortToRunner(port); } diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts index 686c93e1cad6..80570044d64d 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts @@ -25,7 +25,12 @@ conditionalTest({ min: 16 })('nestjs auto instrumentation', () => { 'nestjs.callback': 'getHello', 'nestjs.controller': 'AppController', 'nestjs.type': 'request_context', - 'sentry.op': 'http', + 'sentry.op': 'request_context.nestjs', + 'sentry.origin': 'auto.http.otel.nestjs', + component: '@nestjs/core', + 'http.method': 'GET', + 'http.route': '/', + 'http.url': '/', }), }), ]), diff --git a/packages/opentelemetry/src/utils/parseSpanDescription.ts b/packages/opentelemetry/src/utils/parseSpanDescription.ts index 6d1c9936899b..b600b81f8aec 100644 --- a/packages/opentelemetry/src/utils/parseSpanDescription.ts +++ b/packages/opentelemetry/src/utils/parseSpanDescription.ts @@ -14,7 +14,7 @@ import { import type { SpanAttributes, TransactionSource } from '@sentry/types'; import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils'; -import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from '../semanticAttributes'; import type { AbstractSpan } from '../types'; import { getSpanKind } from './getSpanKind'; @@ -163,10 +163,22 @@ export function descriptionForHttpMethod( data['http.fragment'] = fragment; } + // If the span kind is neither client nor server, we use the original name + // this infers that somebody manually started this span, in which case we don't want to overwrite the name + const isClientOrServerKind = kind === SpanKind.CLIENT || kind === SpanKind.SERVER; + + // If the span is an auto-span (=it comes from one of our instrumentations), + // we always want to infer the name + // this is necessary because some of the auto-instrumentation we use uses kind=INTERNAL + const origin = attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] || 'manual'; + const isManualSpan = !`${origin}`.startsWith('auto'); + + const useInferredDescription = isClientOrServerKind || !isManualSpan; + return { op: opParts.join('.'), - description, - source, + description: useInferredDescription ? description : name, + source: useInferredDescription ? source : 'custom', data, }; } diff --git a/packages/opentelemetry/test/utils/parseSpanDescription.test.ts b/packages/opentelemetry/test/utils/parseSpanDescription.test.ts index cfa1a43094c4..2b1d25dbacff 100644 --- a/packages/opentelemetry/test/utils/parseSpanDescription.test.ts +++ b/packages/opentelemetry/test/utils/parseSpanDescription.test.ts @@ -231,6 +231,25 @@ describe('descriptionForHttpMethod', () => { source: 'route', }, ], + [ + 'works with basic client GET with SpanKind.INTERNAL', + 'GET', + { + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_URL]: 'https://www.example.com/my-path', + [SEMATTRS_HTTP_TARGET]: '/my-path', + }, + 'test name', + SpanKind.INTERNAL, + { + op: 'http', + description: 'test name', + data: { + url: 'https://www.example.com/my-path', + }, + source: 'custom', + }, + ], ])('%s', (_, httpMethod, attributes, name, kind, expected) => { const actual = descriptionForHttpMethod({ attributes, kind, name }, httpMethod); expect(actual).toEqual(expected); From 38d968919818ea9646a192a75a8ba2c9509b9aa4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 12 Aug 2024 13:49:23 +0200 Subject: [PATCH 03/17] fix(astro): Only track access request headers in dynamic page requests (#13306) In Astro middleware, we're not allowed to access the `request.headers` object if the incoming request is for a statically generated/prerendered route. Since we accessed these headers previously, users would get a warning as reported multiple times and tracked in https://github.com/getsentry/sentry-javascript/issues/13116. This patch fixes that by checking for static vs dynamic route --- packages/astro/src/server/middleware.ts | 35 ++++- packages/astro/test/server/middleware.test.ts | 148 +++++++++++++----- 2 files changed, 133 insertions(+), 50 deletions(-) diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 4b2f15eb3be4..f69fa113721a 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -84,19 +84,27 @@ async function instrumentRequest( } addNonEnumerableProperty(locals, '__sentry_wrapped__', true); - const { method, headers } = ctx.request; + const isDynamicPageRequest = checkIsDynamicPageRequest(ctx); + + const { method, headers } = isDynamicPageRequest + ? ctx.request + : // headers can only be accessed in dynamic routes. Accessing `ctx.request.headers` in a static route + // will make the server log a warning. + { method: ctx.request.method, headers: undefined }; return continueTrace( { - sentryTrace: headers.get('sentry-trace') || undefined, - baggage: headers.get('baggage'), + sentryTrace: headers?.get('sentry-trace') || undefined, + baggage: headers?.get('baggage'), }, async () => { - // We store this on the current scope, not isolation scope, - // because we may have multiple requests nested inside each other - getCurrentScope().setSDKProcessingMetadata({ request: ctx.request }); + getCurrentScope().setSDKProcessingMetadata({ + // We store the request on the current scope, not isolation scope, + // because we may have multiple requests nested inside each other + request: isDynamicPageRequest ? ctx.request : { method, url: ctx.request.url }, + }); - if (options.trackClientIp) { + if (options.trackClientIp && isDynamicPageRequest) { getCurrentScope().setUser({ ip_address: ctx.clientAddress }); } @@ -277,3 +285,16 @@ function tryDecodeUrl(url: string): string | undefined { return undefined; } } + +/** + * Checks if the incoming request is a request for a dynamic (server-side rendered) page. + * We can check this by looking at the middleware's `clientAddress` context property because accessing + * this prop in a static route will throw an error which we can conveniently catch. + */ +function checkIsDynamicPageRequest(context: Parameters[0]): boolean { + try { + return context.clientAddress != null; + } catch { + return false; + } +} diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index bf96f6ef9046..2bb3886b5dd8 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -149,55 +149,117 @@ describe('sentryMiddleware', () => { }); }); - it('attaches client IP if `trackClientIp=true`', async () => { - const middleware = handleRequest({ trackClientIp: true }); - const ctx = { - request: { - method: 'GET', - url: '/users', - headers: new Headers({ - 'some-header': 'some-value', - }), - }, - clientAddress: '192.168.0.1', - params: {}, - url: new URL('https://myDomain.io/users/'), - }; - const next = vi.fn(() => nextResult); + describe('track client IP address', () => { + it('attaches client IP if `trackClientIp=true` when handling dynamic page requests', async () => { + const middleware = handleRequest({ trackClientIp: true }); + const ctx = { + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + clientAddress: '192.168.0.1', + params: {}, + url: new URL('https://myDomain.io/users/'), + }; + const next = vi.fn(() => nextResult); - // @ts-expect-error, a partial ctx object is fine here - await middleware(ctx, next); + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, next); - expect(setUserMock).toHaveBeenCalledWith({ ip_address: '192.168.0.1' }); + expect(setUserMock).toHaveBeenCalledWith({ ip_address: '192.168.0.1' }); + }); + + it("doesn't attach a client IP if `trackClientIp=true` when handling static page requests", async () => { + const middleware = handleRequest({ trackClientIp: true }); + + const ctx = { + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + get clientAddress() { + throw new Error('clientAddress.get() should not be called in static page requests'); + }, + params: {}, + url: new URL('https://myDomain.io/users/'), + }; + + const next = vi.fn(() => nextResult); + + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, next); + + expect(setUserMock).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledTimes(1); + }); }); - it('attaches request as SDK processing metadata', async () => { - const middleware = handleRequest({}); - const ctx = { - request: { - method: 'GET', - url: '/users', - headers: new Headers({ - 'some-header': 'some-value', - }), - }, - clientAddress: '192.168.0.1', - params: {}, - url: new URL('https://myDomain.io/users/'), - }; - const next = vi.fn(() => nextResult); + describe('request data', () => { + it('attaches request as SDK processing metadata in dynamic page requests', async () => { + const middleware = handleRequest({}); + const ctx = { + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + clientAddress: '192.168.0.1', + params: {}, + url: new URL('https://myDomain.io/users/'), + }; + const next = vi.fn(() => nextResult); - // @ts-expect-error, a partial ctx object is fine here - await middleware(ctx, next); + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, next); - expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({ - request: { - method: 'GET', - url: '/users', - headers: new Headers({ - 'some-header': 'some-value', - }), - }, + expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({ + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + }); + expect(next).toHaveBeenCalledTimes(1); + }); + + it("doesn't attach request headers as processing metadata for static page requests", async () => { + const middleware = handleRequest({}); + const ctx = { + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + get clientAddress() { + throw new Error('clientAddress.get() should not be called in static page requests'); + }, + params: {}, + url: new URL('https://myDomain.io/users/'), + }; + const next = vi.fn(() => nextResult); + + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, next); + + expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({ + request: { + method: 'GET', + url: '/users', + }, + }); + expect(next).toHaveBeenCalledTimes(1); }); }); From 345dd747b25ca161d2825bdb42f0f71d7877c445 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 12 Aug 2024 07:58:03 -0400 Subject: [PATCH 04/17] build: Bump node to 22.5.1 (#13118) Node 22 brings with it: - `node --run` to run scripts (fast) - stable `node --watch` - `glob` in `node:fs` which should allow us to remove a couple of our dependencies and clean up our scripts. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7c737d5a10d6..ebf4021a7a6a 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "yalc:publish": "lerna run yalc:publish" }, "volta": { - "node": "18.20.3", + "node": "22.5.1", "yarn": "1.22.22" }, "workspaces": [ From 4e6c02cc512d32e59693e6cfa5c53dd3887d3691 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 12 Aug 2024 14:52:17 +0200 Subject: [PATCH 05/17] feat(sveltekit): Add bundle size optimizations to plugin options (#13318) Makes it possible to add bundle size optimizations along with source maps options to the SvelteKit plugin options like this: ```js sourceMapsUploadOptions: { authToken: 'token', org: 'org', project: 'project', }, bundleSizeOptimizations: { excludePerformanceMonitoring: true, excludeTracing: true }, ``` A bit of refactoring was done as well in the PR: - exported all types necessary for the plugin from `./types` - create a function `generateVitePluginOptions` which merges all SvelteKit plugin options correctly to create the Vite Plugin options (+ tests for this function) part of https://github.com/getsentry/sentry-javascript/issues/13011 --- packages/astro/src/integration/types.ts | 2 +- packages/sveltekit/package.json | 2 +- .../sveltekit/src/vite/sentryVitePlugins.ts | 224 ++++-------------- packages/sveltekit/src/vite/sourceMaps.ts | 6 +- packages/sveltekit/src/vite/types.ts | 220 +++++++++++++++++ .../test/vite/sentrySvelteKitPlugins.test.ts | 159 ++++++++++++- yarn.lock | 81 +++++++ 7 files changed, 504 insertions(+), 190 deletions(-) create mode 100644 packages/sveltekit/src/vite/types.ts diff --git a/packages/astro/src/integration/types.ts b/packages/astro/src/integration/types.ts index 026fcd01d8c4..8020bcde7c76 100644 --- a/packages/astro/src/integration/types.ts +++ b/packages/astro/src/integration/types.ts @@ -87,7 +87,7 @@ type BundleSizeOptimizationOptions = { /** * If set to true, the plugin will try to tree-shake performance monitoring statements out. * Note that the success of this depends on tree shaking generally being enabled in your build. - * Attention: DO NOT enable this when you're using any performance monitoring-related SDK features (e.g. Sentry.startTransaction()). + * Attention: DO NOT enable this when you're using any performance monitoring-related SDK features (e.g. Sentry.startSpan()). */ excludeTracing?: boolean; diff --git a/packages/sveltekit/package.json b/packages/sveltekit/package.json index 6b65a767a84d..c2b3fab65322 100644 --- a/packages/sveltekit/package.json +++ b/packages/sveltekit/package.json @@ -46,7 +46,7 @@ "@sentry/svelte": "8.25.0", "@sentry/types": "8.25.0", "@sentry/utils": "8.25.0", - "@sentry/vite-plugin": "2.20.1", + "@sentry/vite-plugin": "2.22.0", "magic-string": "0.30.7", "magicast": "0.2.8", "sorcery": "0.11.0" diff --git a/packages/sveltekit/src/vite/sentryVitePlugins.ts b/packages/sveltekit/src/vite/sentryVitePlugins.ts index 83a5cf4e19d6..7482eee7e610 100644 --- a/packages/sveltekit/src/vite/sentryVitePlugins.ts +++ b/packages/sveltekit/src/vite/sentryVitePlugins.ts @@ -1,178 +1,10 @@ +import { dropUndefinedKeys } from '@sentry/utils'; import type { Plugin } from 'vite'; - -import type { SentryVitePluginOptions } from '@sentry/vite-plugin'; import type { AutoInstrumentSelection } from './autoInstrument'; import { makeAutoInstrumentationPlugin } from './autoInstrument'; -import type { SupportedSvelteKitAdapters } from './detectAdapter'; import { detectAdapter } from './detectAdapter'; import { makeCustomSentryVitePlugins } from './sourceMaps'; - -/** - * Options related to source maps upload to Sentry - */ -type SourceMapsUploadOptions = { - /** - * If this flag is `true`, the Sentry plugins will automatically upload source maps to Sentry. - * @default true`. - */ - autoUploadSourceMaps?: boolean; - - /** - * Options for the Sentry Vite plugin to customize and override the release creation and source maps upload process. - * See [Sentry Vite Plugin Options](https://github.com/getsentry/sentry-javascript-bundler-plugins/tree/main/packages/vite-plugin#configuration) for a detailed description. - */ - sourceMapsUploadOptions?: { - /** - * The auth token to use when uploading source maps to Sentry. - * - * Instead of specifying this option, you can also set the `SENTRY_AUTH_TOKEN` environment variable. - * - * To create an auth token, follow this guide: - * @see https://docs.sentry.io/product/accounts/auth-tokens/#organization-auth-tokens - */ - authToken?: string; - - /** - * The organization slug of your Sentry organization. - * Instead of specifying this option, you can also set the `SENTRY_ORG` environment variable. - */ - org?: string; - - /** - * The project slug of your Sentry project. - * Instead of specifying this option, you can also set the `SENTRY_PROJECT` environment variable. - */ - project?: string; - - /** - * If this flag is `true`, the Sentry plugin will collect some telemetry data and send it to Sentry. - * It will not collect any sensitive or user-specific data. - * - * @default true - */ - telemetry?: boolean; - - /** - * Options related to sourcemaps - */ - sourcemaps?: { - /** - * A glob or an array of globs that specify the build artifacts and source maps that will be uploaded to Sentry. - * - * If this option is not specified, sensible defaults based on your adapter and svelte.config.js - * setup will be used. Use this option to override these defaults, for instance if you have a - * customized build setup that diverges from SvelteKit's defaults. - * - * The globbing patterns must follow the implementation of the `glob` package. - * @see https://www.npmjs.com/package/glob#glob-primer - */ - assets?: string | Array; - - /** - * A glob or an array of globs that specifies which build artifacts should not be uploaded to Sentry. - * - * @default [] - By default no files are ignored. Thus, all files matching the `assets` glob - * or the default value for `assets` are uploaded. - * - * The globbing patterns follow the implementation of the glob package. (https://www.npmjs.com/package/glob) - */ - ignore?: string | Array; - - /** - * A glob or an array of globs that specifies the build artifacts that should be deleted after the artifact - * upload to Sentry has been completed. - * - * @default [] - By default no files are deleted. - * - * The globbing patterns follow the implementation of the glob package. (https://www.npmjs.com/package/glob) - */ - filesToDeleteAfterUpload?: string | Array; - }; - - /** - * Options related to managing the Sentry releases for a build. - * - * Note: Managing releases is optional and not required for uploading source maps. - */ - release?: { - /** - * Unique identifier for the release you want to create. - * This value can also be specified via the SENTRY_RELEASE environment variable. - * - * Defaults to automatically detecting a value for your environment. This includes values for Cordova, Heroku, - * AWS CodeBuild, CircleCI, Xcode, and Gradle, and otherwise uses the git HEAD's commit SHA (the latter requires - * access to git CLI and for the root directory to be a valid repository). - * - * If you didn't provide a value and the plugin can't automatically detect one, no release will be created. - */ - name?: string; - - /** - * Whether the plugin should inject release information into the build for the SDK to pick it up when - * sending events. - * - * Defaults to `true`. - */ - inject?: boolean; - }; - - /** - * Options to further customize the Sentry Vite Plugin (@sentry/vite-plugin) behavior directly. - * Options specified in this object take precedence over the options specified in - * the `sourcemaps` and `release` objects. - * - * @see https://www.npmjs.com/package/@sentry/vite-plugin/v/2.14.2#options which lists all available options. - * - * Warning: Options within this object are subject to change at any time. - * We DO NOT guarantee semantic versioning for these options, meaning breaking - * changes can occur at any time within a major SDK version. - * - * Furthermore, some options are untested with SvelteKit specifically. Use with caution. - */ - unstable_sentryVitePluginOptions?: Partial; - }; -}; - -type AutoInstrumentOptions = { - /** - * The Sentry plugin will automatically instrument certain parts of your SvelteKit application at build time. - * Set this option to `false` to disable this behavior or what is instrumentated by passing an object. - * - * Auto instrumentation includes: - * - Universal `load` functions in `+page.(js|ts)` files - * - Server-only `load` functions in `+page.server.(js|ts)` files - * - * @default true (meaning, the plugin will instrument all of the above) - */ - autoInstrument?: boolean | AutoInstrumentSelection; -}; - -export type SentrySvelteKitPluginOptions = { - /** - * If this flag is `true`, the Sentry plugins will log some useful debug information. - * @default false. - */ - debug?: boolean; - - /** - * Specify which SvelteKit adapter you're using. - * By default, the SDK will attempt auto-detect the used adapter at build time and apply the - * correct config for source maps upload or auto-instrumentation. - * - * Currently, the SDK supports the following adapters: - * - node (@sveltejs/adapter-node) - * - auto (@sveltejs/adapter-auto) only Vercel - * - vercel (@sveltejs/adapter-auto) only Serverless functions, no edge runtime - * - * Set this option, if the SDK detects the wrong adapter or you want to use an adapter - * that is not in this list. If you specify 'other', you'll most likely need to configure - * source maps upload yourself. - * - * @default {} the SDK attempts to auto-detect the used adapter at build time - */ - adapter?: SupportedSvelteKitAdapters; -} & SourceMapsUploadOptions & - AutoInstrumentOptions; +import type { CustomSentryVitePluginOptions, SentrySvelteKitPluginOptions } from './types'; const DEFAULT_PLUGIN_OPTIONS: SentrySvelteKitPluginOptions = { autoUploadSourceMaps: true, @@ -211,18 +43,50 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} ); } - if (mergedOptions.autoUploadSourceMaps && process.env.NODE_ENV !== 'development') { + const sentryVitePluginsOptions = generateVitePluginOptions(mergedOptions); + + if (sentryVitePluginsOptions) { + const sentryVitePlugins = await makeCustomSentryVitePlugins(sentryVitePluginsOptions); + + sentryPlugins.push(...sentryVitePlugins); + } + + return sentryPlugins; +} + +/** + * This function creates the options for the custom Sentry Vite plugin. + * The options are derived from the Sentry SvelteKit plugin options, where the `_unstable` options take precedence. + * + * only exported for testing + */ +export function generateVitePluginOptions( + svelteKitPluginOptions: SentrySvelteKitPluginOptions, +): CustomSentryVitePluginOptions | null { + let sentryVitePluginsOptions: CustomSentryVitePluginOptions | null = null; + + // Bundle Size Optimizations + if (svelteKitPluginOptions.bundleSizeOptimizations) { + sentryVitePluginsOptions = { + bundleSizeOptimizations: { + ...svelteKitPluginOptions.bundleSizeOptimizations, + }, + }; + } + + // Source Maps + if (svelteKitPluginOptions.autoUploadSourceMaps && process.env.NODE_ENV !== 'development') { const { unstable_sentryVitePluginOptions, ...sourceMapsUploadOptions } = - mergedOptions.sourceMapsUploadOptions || {}; + svelteKitPluginOptions.sourceMapsUploadOptions || {}; - const sentryVitePluginsOptions = { - ...sourceMapsUploadOptions, + sentryVitePluginsOptions = { + ...(sentryVitePluginsOptions ? sentryVitePluginsOptions : {}), + ...sourceMapsUploadOptions, ...unstable_sentryVitePluginOptions, - - adapter: mergedOptions.adapter, + adapter: svelteKitPluginOptions.adapter, // override the plugin's debug flag with the one from the top-level options - debug: mergedOptions.debug, + debug: svelteKitPluginOptions.debug, }; if (sentryVitePluginsOptions.sourcemaps) { @@ -238,11 +102,7 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} ...unstable_sentryVitePluginOptions?.release, }; } - - const sentryVitePlugins = await makeCustomSentryVitePlugins(sentryVitePluginsOptions); - - sentryPlugins.push(...sentryVitePlugins); } - return sentryPlugins; + return dropUndefinedKeys(sentryVitePluginsOptions); } diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index 7081e09e1c5d..b2ceace40529 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -11,10 +11,10 @@ import type { Plugin } from 'vite'; import MagicString from 'magic-string'; import { WRAPPED_MODULE_SUFFIX } from './autoInstrument'; -import type { SupportedSvelteKitAdapters } from './detectAdapter'; import type { GlobalSentryValues } from './injectGlobalValues'; import { VIRTUAL_GLOBAL_VALUES_FILE, getGlobalValueInjectionCode } from './injectGlobalValues'; import { getAdapterOutputDir, getHooksFileName, loadSvelteConfig } from './svelteConfig'; +import type { CustomSentryVitePluginOptions } from './types'; // sorcery has no types, so these are some basic type definitions: type Chain = { @@ -25,10 +25,6 @@ type Sorcery = { load(filepath: string): Promise; }; -type CustomSentryVitePluginOptions = SentryVitePluginOptions & { - adapter: SupportedSvelteKitAdapters; -}; - // storing this in the module scope because `makeCustomSentryVitePlugin` is called multiple times // and we only want to generate a uuid once in case we have to fall back to it. const releaseName = detectSentryRelease(); diff --git a/packages/sveltekit/src/vite/types.ts b/packages/sveltekit/src/vite/types.ts new file mode 100644 index 000000000000..abd526c1e13a --- /dev/null +++ b/packages/sveltekit/src/vite/types.ts @@ -0,0 +1,220 @@ +import type { SentryVitePluginOptions } from '@sentry/vite-plugin'; +import type { AutoInstrumentSelection } from './autoInstrument'; +import type { SupportedSvelteKitAdapters } from './detectAdapter'; + +/** Options for the Custom Sentry Vite plugin */ +export type CustomSentryVitePluginOptions = SentryVitePluginOptions & { + adapter?: SupportedSvelteKitAdapters; +}; + +/** + * Options for the Sentry Vite plugin to customize and override the release creation and source maps upload process. + * See [Sentry Vite Plugin Options](https://github.com/getsentry/sentry-javascript-bundler-plugins/tree/main/packages/vite-plugin#configuration) for a detailed description. + */ +type SourceMapsUploadOptions = { + /** + * The auth token to use when uploading source maps to Sentry. + * + * Instead of specifying this option, you can also set the `SENTRY_AUTH_TOKEN` environment variable. + * + * To create an auth token, follow this guide: + * @see https://docs.sentry.io/product/accounts/auth-tokens/#organization-auth-tokens + */ + authToken?: string; + + /** + * The organization slug of your Sentry organization. + * Instead of specifying this option, you can also set the `SENTRY_ORG` environment variable. + */ + org?: string; + + /** + * The project slug of your Sentry project. + * Instead of specifying this option, you can also set the `SENTRY_PROJECT` environment variable. + */ + project?: string; + + /** + * If this flag is `true`, the Sentry plugin will collect some telemetry data and send it to Sentry. + * It will not collect any sensitive or user-specific data. + * + * @default true + */ + telemetry?: boolean; + + /** + * Options related to sourcemaps + */ + sourcemaps?: { + /** + * A glob or an array of globs that specify the build artifacts and source maps that will be uploaded to Sentry. + * + * If this option is not specified, sensible defaults based on your adapter and svelte.config.js + * setup will be used. Use this option to override these defaults, for instance if you have a + * customized build setup that diverges from SvelteKit's defaults. + * + * The globbing patterns must follow the implementation of the `glob` package. + * @see https://www.npmjs.com/package/glob#glob-primer + */ + assets?: string | Array; + + /** + * A glob or an array of globs that specifies which build artifacts should not be uploaded to Sentry. + * + * @default [] - By default no files are ignored. Thus, all files matching the `assets` glob + * or the default value for `assets` are uploaded. + * + * The globbing patterns follow the implementation of the glob package. (https://www.npmjs.com/package/glob) + */ + ignore?: string | Array; + + /** + * A glob or an array of globs that specifies the build artifacts that should be deleted after the artifact + * upload to Sentry has been completed. + * + * @default [] - By default no files are deleted. + * + * The globbing patterns follow the implementation of the glob package. (https://www.npmjs.com/package/glob) + */ + filesToDeleteAfterUpload?: string | Array; + }; + + /** + * Options related to managing the Sentry releases for a build. + * + * Note: Managing releases is optional and not required for uploading source maps. + */ + release?: { + /** + * Unique identifier for the release you want to create. + * This value can also be specified via the SENTRY_RELEASE environment variable. + * + * Defaults to automatically detecting a value for your environment. This includes values for Cordova, Heroku, + * AWS CodeBuild, CircleCI, Xcode, and Gradle, and otherwise uses the git HEAD's commit SHA (the latter requires + * access to git CLI and for the root directory to be a valid repository). + * + * If you didn't provide a value and the plugin can't automatically detect one, no release will be created. + */ + name?: string; + + /** + * Whether the plugin should inject release information into the build for the SDK to pick it up when + * sending events. + * + * Defaults to `true`. + */ + inject?: boolean; + }; + /** + * Options to further customize the Sentry Vite Plugin (@sentry/vite-plugin) behavior directly. + * Options specified in this object take precedence over the options specified in + * the `sourcemaps` and `release` objects. + * + * @see https://www.npmjs.com/package/@sentry/vite-plugin/v/2.14.2#options which lists all available options. + * + * Warning: Options within this object are subject to change at any time. + * We DO NOT guarantee semantic versioning for these options, meaning breaking + * changes can occur at any time within a major SDK version. + * + * Furthermore, some options are untested with SvelteKit specifically. Use with caution. + */ + unstable_sentryVitePluginOptions?: Partial; +}; + +type BundleSizeOptimizationOptions = { + /** + * If set to `true`, the plugin will attempt to tree-shake (remove) any debugging code within the Sentry SDK. + * Note that the success of this depends on tree shaking being enabled in your build tooling. + * + * Setting this option to `true` will disable features like the SDK's `debug` option. + */ + excludeDebugStatements?: boolean; + + /** + * If set to true, the plugin will try to tree-shake tracing statements out. + * Note that the success of this depends on tree shaking generally being enabled in your build. + * Attention: DO NOT enable this when you're using any performance monitoring-related SDK features (e.g. Sentry.startSpan()). + */ + excludeTracing?: boolean; + + /** + * If set to `true`, the plugin will attempt to tree-shake (remove) code related to the Sentry SDK's Session Replay Shadow DOM recording functionality. + * Note that the success of this depends on tree shaking being enabled in your build tooling. + * + * This option is safe to be used when you do not want to capture any Shadow DOM activity via Sentry Session Replay. + */ + excludeReplayShadowDom?: boolean; + + /** + * If set to `true`, the plugin will attempt to tree-shake (remove) code related to the Sentry SDK's Session Replay `iframe` recording functionality. + * Note that the success of this depends on tree shaking being enabled in your build tooling. + * + * You can safely do this when you do not want to capture any `iframe` activity via Sentry Session Replay. + */ + excludeReplayIframe?: boolean; + + /** + * If set to `true`, the plugin will attempt to tree-shake (remove) code related to the Sentry SDK's Session Replay's Compression Web Worker. + * Note that the success of this depends on tree shaking being enabled in your build tooling. + * + * **Notice:** You should only do use this option if you manually host a compression worker and configure it in your Sentry Session Replay integration config via the `workerUrl` option. + */ + excludeReplayWorker?: boolean; +}; + +/** Options for the Sentry SvelteKit plugin */ +export type SentrySvelteKitPluginOptions = { + /** + * If this flag is `true`, the Sentry plugins will log some useful debug information. + * @default false. + */ + debug?: boolean; + + /** + * The Sentry plugin will automatically instrument certain parts of your SvelteKit application at build time. + * Set this option to `false` to disable this behavior or what is instrumentated by passing an object. + * + * Auto instrumentation includes: + * - Universal `load` functions in `+page.(js|ts)` files + * - Server-only `load` functions in `+page.server.(js|ts)` files + * + * @default true (meaning, the plugin will instrument all of the above) + */ + autoInstrument?: boolean | AutoInstrumentSelection; + + /** + * Specify which SvelteKit adapter you're using. + * By default, the SDK will attempt auto-detect the used adapter at build time and apply the + * correct config for source maps upload or auto-instrumentation. + * + * Currently, the SDK supports the following adapters: + * - node (@sveltejs/adapter-node) + * - auto (@sveltejs/adapter-auto) only Vercel + * - vercel (@sveltejs/adapter-auto) only Serverless functions, no edge runtime + * + * Set this option, if the SDK detects the wrong adapter or you want to use an adapter + * that is not in this list. If you specify 'other', you'll most likely need to configure + * source maps upload yourself. + * + * @default {} the SDK attempts to auto-detect the used adapter at build time + */ + adapter?: SupportedSvelteKitAdapters; + + /** + * Options for the Sentry Vite plugin to customize bundle size optimizations. + * + * These options are always read from the `sentryAstro` integration. + * Do not define them in the `sentry.client.config.(js|ts)` or `sentry.server.config.(js|ts)` files. + */ + bundleSizeOptimizations?: BundleSizeOptimizationOptions; + + /** + * If this flag is `true`, the Sentry plugins will automatically upload source maps to Sentry. + * @default true`. + */ + autoUploadSourceMaps?: boolean; + /** + * Options related to source maps upload to Sentry + */ + sourceMapsUploadOptions?: SourceMapsUploadOptions; +}; diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index 796b4aa4957b..29dc1b09fb34 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -2,8 +2,9 @@ import { describe, expect, it, vi } from 'vitest'; import type { Plugin } from 'vite'; import * as autoInstrument from '../../src/vite/autoInstrument'; -import { sentrySvelteKit } from '../../src/vite/sentryVitePlugins'; +import { generateVitePluginOptions, sentrySvelteKit } from '../../src/vite/sentryVitePlugins'; import * as sourceMaps from '../../src/vite/sourceMaps'; +import type { CustomSentryVitePluginOptions, SentrySvelteKitPluginOptions } from '../../src/vite/types'; vi.mock('fs', async () => { const actual = await vi.importActual('fs'); @@ -191,3 +192,159 @@ describe('sentrySvelteKit()', () => { }); }); }); + +describe('generateVitePluginOptions', () => { + it('should return null if no relevant options are provided', () => { + const options: SentrySvelteKitPluginOptions = {}; + const result = generateVitePluginOptions(options); + expect(result).toBeNull(); + }); + + it('should use default `debug` value if only default options are provided', () => { + const options: SentrySvelteKitPluginOptions = { autoUploadSourceMaps: true, autoInstrument: true, debug: false }; + const expected: CustomSentryVitePluginOptions = { + debug: false, + }; + const result = generateVitePluginOptions(options); + expect(result).toEqual(expected); + }); + + it('should apply user-defined sourceMapsUploadOptions', () => { + const options: SentrySvelteKitPluginOptions = { + autoUploadSourceMaps: true, + sourceMapsUploadOptions: { + authToken: 'token', + org: 'org', + project: 'project', + sourcemaps: { + assets: ['foo/*.js'], + }, + }, + }; + const expected: CustomSentryVitePluginOptions = { + authToken: 'token', + org: 'org', + project: 'project', + sourcemaps: { + assets: ['foo/*.js'], + }, + }; + const result = generateVitePluginOptions(options); + expect(result).toEqual(expected); + }); + + it('should override options with unstable_sentryVitePluginOptions', () => { + const options: SentrySvelteKitPluginOptions = { + autoUploadSourceMaps: true, + sourceMapsUploadOptions: { + authToken: 'token', + org: 'org', + project: 'project', + sourcemaps: { + assets: ['foo/*.js'], + }, + unstable_sentryVitePluginOptions: { + org: 'unstable-org', + sourcemaps: { + assets: ['unstable/*.js'], + }, + }, + }, + }; + const expected: CustomSentryVitePluginOptions = { + authToken: 'token', + org: 'unstable-org', + project: 'project', + sourcemaps: { + assets: ['unstable/*.js'], + }, + }; + const result = generateVitePluginOptions(options); + expect(result).toEqual(expected); + }); + + it('should merge release options correctly', () => { + const options: SentrySvelteKitPluginOptions = { + autoUploadSourceMaps: true, + sourceMapsUploadOptions: { + release: { + name: '1.0.0', + }, + unstable_sentryVitePluginOptions: { + release: { + name: '2.0.0', + setCommits: { + auto: true, + }, + }, + }, + }, + }; + const expected: CustomSentryVitePluginOptions = { + release: { + name: '2.0.0', + setCommits: { + auto: true, + }, + }, + }; + const result = generateVitePluginOptions(options); + expect(result).toEqual(expected); + }); + + it('should handle adapter and debug options correctly', () => { + const options: SentrySvelteKitPluginOptions = { + autoUploadSourceMaps: true, + adapter: 'vercel', + debug: true, + sourceMapsUploadOptions: { + authToken: 'token', + org: 'org', + project: 'project', + }, + }; + const expected: CustomSentryVitePluginOptions = { + authToken: 'token', + org: 'org', + project: 'project', + adapter: 'vercel', + debug: true, + }; + const result = generateVitePluginOptions(options); + expect(result).toEqual(expected); + }); + + it('should apply bundleSizeOptimizations AND sourceMapsUploadOptions when both are set', () => { + const options: SentrySvelteKitPluginOptions = { + bundleSizeOptimizations: { + excludeTracing: true, + excludeReplayWorker: true, + excludeDebugStatements: false, + }, + autoUploadSourceMaps: true, + sourceMapsUploadOptions: { + authToken: 'token', + org: 'org', + project: 'project', + sourcemaps: { + assets: ['foo/*.js'], + }, + }, + }; + const expected = { + bundleSizeOptimizations: { + excludeTracing: true, + excludeReplayWorker: true, + excludeDebugStatements: false, + }, + authToken: 'token', + org: 'org', + project: 'project', + sourcemaps: { + assets: ['foo/*.js'], + }, + }; + const result = generateVitePluginOptions(options); + expect(result).toEqual(expected); + }); +}); diff --git a/yarn.lock b/yarn.lock index 08a7a3eb7487..5603f142ea95 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8198,6 +8198,11 @@ resolved "https://registry.yarnpkg.com/@sentry/babel-plugin-component-annotate/-/babel-plugin-component-annotate-2.20.1.tgz#204c63ed006a048f48f633876e1b8bacf87a9722" integrity sha512-4mhEwYTK00bIb5Y9UWIELVUfru587Vaeg0DQGswv4aIRHIiMKLyNqCEejaaybQ/fNChIZOKmvyqXk430YVd7Qg== +"@sentry/babel-plugin-component-annotate@2.22.0": + version "2.22.0" + resolved "https://registry.yarnpkg.com/@sentry/babel-plugin-component-annotate/-/babel-plugin-component-annotate-2.22.0.tgz#a7e1cc99d1a738d1eb17757341dff4db3a93c2dc" + integrity sha512-UzH+NNhgnOo6UFku3C4TEz+pO/yDcIA5FKTJvLbJ7lQwAjsqLs3DZWm4cCA08skICb8mULArF6S/dn5/butVCA== + "@sentry/bundler-plugin-core@2.16.0": version "2.16.0" resolved "https://registry.yarnpkg.com/@sentry/bundler-plugin-core/-/bundler-plugin-core-2.16.0.tgz#0c33e7a054fb56e43bd160ac141f71dfebf6dda5" @@ -8240,41 +8245,90 @@ magic-string "0.30.8" unplugin "1.0.1" +"@sentry/bundler-plugin-core@2.22.0": + version "2.22.0" + resolved "https://registry.yarnpkg.com/@sentry/bundler-plugin-core/-/bundler-plugin-core-2.22.0.tgz#6a67761ff5bc0dc897e56acba0b12547bc623e14" + integrity sha512-/xXN8o7565WMsewBnQFfjm0E5wqhYsegg++HJ5RjrY/cTM4qcd/ven44GEMxqGFJitZizvkk3NHszaHylzcRUw== + dependencies: + "@babel/core" "^7.18.5" + "@sentry/babel-plugin-component-annotate" "2.22.0" + "@sentry/cli" "^2.33.1" + dotenv "^16.3.1" + find-up "^5.0.0" + glob "^9.3.2" + magic-string "0.30.8" + unplugin "1.0.1" + "@sentry/cli-darwin@2.33.0": version "2.33.0" resolved "https://registry.yarnpkg.com/@sentry/cli-darwin/-/cli-darwin-2.33.0.tgz#c0f3352a9e58e4f02deca52f0d5a9bd14b3e4a32" integrity sha512-LQFvD7uCOQ2P/vYru7IBKqJDHwJ9Rr2vqqkdjbxe2YCQS/N3NPXvi3eVM9hDJ284oyV/BMZ5lrmVTuIicf/hhw== +"@sentry/cli-darwin@2.33.1": + version "2.33.1" + resolved "https://registry.yarnpkg.com/@sentry/cli-darwin/-/cli-darwin-2.33.1.tgz#e4eb1dd01ee3ce2788025426b860ccc63759589c" + integrity sha512-+4/VIx/E1L2hChj5nGf5MHyEPHUNHJ/HoG5RY+B+vyEutGily1c1+DM2bum7RbD0xs6wKLIyup5F02guzSzG8A== + "@sentry/cli-linux-arm64@2.33.0": version "2.33.0" resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm64/-/cli-linux-arm64-2.33.0.tgz#14bc2556aa1011b96e7964756f84c4215a087ea7" integrity sha512-mR2ZhqpU8RBVGLF5Ji19iOmVznk1B7Bzg5VhA8bVPuKsQmFN/3SyqE87IPMhwKoAsSRXyctwmbAkKs4240fxGA== +"@sentry/cli-linux-arm64@2.33.1": + version "2.33.1" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm64/-/cli-linux-arm64-2.33.1.tgz#9ea1718c21ef32ca83b0852ca29fb461fd26d25a" + integrity sha512-DbGV56PRKOLsAZJX27Jt2uZ11QfQEMmWB4cIvxkKcFVE+LJP4MVA+MGGRUL6p+Bs1R9ZUuGbpKGtj0JiG6CoXw== + "@sentry/cli-linux-arm@2.33.0": version "2.33.0" resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm/-/cli-linux-arm-2.33.0.tgz#e00f9698b6c79e064490a32d11ad7d1909a15314" integrity sha512-gY1bFE7wjDJc7WiNq1AS0WrILqLLJUw6Ou4pFQS45KjaH3/XJ1eohHhGJNy/UBHJ/Gq32b/BA9vsnWTXClZJ7g== +"@sentry/cli-linux-arm@2.33.1": + version "2.33.1" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm/-/cli-linux-arm-2.33.1.tgz#e8a1dca4d008dd6a72ab5935304c104e98e2901c" + integrity sha512-zbxEvQju+tgNvzTOt635le4kS/Fbm2XC2RtYbCTs034Vb8xjrAxLnK0z1bQnStUV8BkeBHtsNVrG+NSQDym2wg== + "@sentry/cli-linux-i686@2.33.0": version "2.33.0" resolved "https://registry.yarnpkg.com/@sentry/cli-linux-i686/-/cli-linux-i686-2.33.0.tgz#f2475caa9897067f25114aa368e6b3ac11c86652" integrity sha512-XPIy0XpqgAposHtWsy58qsX85QnZ8q0ktBuT4skrsCrLMzfhoQg4Ua+YbUr3RvE814Rt8Hzowx2ar2Rl3pyCyw== +"@sentry/cli-linux-i686@2.33.1": + version "2.33.1" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-i686/-/cli-linux-i686-2.33.1.tgz#f1fe8dd4d6dde0812a94fba31de8054ddfb7284a" + integrity sha512-g2LS4oPXkPWOfKWukKzYp4FnXVRRSwBxhuQ9eSw2peeb58ZIObr4YKGOA/8HJRGkooBJIKGaAR2mH2Pk1TKaiA== + "@sentry/cli-linux-x64@2.33.0": version "2.33.0" resolved "https://registry.yarnpkg.com/@sentry/cli-linux-x64/-/cli-linux-x64-2.33.0.tgz#181936a6f37dd237a2f867c11244b26e2d58d5fa" integrity sha512-qe1DdCUv4tmqS03s8RtCkEX9vCW2G+NgOxX6jZ5jN/sKDwjUlquljqo7JHUGSupkoXmymnNPm5By3rNr6VyNHg== +"@sentry/cli-linux-x64@2.33.1": + version "2.33.1" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-x64/-/cli-linux-x64-2.33.1.tgz#6e086675356a9eb79731bf9e447d078bae1b5adf" + integrity sha512-IV3dcYV/ZcvO+VGu9U6kuxSdbsV2kzxaBwWUQxtzxJ+cOa7J8Hn1t0koKGtU53JVZNBa06qJWIcqgl4/pCuKIg== + "@sentry/cli-win32-i686@2.33.0": version "2.33.0" resolved "https://registry.yarnpkg.com/@sentry/cli-win32-i686/-/cli-win32-i686-2.33.0.tgz#3ab02ea0ef159a801701d41e0a16f52d4e751cdb" integrity sha512-VEXWtJ69C3b+kuSmXQJRwdQ0ypPGH88hpqyQuosbAOIqh/sv4g9B/u1ETHZc+whLdFDpPcTLVMbLDbXTGug0Yg== +"@sentry/cli-win32-i686@2.33.1": + version "2.33.1" + resolved "https://registry.yarnpkg.com/@sentry/cli-win32-i686/-/cli-win32-i686-2.33.1.tgz#0e6b36c4a2f5f6e85a59247a123d276b3ef10f1a" + integrity sha512-F7cJySvkpzIu7fnLKNHYwBzZYYwlhoDbAUnaFX0UZCN+5DNp/5LwTp37a5TWOsmCaHMZT4i9IO4SIsnNw16/zQ== + "@sentry/cli-win32-x64@2.33.0": version "2.33.0" resolved "https://registry.yarnpkg.com/@sentry/cli-win32-x64/-/cli-win32-x64-2.33.0.tgz#fc9ec9b7cbec80d7cd39aaa570b7682399a0b1de" integrity sha512-GIUKysZ1xbSklY9h1aVaLMSYLsnMSd+JuwQLR+0wKw2wJC4O5kNCPFSGikhiOZM/kvh3GO1WnXNyazFp8nLAzw== +"@sentry/cli-win32-x64@2.33.1": + version "2.33.1" + resolved "https://registry.yarnpkg.com/@sentry/cli-win32-x64/-/cli-win32-x64-2.33.1.tgz#2d00b38a2dd9f3355df91825582ada3ea0034e86" + integrity sha512-8VyRoJqtb2uQ8/bFRKNuACYZt7r+Xx0k2wXRGTyH05lCjAiVIXn7DiS2BxHFty7M1QEWUCMNsb/UC/x/Cu2wuA== + "@sentry/cli@^2.22.3", "@sentry/cli@^2.33.0": version "2.33.0" resolved "https://registry.yarnpkg.com/@sentry/cli/-/cli-2.33.0.tgz#5de59f829070ab20d360fae25924f39c55afd8ba" @@ -8294,6 +8348,25 @@ "@sentry/cli-win32-i686" "2.33.0" "@sentry/cli-win32-x64" "2.33.0" +"@sentry/cli@^2.33.1": + version "2.33.1" + resolved "https://registry.yarnpkg.com/@sentry/cli/-/cli-2.33.1.tgz#cfbdffdd896b05b92a659baf435b5607037af928" + integrity sha512-dUlZ4EFh98VFRPJ+f6OW3JEYQ7VvqGNMa0AMcmvk07ePNeK/GicAWmSQE4ZfJTTl80ul6HZw1kY01fGQOQlVRA== + dependencies: + https-proxy-agent "^5.0.0" + node-fetch "^2.6.7" + progress "^2.0.3" + proxy-from-env "^1.1.0" + which "^2.0.2" + optionalDependencies: + "@sentry/cli-darwin" "2.33.1" + "@sentry/cli-linux-arm" "2.33.1" + "@sentry/cli-linux-arm64" "2.33.1" + "@sentry/cli-linux-i686" "2.33.1" + "@sentry/cli-linux-x64" "2.33.1" + "@sentry/cli-win32-i686" "2.33.1" + "@sentry/cli-win32-x64" "2.33.1" + "@sentry/vite-plugin@2.19.0": version "2.19.0" resolved "https://registry.yarnpkg.com/@sentry/vite-plugin/-/vite-plugin-2.19.0.tgz#c7938fb13eee15036963b87d7b12c4fc851e488b" @@ -8310,6 +8383,14 @@ "@sentry/bundler-plugin-core" "2.20.1" unplugin "1.0.1" +"@sentry/vite-plugin@2.22.0": + version "2.22.0" + resolved "https://registry.yarnpkg.com/@sentry/vite-plugin/-/vite-plugin-2.22.0.tgz#09743ac390cf8c1609f2fa6d5424548d0b6f7928" + integrity sha512-U1dWldo3gb1oDqERgiSM7zexMwAuqiXO/YUO3xVSpWmhoHz2AqxOcfIX1SygW02NF7Ss3ay4qMAta8PbvdsrnQ== + dependencies: + "@sentry/bundler-plugin-core" "2.22.0" + unplugin "1.0.1" + "@sentry/webpack-plugin@2.16.0": version "2.16.0" resolved "https://registry.yarnpkg.com/@sentry/webpack-plugin/-/webpack-plugin-2.16.0.tgz#4764577edb10c9575a8b4ce03135493f995f56b9" From bd874575a81a8f41ec657ebcf7544b64d0417d1a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Aug 2024 14:54:09 +0200 Subject: [PATCH 06/17] docs: Remove alpha message for `@sentry/opentelemetry` (#13320) Noticed that we still called this out as alpha, even though it is not. --- .github/workflows/build.yml | 4 ++-- packages/opentelemetry/README.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8ab03a313253..d19c648e39b1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -818,10 +818,10 @@ jobs: pattern: profiling-node-binaries-${{ github.sha }}-* path: ${{ github.workspace }}/packages/profiling-node/lib/ merge-multiple: true + # End rebuild profiling - - name: Build Profiling tarball + - name: Build tarballs run: yarn build:tarball - # End rebuild profiling - name: Stores tarballs in cache uses: actions/cache/save@v4 diff --git a/packages/opentelemetry/README.md b/packages/opentelemetry/README.md index 3a3058746701..bc4266c85ce0 100644 --- a/packages/opentelemetry/README.md +++ b/packages/opentelemetry/README.md @@ -12,8 +12,8 @@ This package allows you to send your OpenTelemetry trace data to Sentry via OpenTelemetry SpanProcessors. -This SDK is **considered experimental and in an alpha state**. It may experience breaking changes. Please reach out on -[GitHub](https://github.com/getsentry/sentry-javascript/issues/new/choose) if you have any feedback/concerns. +If you are using `@sentry/node`, OpenTelemetry support is included out of the box. This package is only necessary if you +are setting up OpenTelemetry support for Sentry yourself. ## Installation From 2c24a33d473b1824a4e9d40ec286a2667079bda4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 12 Aug 2024 15:00:40 +0200 Subject: [PATCH 07/17] fix(astro): Correctly extract request data (#13315) builds on top of #13306, found while working on #13116 This PR ensures that we correctly extract the request data in our Astro middleware. Previously we didn't convert the `request.headers` object into a `Record` but simply passed a `Headers` instance. This caused problems with the `requestDataIntegration` which doesn't handle the instance correctly. --- packages/astro/src/server/middleware.ts | 17 ++++++++++++----- packages/astro/test/server/middleware.test.ts | 4 ++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index f69fa113721a..828b2c3b58f5 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -12,7 +12,12 @@ import { withIsolationScope, } from '@sentry/node'; import type { Client, Scope, Span, SpanAttributes } from '@sentry/types'; -import { addNonEnumerableProperty, objectify, stripUrlQueryAndFragment } from '@sentry/utils'; +import { + addNonEnumerableProperty, + objectify, + stripUrlQueryAndFragment, + winterCGRequestToRequestData, +} from '@sentry/utils'; import type { APIContext, MiddlewareResponseHandler } from 'astro'; type MiddlewareOptions = { @@ -86,11 +91,13 @@ async function instrumentRequest( const isDynamicPageRequest = checkIsDynamicPageRequest(ctx); + const request = ctx.request; + const { method, headers } = isDynamicPageRequest - ? ctx.request - : // headers can only be accessed in dynamic routes. Accessing `ctx.request.headers` in a static route + ? request + : // headers can only be accessed in dynamic routes. Accessing `request.headers` in a static route // will make the server log a warning. - { method: ctx.request.method, headers: undefined }; + { method: request.method, headers: undefined }; return continueTrace( { @@ -101,7 +108,7 @@ async function instrumentRequest( getCurrentScope().setSDKProcessingMetadata({ // We store the request on the current scope, not isolation scope, // because we may have multiple requests nested inside each other - request: isDynamicPageRequest ? ctx.request : { method, url: ctx.request.url }, + request: isDynamicPageRequest ? winterCGRequestToRequestData(request) : { method, url: request.url }, }); if (options.trackClientIp && isDynamicPageRequest) { diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index 2bb3886b5dd8..093b2fad2d6b 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -224,9 +224,9 @@ describe('sentryMiddleware', () => { request: { method: 'GET', url: '/users', - headers: new Headers({ + headers: { 'some-header': 'some-value', - }), + }, }, }); expect(next).toHaveBeenCalledTimes(1); From 69a5e8e1829e63f9f148334937f45c43b234fb7a Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 12 Aug 2024 15:03:33 +0100 Subject: [PATCH 08/17] feat(browser): Add spotlightBrowser integration (#13263) Adds a browser-side integration for sending events and Sentry requests to Spotlight. The integration is not enabled by default but can be added by users if they want to explicitly send browser SDK events to spotlight. This is especially helpful if people use spotlight in the electron app or a standalone browser window instead of the overlay. --- Co-authored-by: Burak Yigit Kaya --- .size-limit.js | 2 +- .vscode/settings.json | 5 +- .../src/integrations-bundle/index.debug.ts | 1 + .../browser/src/integrations/spotlight.ts | 91 +++++++++++++++++++ packages/core/src/baseclient.ts | 10 +- packages/core/src/integration.ts | 2 +- packages/node/src/integrations/spotlight.ts | 6 +- packages/node/src/sdk/index.ts | 34 +++---- 8 files changed, 122 insertions(+), 29 deletions(-) create mode 100644 packages/browser/src/integrations/spotlight.ts diff --git a/.size-limit.js b/.size-limit.js index 437e466a89e1..6369aa49e3e9 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -193,7 +193,7 @@ module.exports = [ import: createImport('init'), ignore: ['next/router', 'next/constants'], gzip: true, - limit: '38 KB', + limit: '38.03 KB', }, // SvelteKit SDK (ESM) { diff --git a/.vscode/settings.json b/.vscode/settings.json index 1a8f9ce92cfc..615ca5b24472 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -36,10 +36,11 @@ ], "deno.enablePaths": ["packages/deno/test"], "editor.codeActionsOnSave": { - "source.organizeImports.biome": "explicit", + "source.organizeImports.biome": "explicit" }, "editor.defaultFormatter": "biomejs.biome", "[typescript]": { "editor.defaultFormatter": "biomejs.biome" - } + }, + "cSpell.words": ["arrayify"] } diff --git a/packages/browser/src/integrations-bundle/index.debug.ts b/packages/browser/src/integrations-bundle/index.debug.ts index 39e8920e381f..c6da394f3a13 100644 --- a/packages/browser/src/integrations-bundle/index.debug.ts +++ b/packages/browser/src/integrations-bundle/index.debug.ts @@ -1 +1,2 @@ export { debugIntegration } from '@sentry/core'; +export { spotlightBrowserIntegration } from '../integrations/spotlight'; diff --git a/packages/browser/src/integrations/spotlight.ts b/packages/browser/src/integrations/spotlight.ts new file mode 100644 index 000000000000..75ed18e7f34d --- /dev/null +++ b/packages/browser/src/integrations/spotlight.ts @@ -0,0 +1,91 @@ +import { getNativeImplementation } from '@sentry-internal/browser-utils'; +import { defineIntegration } from '@sentry/core'; +import type { Client, Envelope, Event, IntegrationFn } from '@sentry/types'; +import { logger, serializeEnvelope } from '@sentry/utils'; +import type { WINDOW } from '../helpers'; + +import { DEBUG_BUILD } from '../debug-build'; + +export type SpotlightConnectionOptions = { + /** + * Set this if the Spotlight Sidecar is not running on localhost:8969 + * By default, the Url is set to http://localhost:8969/stream + */ + sidecarUrl?: string; +}; + +export const INTEGRATION_NAME = 'SpotlightBrowser'; + +const _spotlightIntegration = ((options: Partial = {}) => { + const sidecarUrl = options.sidecarUrl || 'http://localhost:8969/stream'; + + return { + name: INTEGRATION_NAME, + setup: () => { + DEBUG_BUILD && logger.log('Using Sidecar URL', sidecarUrl); + }, + // We don't want to send interaction transactions/root spans created from + // clicks within Spotlight to Sentry. Neither do we want them to be sent to + // spotlight. + processEvent: event => (isSpotlightInteraction(event) ? null : event), + afterAllSetup: (client: Client) => { + setupSidecarForwarding(client, sidecarUrl); + }, + }; +}) satisfies IntegrationFn; + +function setupSidecarForwarding(client: Client, sidecarUrl: string): void { + const makeFetch: typeof WINDOW.fetch | undefined = getNativeImplementation('fetch'); + let failCount = 0; + + client.on('beforeEnvelope', (envelope: Envelope) => { + if (failCount > 3) { + logger.warn('[Spotlight] Disabled Sentry -> Spotlight integration due to too many failed requests:', failCount); + return; + } + + makeFetch(sidecarUrl, { + method: 'POST', + body: serializeEnvelope(envelope), + headers: { + 'Content-Type': 'application/x-sentry-envelope', + }, + mode: 'cors', + }).then( + res => { + if (res.status >= 200 && res.status < 400) { + // Reset failed requests counter on success + failCount = 0; + } + }, + err => { + failCount++; + logger.error( + "Sentry SDK can't connect to Sidecar is it running? See: https://spotlightjs.com/sidecar/npx/", + err, + ); + }, + ); + }); +} + +/** + * Use this integration to send errors and transactions to Spotlight. + * + * Learn more about spotlight at https://spotlightjs.com + */ +export const spotlightBrowserIntegration = defineIntegration(_spotlightIntegration); + +/** + * Flags if the event is a transaction created from an interaction with the spotlight UI. + */ +export function isSpotlightInteraction(event: Event): boolean { + return Boolean( + event.type === 'transaction' && + event.spans && + event.contexts && + event.contexts.trace && + event.contexts.trace.op === 'ui.action.click' && + event.spans.some(({ description }) => description && description.includes('#sentry-spotlight')), + ); +} diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 64410360e51d..c7a26f45ab70 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -311,7 +311,15 @@ export abstract class BaseClient implements Client { /** @inheritdoc */ public init(): void { - if (this._isEnabled()) { + if ( + this._isEnabled() || + // Force integrations to be setup even if no DSN was set when we have + // Spotlight enabled. This is particularly important for browser as we + // don't support the `spotlight` option there and rely on the users + // adding the `spotlightBrowserIntegration()` to their integrations which + // wouldn't get initialized with the check below when there's no DSN set. + this._options.integrations.some(({ name }) => name.startsWith('Spotlight')) + ) { this._setupIntegrations(); } } diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 80a539bbe3d7..500b717c3487 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -19,7 +19,7 @@ export type IntegrationIndex = { /** * Remove duplicates from the given array, preferring the last instance of any duplicate. Not guaranteed to - * preseve the order of integrations in the array. + * preserve the order of integrations in the array. * * @private */ diff --git a/packages/node/src/integrations/spotlight.ts b/packages/node/src/integrations/spotlight.ts index bfb9559958f9..1021827312be 100644 --- a/packages/node/src/integrations/spotlight.ts +++ b/packages/node/src/integrations/spotlight.ts @@ -11,7 +11,7 @@ type SpotlightConnectionOptions = { sidecarUrl?: string; }; -const INTEGRATION_NAME = 'Spotlight'; +export const INTEGRATION_NAME = 'Spotlight'; const _spotlightIntegration = ((options: Partial = {}) => { const _options = { @@ -66,6 +66,10 @@ function connectToSpotlight(client: Client, options: Required { + if (res.statusCode && res.statusCode >= 200 && res.statusCode < 400) { + // Reset failed requests counter on success + failedRequests = 0; + } res.on('data', () => { // Drain socket }); diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index cab3ac8274d1..1a20458802a0 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -17,7 +17,7 @@ import { setOpenTelemetryContextAsyncContextStrategy, setupEventContextTrace, } from '@sentry/opentelemetry'; -import type { Client, Integration, Options } from '@sentry/types'; +import type { Integration, Options } from '@sentry/types'; import { consoleSandbox, dropUndefinedKeys, @@ -36,7 +36,7 @@ import { modulesIntegration } from '../integrations/modules'; import { nativeNodeFetchIntegration } from '../integrations/node-fetch'; import { onUncaughtExceptionIntegration } from '../integrations/onuncaughtexception'; import { onUnhandledRejectionIntegration } from '../integrations/onunhandledrejection'; -import { spotlightIntegration } from '../integrations/spotlight'; +import { INTEGRATION_NAME as SPOTLIGHT_INTEGRATION_NAME, spotlightIntegration } from '../integrations/spotlight'; import { getAutoPerformanceIntegrations } from '../integrations/tracing'; import { makeNodeTransport } from '../transports'; import type { NodeClientOptions, NodeOptions } from '../types'; @@ -140,13 +140,19 @@ function _init( const scope = getCurrentScope(); scope.update(options.initialScope); + if (options.spotlight && !options.integrations.some(({ name }) => name === SPOTLIGHT_INTEGRATION_NAME)) { + options.integrations.push( + spotlightIntegration({ + sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined, + }), + ); + } + const client = new NodeClient(options); // The client is on the current scope, from where it generally is inherited getCurrentScope().setClient(client); - if (isEnabled(client)) { - client.init(); - } + client.init(); logger.log(`Running in ${isCjs() ? 'CommonJS' : 'ESM'} mode.`); @@ -158,20 +164,6 @@ function _init( updateScopeFromEnvVariables(); - if (options.spotlight) { - // force integrations to be setup even if no DSN was set - // If they have already been added before, they will be ignored anyhow - const integrations = client.getOptions().integrations; - for (const integration of integrations) { - client.addIntegration(integration); - } - client.addIntegration( - spotlightIntegration({ - sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined, - }), - ); - } - // If users opt-out of this, they _have_ to set up OpenTelemetry themselves // There is no way to use this SDK without OpenTelemetry! if (!options.skipOpenTelemetrySetup) { @@ -336,7 +328,3 @@ function startSessionTracking(): void { } }); } - -function isEnabled(client: Client): boolean { - return client.getOptions().enabled !== false && client.getTransport() !== undefined; -} From 0654dd0ba18790b166f4b4186fb64e1ecaf3abe4 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 12 Aug 2024 17:03:31 +0200 Subject: [PATCH 09/17] feat(nuxt): Set transaction name for server error (#13292) Setting the transaction name to display the API route where the error happened: ![image](https://github.com/user-attachments/assets/5954fc45-e710-44ab-b29a-f90e6bd480a4) --- .../test-applications/nuxt-3/app.vue | 4 ++ .../nuxt-3/pages/fetch-server-error.vue | 11 +++++ .../nuxt-3/pages/test-param/[param].vue | 11 +++++ .../nuxt-3/server/api/param-error/[param].ts | 3 ++ .../nuxt-3/server/api/server-error.ts | 3 ++ .../nuxt-3/server/api/test-param/[param].ts | 5 +++ .../nuxt-3/server/tsconfig.json | 3 ++ .../nuxt-3/tests/errors.server.test.ts | 40 +++++++++++++++++++ .../nuxt/src/runtime/plugins/sentry.server.ts | 13 +++++- 9 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nuxt-3/pages/fetch-server-error.vue create mode 100644 dev-packages/e2e-tests/test-applications/nuxt-3/server/api/param-error/[param].ts create mode 100644 dev-packages/e2e-tests/test-applications/nuxt-3/server/api/server-error.ts create mode 100644 dev-packages/e2e-tests/test-applications/nuxt-3/server/api/test-param/[param].ts create mode 100644 dev-packages/e2e-tests/test-applications/nuxt-3/server/tsconfig.json create mode 100644 dev-packages/e2e-tests/test-applications/nuxt-3/tests/errors.server.test.ts diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/app.vue b/dev-packages/e2e-tests/test-applications/nuxt-3/app.vue index 06f3020220dd..4e7954ceb4af 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/app.vue +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/app.vue @@ -3,6 +3,8 @@
@@ -11,3 +13,5 @@ + diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/pages/fetch-server-error.vue b/dev-packages/e2e-tests/test-applications/nuxt-3/pages/fetch-server-error.vue new file mode 100644 index 000000000000..4643f045582e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/pages/fetch-server-error.vue @@ -0,0 +1,11 @@ + + + \ No newline at end of file diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/pages/test-param/[param].vue b/dev-packages/e2e-tests/test-applications/nuxt-3/pages/test-param/[param].vue index a9bb6177cb15..4b2b7e35a83e 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/pages/test-param/[param].vue +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/pages/test-param/[param].vue @@ -1,4 +1,15 @@ + + diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/server/api/param-error/[param].ts b/dev-packages/e2e-tests/test-applications/nuxt-3/server/api/param-error/[param].ts new file mode 100644 index 000000000000..3fa894e0896a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/server/api/param-error/[param].ts @@ -0,0 +1,3 @@ +export default defineEventHandler(_e => { + throw new Error('Nuxt 3 Param Server error'); +}); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/server/api/server-error.ts b/dev-packages/e2e-tests/test-applications/nuxt-3/server/api/server-error.ts new file mode 100644 index 000000000000..f8533bfab1e5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/server/api/server-error.ts @@ -0,0 +1,3 @@ +export default defineEventHandler(event => { + throw new Error('Nuxt 3 Server error'); +}); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/server/api/test-param/[param].ts b/dev-packages/e2e-tests/test-applications/nuxt-3/server/api/test-param/[param].ts new file mode 100644 index 000000000000..6e4674ee21a9 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/server/api/test-param/[param].ts @@ -0,0 +1,5 @@ +export default defineEventHandler(event => { + const param = getRouterParam(event, 'param'); + + return `Param: ${param}!`; +}); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/server/tsconfig.json b/dev-packages/e2e-tests/test-applications/nuxt-3/server/tsconfig.json new file mode 100644 index 000000000000..b9ed69c19eaf --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/server/tsconfig.json @@ -0,0 +1,3 @@ +{ + "extends": "../.nuxt/tsconfig.server.json" +} diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/tests/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/nuxt-3/tests/errors.server.test.ts new file mode 100644 index 000000000000..e9445d4c2382 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/tests/errors.server.test.ts @@ -0,0 +1,40 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test.describe('server-side errors', async () => { + test('captures api fetch error (fetched on click)', async ({ page }) => { + const errorPromise = waitForError('nuxt-3', async errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Nuxt 3 Server error'; + }); + + await page.goto(`/fetch-server-error`); + await page.getByText('Fetch Server Data').click(); + + const error = await errorPromise; + + expect(error.transaction).toEqual('GET /api/server-error'); + + const exception = error.exception.values[0]; + expect(exception.type).toEqual('Error'); + expect(exception.value).toEqual('Nuxt 3 Server error'); + expect(exception.mechanism.handled).toBe(false); + }); + + test('captures api fetch error (fetched on click) with parametrized route', async ({ page }) => { + const errorPromise = waitForError('nuxt-3', async errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Nuxt 3 Param Server error'; + }); + + await page.goto(`/test-param/1234`); + await page.getByText('Fetch Server Data').click(); + + const error = await errorPromise; + + expect(error.transaction).toEqual('GET /api/param-error/1234'); + + const exception = error.exception.values[0]; + expect(exception.type).toEqual('Error'); + expect(exception.value).toEqual('Nuxt 3 Param Server error'); + expect(exception.mechanism.handled).toBe(false); + }); +}); diff --git a/packages/nuxt/src/runtime/plugins/sentry.server.ts b/packages/nuxt/src/runtime/plugins/sentry.server.ts index 476037ac980b..1159a6d427ff 100644 --- a/packages/nuxt/src/runtime/plugins/sentry.server.ts +++ b/packages/nuxt/src/runtime/plugins/sentry.server.ts @@ -1,4 +1,4 @@ -import { captureException } from '@sentry/node'; +import * as Sentry from '@sentry/node'; import { H3Error } from 'h3'; import { defineNitroPlugin } from 'nitropack/runtime'; import type { NuxtRenderHTMLContext } from 'nuxt/app'; @@ -14,9 +14,18 @@ export default defineNitroPlugin(nitroApp => { } } + const { method, path } = { + method: errorContext.event && errorContext.event._method ? errorContext.event._method : '', + path: errorContext.event && errorContext.event._path ? errorContext.event._path : null, + }; + + if (path) { + Sentry.getCurrentScope().setTransactionName(`${method} ${path}`); + } + const structuredContext = extractErrorContext(errorContext); - captureException(error, { + Sentry.captureException(error, { captureContext: { contexts: { nuxt: structuredContext } }, mechanism: { handled: false }, }); From 5aef4a00a2a2982063e22f12f3e7a3ef26674e25 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 12 Aug 2024 18:07:29 +0200 Subject: [PATCH 10/17] feat(core): Add OpenTelemetry-specific `getTraceData` implementation (#13281) Add an Otel-specific implementation of `getTraceData` and add the `getTraceData` function to the `AsyncContextStrategy` interface. This allows us to dynamically choose either the default implementation (which works correctly for browser/non-POTEL SDKs) and the Otel-specific version. --- .../suites/tracing/meta-tags-twp/server.js | 32 +++++ .../suites/tracing/meta-tags-twp/test.ts | 31 +++++ packages/astro/src/server/middleware.ts | 9 +- packages/core/src/asyncContext/types.ts | 4 + packages/core/src/index.ts | 2 +- packages/core/src/utils/meta.ts | 5 +- packages/core/src/utils/traceData.ts | 37 +++--- .../core/test/lib/utils/traceData.test.ts | 124 +++++++++--------- .../opentelemetry/src/asyncContextStrategy.ts | 4 +- .../opentelemetry/src/utils/getTraceData.ts | 22 ++++ packages/types/src/index.ts | 2 +- packages/types/src/tracing.ts | 9 ++ 12 files changed, 192 insertions(+), 89 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts create mode 100644 packages/opentelemetry/src/utils/getTraceData.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js new file mode 100644 index 000000000000..4dded9cd0ef6 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js @@ -0,0 +1,32 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.get('/test', (_req, res) => { + res.send({ + response: ` + + + ${Sentry.getTraceMetaTags()} + + + Hi :) + + + `, + }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts new file mode 100644 index 000000000000..f3179beede6d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts @@ -0,0 +1,31 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('getTraceMetaTags', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('injects sentry tracing tags without sampled flag for Tracing Without Performance', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + + const response = await runner.makeRequest('get', '/test'); + + // @ts-ignore - response is defined, types just don't reflect it + const html = response?.response as unknown as string; + + const [, traceId, spanId] = html.match(//) || [ + undefined, + undefined, + undefined, + ]; + + expect(traceId).toBeDefined(); + expect(spanId).toBeDefined(); + + const sentryBaggageContent = html.match(//)?.[1]; + + expect(sentryBaggageContent).toContain('sentry-environment=production'); + expect(sentryBaggageContent).toContain('sentry-public_key=public'); + expect(sentryBaggageContent).toContain(`sentry-trace_id=${traceId}`); + }); +}); diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 828b2c3b58f5..3752bd30d448 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -11,7 +11,7 @@ import { startSpan, withIsolationScope, } from '@sentry/node'; -import type { Client, Scope, Span, SpanAttributes } from '@sentry/types'; +import type { Scope, SpanAttributes } from '@sentry/types'; import { addNonEnumerableProperty, objectify, @@ -151,7 +151,6 @@ async function instrumentRequest( setHttpStatus(span, originalResponse.status); } - const scope = getCurrentScope(); const client = getClient(); const contentType = originalResponse.headers.get('content-type'); @@ -175,7 +174,7 @@ async function instrumentRequest( start: async controller => { for await (const chunk of originalBody) { const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); - const modifiedHtml = addMetaTagToHead(html, scope, client, span); + const modifiedHtml = addMetaTagToHead(html); controller.enqueue(new TextEncoder().encode(modifiedHtml)); } controller.close(); @@ -199,11 +198,11 @@ async function instrumentRequest( * This function optimistically assumes that the HTML coming in chunks will not be split * within the tag. If this still happens, we simply won't replace anything. */ -function addMetaTagToHead(htmlChunk: string, scope: Scope, client: Client, span?: Span): string { +function addMetaTagToHead(htmlChunk: string): string { if (typeof htmlChunk !== 'string') { return htmlChunk; } - const metaTags = getTraceMetaTags(span, scope, client); + const metaTags = getTraceMetaTags(); if (!metaTags) { return htmlChunk; diff --git a/packages/core/src/asyncContext/types.ts b/packages/core/src/asyncContext/types.ts index bd69c8e63e78..9fb9f9f4bec8 100644 --- a/packages/core/src/asyncContext/types.ts +++ b/packages/core/src/asyncContext/types.ts @@ -1,4 +1,5 @@ import type { Scope } from '@sentry/types'; +import type { getTraceData } from '../utils/traceData'; import type { startInactiveSpan, startSpan, @@ -64,4 +65,7 @@ export interface AsyncContextStrategy { /** Suppress tracing in the given callback, ensuring no spans are generated inside of it. */ suppressTracing?: typeof suppressTracing; + + /** Get trace data as serialized string values for propagation via `sentry-trace` and `baggage`. */ + getTraceData?: typeof getTraceData; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 73295f7df64c..792bf3572934 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,4 +1,4 @@ -export type { ClientClass } from './sdk'; +export type { ClientClass as SentryCoreCurrentScopes } from './sdk'; export type { AsyncContextStrategy } from './asyncContext/types'; export type { Carrier } from './carrier'; export type { OfflineStore, OfflineTransportOptions } from './transports/offline'; diff --git a/packages/core/src/utils/meta.ts b/packages/core/src/utils/meta.ts index 339dfcee2f28..7db802582eef 100644 --- a/packages/core/src/utils/meta.ts +++ b/packages/core/src/utils/meta.ts @@ -1,4 +1,3 @@ -import type { Client, Scope, Span } from '@sentry/types'; import { getTraceData } from './traceData'; /** @@ -22,8 +21,8 @@ import { getTraceData } from './traceData'; * ``` * */ -export function getTraceMetaTags(span?: Span, scope?: Scope, client?: Client): string { - return Object.entries(getTraceData(span, scope, client)) +export function getTraceMetaTags(): string { + return Object.entries(getTraceData()) .map(([key, value]) => ``) .join('\n'); } diff --git a/packages/core/src/utils/traceData.ts b/packages/core/src/utils/traceData.ts index abc05f449365..831e8187996e 100644 --- a/packages/core/src/utils/traceData.ts +++ b/packages/core/src/utils/traceData.ts @@ -1,19 +1,16 @@ -import type { Client, Scope, Span } from '@sentry/types'; +import type { SerializedTraceData } from '@sentry/types'; import { TRACEPARENT_REGEXP, dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, logger, } from '@sentry/utils'; +import { getAsyncContextStrategy } from '../asyncContext'; +import { getMainCarrier } from '../carrier'; import { getClient, getCurrentScope } from '../currentScopes'; import { getDynamicSamplingContextFromClient, getDynamicSamplingContextFromSpan } from '../tracing'; import { getActiveSpan, getRootSpan, spanToTraceHeader } from './spanUtils'; -type TraceData = { - 'sentry-trace'?: string; - baggage?: string; -}; - /** * Extracts trace propagation data from the current span or from the client's scope (via transaction or propagation * context) and serializes it to `sentry-trace` and `baggage` values to strings. These values can be used to propagate @@ -22,29 +19,31 @@ type TraceData = { * This function also applies some validation to the generated sentry-trace and baggage values to ensure that * only valid strings are returned. * - * @param span a span to take the trace data from. By default, the currently active span is used. - * @param scope the scope to take trace data from By default, the active current scope is used. - * @param client the SDK's client to take trace data from. By default, the current client is used. - * * @returns an object with the tracing data values. The object keys are the name of the tracing key to be used as header * or meta tag name. */ -export function getTraceData(span?: Span, scope?: Scope, client?: Client): TraceData { - const clientToUse = client || getClient(); - const scopeToUse = scope || getCurrentScope(); - const spanToUse = span || getActiveSpan(); +export function getTraceData(): SerializedTraceData { + const carrier = getMainCarrier(); + const acs = getAsyncContextStrategy(carrier); + if (acs.getTraceData) { + return acs.getTraceData(); + } + + const client = getClient(); + const scope = getCurrentScope(); + const span = getActiveSpan(); - const { dsc, sampled, traceId } = scopeToUse.getPropagationContext(); - const rootSpan = spanToUse && getRootSpan(spanToUse); + const { dsc, sampled, traceId } = scope.getPropagationContext(); + const rootSpan = span && getRootSpan(span); - const sentryTrace = spanToUse ? spanToTraceHeader(spanToUse) : generateSentryTraceHeader(traceId, undefined, sampled); + const sentryTrace = span ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, undefined, sampled); const dynamicSamplingContext = rootSpan ? getDynamicSamplingContextFromSpan(rootSpan) : dsc ? dsc - : clientToUse - ? getDynamicSamplingContextFromClient(traceId, clientToUse) + : client + ? getDynamicSamplingContextFromClient(traceId, client) : undefined; const baggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); diff --git a/packages/core/test/lib/utils/traceData.test.ts b/packages/core/test/lib/utils/traceData.test.ts index e757926ca30d..a6fb3c57814e 100644 --- a/packages/core/test/lib/utils/traceData.test.ts +++ b/packages/core/test/lib/utils/traceData.test.ts @@ -1,5 +1,7 @@ import { SentrySpan, getTraceData } from '../../../src/'; +import * as SentryCoreCurrentScopes from '../../../src/currentScopes'; import * as SentryCoreTracing from '../../../src/tracing'; +import * as SentryCoreSpanUtils from '../../../src/utils/spanUtils'; import { isValidBaggageString } from '../../../src/utils/traceData'; @@ -25,10 +27,12 @@ describe('getTraceData', () => { jest.spyOn(SentryCoreTracing, 'getDynamicSamplingContextFromSpan').mockReturnValueOnce({ environment: 'production', }); + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => mockedSpan); + jest.spyOn(SentryCoreCurrentScopes, 'getCurrentScope').mockImplementationOnce(() => mockedScope); - const tags = getTraceData(mockedSpan, mockedScope, mockedClient); + const data = getTraceData(); - expect(tags).toEqual({ + expect(data).toEqual({ 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', baggage: 'sentry-environment=production', }); @@ -36,22 +40,25 @@ describe('getTraceData', () => { }); it('returns propagationContext DSC data if no span is available', () => { - const traceData = getTraceData( - undefined, - { - getPropagationContext: () => ({ - traceId: '12345678901234567890123456789012', - sampled: true, - spanId: '1234567890123456', - dsc: { - environment: 'staging', - public_key: 'key', - trace_id: '12345678901234567890123456789012', - }, - }), - } as any, - mockedClient, + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => undefined); + jest.spyOn(SentryCoreCurrentScopes, 'getCurrentScope').mockImplementationOnce( + () => + ({ + getPropagationContext: () => ({ + traceId: '12345678901234567890123456789012', + sampled: true, + spanId: '1234567890123456', + dsc: { + environment: 'staging', + public_key: 'key', + trace_id: '12345678901234567890123456789012', + }, + }), + }) as any, ); + jest.spyOn(SentryCoreCurrentScopes, 'getClient').mockImplementationOnce(() => mockedClient); + + const traceData = getTraceData(); expect(traceData).toEqual({ 'sentry-trace': expect.stringMatching(/12345678901234567890123456789012-(.{16})-1/), @@ -65,21 +72,22 @@ describe('getTraceData', () => { public_key: undefined, }); - const traceData = getTraceData( - // @ts-expect-error - we don't need to provide all the properties - { - isRecording: () => true, - spanContext: () => { - return { - traceId: '12345678901234567890123456789012', - spanId: '1234567890123456', - traceFlags: TRACE_FLAG_SAMPLED, - }; - }, + // @ts-expect-error - we don't need to provide all the properties + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => ({ + isRecording: () => true, + spanContext: () => { + return { + traceId: '12345678901234567890123456789012', + spanId: '1234567890123456', + traceFlags: TRACE_FLAG_SAMPLED, + }; }, - mockedScope, - mockedClient, - ); + })); + + jest.spyOn(SentryCoreCurrentScopes, 'getCurrentScope').mockImplementationOnce(() => mockedScope); + jest.spyOn(SentryCoreCurrentScopes, 'getClient').mockImplementationOnce(() => mockedClient); + + const traceData = getTraceData(); expect(traceData).toEqual({ 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', @@ -92,21 +100,21 @@ describe('getTraceData', () => { public_key: undefined, }); - const traceData = getTraceData( - // @ts-expect-error - we don't need to provide all the properties - { - isRecording: () => true, - spanContext: () => { - return { - traceId: '12345678901234567890123456789012', - spanId: '1234567890123456', - traceFlags: TRACE_FLAG_SAMPLED, - }; - }, + // @ts-expect-error - we don't need to provide all the properties + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => ({ + isRecording: () => true, + spanContext: () => { + return { + traceId: '12345678901234567890123456789012', + spanId: '1234567890123456', + traceFlags: TRACE_FLAG_SAMPLED, + }; }, - mockedScope, - undefined, - ); + })); + jest.spyOn(SentryCoreCurrentScopes, 'getCurrentScope').mockImplementationOnce(() => mockedScope); + jest.spyOn(SentryCoreCurrentScopes, 'getClient').mockImplementationOnce(() => undefined); + + const traceData = getTraceData(); expect(traceData).toEqual({ 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', @@ -115,21 +123,19 @@ describe('getTraceData', () => { }); it('returns an empty object if the `sentry-trace` value is invalid', () => { - const traceData = getTraceData( - // @ts-expect-error - we don't need to provide all the properties - { - isRecording: () => true, - spanContext: () => { - return { - traceId: '1234567890123456789012345678901+', - spanId: '1234567890123456', - traceFlags: TRACE_FLAG_SAMPLED, - }; - }, + // @ts-expect-error - we don't need to provide all the properties + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => ({ + isRecording: () => true, + spanContext: () => { + return { + traceId: '1234567890123456789012345678901+', + spanId: '1234567890123456', + traceFlags: TRACE_FLAG_SAMPLED, + }; }, - mockedScope, - mockedClient, - ); + })); + + const traceData = getTraceData(); expect(traceData).toEqual({}); }); diff --git a/packages/opentelemetry/src/asyncContextStrategy.ts b/packages/opentelemetry/src/asyncContextStrategy.ts index 69878d27b252..31da9479921f 100644 --- a/packages/opentelemetry/src/asyncContextStrategy.ts +++ b/packages/opentelemetry/src/asyncContextStrategy.ts @@ -12,6 +12,7 @@ import { startInactiveSpan, startSpan, startSpanManual, withActiveSpan } from '. import type { CurrentScopes } from './types'; import { getScopesFromContext } from './utils/contextData'; import { getActiveSpan } from './utils/getActiveSpan'; +import { getTraceData } from './utils/getTraceData'; import { suppressTracing } from './utils/suppressTracing'; /** @@ -102,9 +103,10 @@ export function setOpenTelemetryContextAsyncContextStrategy(): void { startSpanManual, startInactiveSpan, getActiveSpan, + suppressTracing, + getTraceData, // The types here don't fully align, because our own `Span` type is narrower // than the OTEL one - but this is OK for here, as we now we'll only have OTEL spans passed around withActiveSpan: withActiveSpan as typeof defaultWithActiveSpan, - suppressTracing: suppressTracing, }); } diff --git a/packages/opentelemetry/src/utils/getTraceData.ts b/packages/opentelemetry/src/utils/getTraceData.ts new file mode 100644 index 000000000000..d85f6f699ef3 --- /dev/null +++ b/packages/opentelemetry/src/utils/getTraceData.ts @@ -0,0 +1,22 @@ +import * as api from '@opentelemetry/api'; +import type { SerializedTraceData } from '@sentry/types'; +import { dropUndefinedKeys } from '@sentry/utils'; + +/** + * Otel-specific implementation of `getTraceData`. + * @see `@sentry/core` version of `getTraceData` for more information + */ +export function getTraceData(): SerializedTraceData { + const headersObject: Record = {}; + + api.propagation.inject(api.context.active(), headersObject); + + if (!headersObject['sentry-trace']) { + return {}; + } + + return dropUndefinedKeys({ + 'sentry-trace': headersObject['sentry-trace'], + baggage: headersObject.baggage, + }); +} diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 8f7fdce74c33..1022e69ad49e 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -121,7 +121,7 @@ export type { SpanStatus } from './spanStatus'; export type { TimedEvent } from './timedEvent'; export type { StackFrame } from './stackframe'; export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace'; -export type { PropagationContext, TracePropagationTargets } from './tracing'; +export type { PropagationContext, TracePropagationTargets, SerializedTraceData } from './tracing'; export type { StartSpanOptions } from './startSpanOptions'; export type { TraceparentData, diff --git a/packages/types/src/tracing.ts b/packages/types/src/tracing.ts index 701f9930d314..7af40f3507f7 100644 --- a/packages/types/src/tracing.ts +++ b/packages/types/src/tracing.ts @@ -42,3 +42,12 @@ export interface PropagationContext { */ dsc?: Partial; } + +/** + * An object holding trace data, like span and trace ids, sampling decision, and dynamic sampling context + * in a serialized form. Both keys are expected to be used as Http headers or Html meta tags. + */ +export interface SerializedTraceData { + 'sentry-trace'?: string; + baggage?: string; +} From abc7259d384eed3c5fee1dacd950b69e58671cb8 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 12 Aug 2024 19:33:02 -0230 Subject: [PATCH 11/17] feat(replay): Add a replay-specific logger (#13256) Removes the old `logInfo` function and replaces it with a new replay-specific logger. Configuration is done when the replay integration is first initialized to avoid needing to pass around configuration options. This also means that we cannot select individual log statements to be added as breadcrumbs with `traceInternals` options. This also adds a `logger.exception` that wraps `captureException`. Note that only the following logging levels are supported: * `info` * `log` * `warn` * `error` With two additions: * `exception` * `infoTick` (needs a better name) - same as `info` but adds the breadcrumb in the next tick due to some pre-existing race conditions There is one method to configure the logger: * `setConfig({ traceInternals, captureExceptions })` --- .../src/coreHandlers/handleGlobalEvent.ts | 4 +- .../coreHandlers/handleNetworkBreadcrumbs.ts | 4 +- .../src/coreHandlers/util/fetchUtils.ts | 10 +- .../src/coreHandlers/util/networkUtils.ts | 9 +- .../src/coreHandlers/util/xhrUtils.ts | 12 +- .../EventBufferCompressionWorker.ts | 4 +- .../src/eventBuffer/EventBufferProxy.ts | 7 +- .../src/eventBuffer/WorkerHandler.ts | 8 +- .../replay-internal/src/eventBuffer/index.ts | 9 +- packages/replay-internal/src/replay.ts | 78 ++++++------- .../src/session/fetchSession.ts | 7 +- .../src/session/loadOrCreateSession.ts | 11 +- packages/replay-internal/src/util/addEvent.ts | 11 +- .../src/util/handleRecordingEmit.ts | 11 +- packages/replay-internal/src/util/log.ts | 54 --------- packages/replay-internal/src/util/logger.ts | 105 ++++++++++++++++++ .../src/util/sendReplayRequest.ts | 5 +- .../test/integration/flush.test.ts | 12 +- 18 files changed, 198 insertions(+), 163 deletions(-) delete mode 100644 packages/replay-internal/src/util/log.ts create mode 100644 packages/replay-internal/src/util/logger.ts diff --git a/packages/replay-internal/src/coreHandlers/handleGlobalEvent.ts b/packages/replay-internal/src/coreHandlers/handleGlobalEvent.ts index 88651d449fe6..a13f4d24827e 100644 --- a/packages/replay-internal/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay-internal/src/coreHandlers/handleGlobalEvent.ts @@ -1,10 +1,10 @@ import type { Event, EventHint } from '@sentry/types'; -import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; import type { ReplayContainer } from '../types'; import { isErrorEvent, isFeedbackEvent, isReplayEvent, isTransactionEvent } from '../util/eventUtils'; import { isRrwebError } from '../util/isRrwebError'; +import { logger } from '../util/logger'; import { addFeedbackBreadcrumb } from './util/addFeedbackBreadcrumb'; import { shouldSampleForBufferEvent } from './util/shouldSampleForBufferEvent'; @@ -50,7 +50,7 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even // Unless `captureExceptions` is enabled, we want to ignore errors coming from rrweb // As there can be a bunch of stuff going wrong in internals there, that we don't want to bubble up to users if (isRrwebError(event, hint) && !replay.getOptions()._experiments.captureExceptions) { - DEBUG_BUILD && logger.log('[Replay] Ignoring error from rrweb internals', event); + DEBUG_BUILD && logger.log('Ignoring error from rrweb internals', event); return null; } diff --git a/packages/replay-internal/src/coreHandlers/handleNetworkBreadcrumbs.ts b/packages/replay-internal/src/coreHandlers/handleNetworkBreadcrumbs.ts index a31fc046b17a..8b95a1f5fabe 100644 --- a/packages/replay-internal/src/coreHandlers/handleNetworkBreadcrumbs.ts +++ b/packages/replay-internal/src/coreHandlers/handleNetworkBreadcrumbs.ts @@ -1,9 +1,9 @@ import { getClient } from '@sentry/core'; import type { Breadcrumb, BreadcrumbHint, FetchBreadcrumbData, XhrBreadcrumbData } from '@sentry/types'; -import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; import type { FetchHint, ReplayContainer, ReplayNetworkOptions, XhrHint } from '../types'; +import { logger } from '../util/logger'; import { captureFetchBreadcrumbToReplay, enrichFetchBreadcrumb } from './util/fetchUtils'; import { captureXhrBreadcrumbToReplay, enrichXhrBreadcrumb } from './util/xhrUtils'; @@ -79,7 +79,7 @@ export function beforeAddNetworkBreadcrumb( captureFetchBreadcrumbToReplay(breadcrumb, hint, options); } } catch (e) { - DEBUG_BUILD && logger.warn('Error when enriching network breadcrumb'); + DEBUG_BUILD && logger.exception(e, 'Error when enriching network breadcrumb'); } } diff --git a/packages/replay-internal/src/coreHandlers/util/fetchUtils.ts b/packages/replay-internal/src/coreHandlers/util/fetchUtils.ts index b5c2c3c36305..6502206b58b6 100644 --- a/packages/replay-internal/src/coreHandlers/util/fetchUtils.ts +++ b/packages/replay-internal/src/coreHandlers/util/fetchUtils.ts @@ -1,6 +1,5 @@ import { setTimeout } from '@sentry-internal/browser-utils'; import type { Breadcrumb, FetchBreadcrumbData } from '@sentry/types'; -import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../../debug-build'; import type { @@ -11,6 +10,7 @@ import type { ReplayNetworkRequestData, ReplayNetworkRequestOrResponse, } from '../../types'; +import { logger } from '../../util/logger'; import { addNetworkBreadcrumb } from './addNetworkBreadcrumb'; import { buildNetworkRequestOrResponse, @@ -42,7 +42,7 @@ export async function captureFetchBreadcrumbToReplay( const result = makeNetworkReplayBreadcrumb('resource.fetch', data); addNetworkBreadcrumb(options.replay, result); } catch (error) { - DEBUG_BUILD && logger.error('[Replay] Failed to capture fetch breadcrumb', error); + DEBUG_BUILD && logger.exception(error, 'Failed to capture fetch breadcrumb'); } } @@ -192,7 +192,7 @@ function getResponseData( return buildNetworkRequestOrResponse(headers, size, undefined); } catch (error) { - DEBUG_BUILD && logger.warn('[Replay] Failed to serialize response body', error); + DEBUG_BUILD && logger.exception(error, 'Failed to serialize response body'); // fallback return buildNetworkRequestOrResponse(headers, responseBodySize, undefined); } @@ -209,7 +209,7 @@ async function _parseFetchResponseBody(response: Response): Promise<[string | un const text = await _tryGetResponseText(res); return [text]; } catch (error) { - DEBUG_BUILD && logger.warn('[Replay] Failed to get text body from response', error); + DEBUG_BUILD && logger.exception(error, 'Failed to get text body from response'); return [undefined, 'BODY_PARSE_ERROR']; } } @@ -279,7 +279,7 @@ function _tryCloneResponse(response: Response): Response | void { return response.clone(); } catch (error) { // this can throw if the response was already consumed before - DEBUG_BUILD && logger.warn('[Replay] Failed to clone response body', error); + DEBUG_BUILD && logger.exception(error, 'Failed to clone response body'); } } diff --git a/packages/replay-internal/src/coreHandlers/util/networkUtils.ts b/packages/replay-internal/src/coreHandlers/util/networkUtils.ts index 06e96b7ab7df..2267fa502333 100644 --- a/packages/replay-internal/src/coreHandlers/util/networkUtils.ts +++ b/packages/replay-internal/src/coreHandlers/util/networkUtils.ts @@ -1,4 +1,4 @@ -import { dropUndefinedKeys, logger, stringMatchesSomePattern } from '@sentry/utils'; +import { dropUndefinedKeys, stringMatchesSomePattern } from '@sentry/utils'; import { NETWORK_BODY_MAX_SIZE, WINDOW } from '../../constants'; import { DEBUG_BUILD } from '../../debug-build'; @@ -10,6 +10,7 @@ import type { ReplayNetworkRequestOrResponse, ReplayPerformanceEntry, } from '../../types'; +import { logger } from '../../util/logger'; /** Get the size of a body. */ export function getBodySize(body: RequestInit['body']): number | undefined { @@ -77,12 +78,12 @@ export function getBodyString(body: unknown): [string | undefined, NetworkMetaWa if (!body) { return [undefined]; } - } catch { - DEBUG_BUILD && logger.warn('[Replay] Failed to serialize body', body); + } catch (error) { + DEBUG_BUILD && logger.exception(error, 'Failed to serialize body', body); return [undefined, 'BODY_PARSE_ERROR']; } - DEBUG_BUILD && logger.info('[Replay] Skipping network body because of body type', body); + DEBUG_BUILD && logger.info('Skipping network body because of body type', body); return [undefined, 'UNPARSEABLE_BODY_TYPE']; } diff --git a/packages/replay-internal/src/coreHandlers/util/xhrUtils.ts b/packages/replay-internal/src/coreHandlers/util/xhrUtils.ts index b86e2d2991a9..52b6cafdfec7 100644 --- a/packages/replay-internal/src/coreHandlers/util/xhrUtils.ts +++ b/packages/replay-internal/src/coreHandlers/util/xhrUtils.ts @@ -1,6 +1,5 @@ import { SENTRY_XHR_DATA_KEY } from '@sentry-internal/browser-utils'; import type { Breadcrumb, XhrBreadcrumbData } from '@sentry/types'; -import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../../debug-build'; import type { @@ -10,6 +9,7 @@ import type { ReplayNetworkRequestData, XhrHint, } from '../../types'; +import { logger } from '../../util/logger'; import { addNetworkBreadcrumb } from './addNetworkBreadcrumb'; import { buildNetworkRequestOrResponse, @@ -39,7 +39,7 @@ export async function captureXhrBreadcrumbToReplay( const result = makeNetworkReplayBreadcrumb('resource.xhr', data); addNetworkBreadcrumb(options.replay, result); } catch (error) { - DEBUG_BUILD && logger.error('[Replay] Failed to capture xhr breadcrumb', error); + DEBUG_BUILD && logger.exception(error, 'Failed to capture xhr breadcrumb'); } } @@ -161,7 +161,7 @@ function _getXhrResponseBody(xhr: XMLHttpRequest): [string | undefined, NetworkM errors.push(e); } - DEBUG_BUILD && logger.warn('[Replay] Failed to get xhr response body', ...errors); + DEBUG_BUILD && logger.warn('Failed to get xhr response body', ...errors); return [undefined]; } @@ -197,12 +197,12 @@ export function _parseXhrResponse( if (!body) { return [undefined]; } - } catch { - DEBUG_BUILD && logger.warn('[Replay] Failed to serialize body', body); + } catch (error) { + DEBUG_BUILD && logger.exception(error, 'Failed to serialize body', body); return [undefined, 'BODY_PARSE_ERROR']; } - DEBUG_BUILD && logger.info('[Replay] Skipping network body because of body type', body); + DEBUG_BUILD && logger.info('Skipping network body because of body type', body); return [undefined, 'UNPARSEABLE_BODY_TYPE']; } diff --git a/packages/replay-internal/src/eventBuffer/EventBufferCompressionWorker.ts b/packages/replay-internal/src/eventBuffer/EventBufferCompressionWorker.ts index 21206ea652ac..90a54bbf07f3 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferCompressionWorker.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferCompressionWorker.ts @@ -1,9 +1,9 @@ import type { ReplayRecordingData } from '@sentry/types'; -import { logger } from '@sentry/utils'; import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants'; import { DEBUG_BUILD } from '../debug-build'; import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; +import { logger } from '../util/logger'; import { timestampToMs } from '../util/timestamp'; import { WorkerHandler } from './WorkerHandler'; import { EventBufferSizeExceededError } from './error'; @@ -88,7 +88,7 @@ export class EventBufferCompressionWorker implements EventBuffer { // We do not wait on this, as we assume the order of messages is consistent for the worker this._worker.postMessage('clear').then(null, e => { - DEBUG_BUILD && logger.warn('[Replay] Sending "clear" message to worker failed', e); + DEBUG_BUILD && logger.exception(e, 'Sending "clear" message to worker failed', e); }); } diff --git a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts index af6645a89e69..413bb6fb6372 100644 --- a/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay-internal/src/eventBuffer/EventBufferProxy.ts @@ -1,9 +1,8 @@ import type { ReplayRecordingData } from '@sentry/types'; -import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; -import { logInfo } from '../util/log'; +import { logger } from '../util/logger'; import { EventBufferArray } from './EventBufferArray'; import { EventBufferCompressionWorker } from './EventBufferCompressionWorker'; @@ -90,7 +89,7 @@ export class EventBufferProxy implements EventBuffer { } catch (error) { // If the worker fails to load, we fall back to the simple buffer. // Nothing more to do from our side here - logInfo('[Replay] Failed to load the compression worker, falling back to simple buffer'); + DEBUG_BUILD && logger.exception(error, 'Failed to load the compression worker, falling back to simple buffer'); return; } @@ -117,7 +116,7 @@ export class EventBufferProxy implements EventBuffer { try { await Promise.all(addEventPromises); } catch (error) { - DEBUG_BUILD && logger.warn('[Replay] Failed to add events when switching buffers.', error); + DEBUG_BUILD && logger.exception(error, 'Failed to add events when switching buffers.'); } } } diff --git a/packages/replay-internal/src/eventBuffer/WorkerHandler.ts b/packages/replay-internal/src/eventBuffer/WorkerHandler.ts index 1014521e652f..2ccc3ee94b3c 100644 --- a/packages/replay-internal/src/eventBuffer/WorkerHandler.ts +++ b/packages/replay-internal/src/eventBuffer/WorkerHandler.ts @@ -1,8 +1,6 @@ -import { logger } from '@sentry/utils'; - import { DEBUG_BUILD } from '../debug-build'; import type { WorkerRequest, WorkerResponse } from '../types'; -import { logInfo } from '../util/log'; +import { logger } from '../util/logger'; /** * Event buffer that uses a web worker to compress events. @@ -57,7 +55,7 @@ export class WorkerHandler { * Destroy the worker. */ public destroy(): void { - logInfo('[Replay] Destroying compression worker'); + DEBUG_BUILD && logger.info('Destroying compression worker'); this._worker.terminate(); } @@ -85,7 +83,7 @@ export class WorkerHandler { if (!response.success) { // TODO: Do some error handling, not sure what - DEBUG_BUILD && logger.error('[Replay]', response.response); + DEBUG_BUILD && logger.error('Error in compression worker: ', response.response); reject(new Error('Error in compression worker')); return; diff --git a/packages/replay-internal/src/eventBuffer/index.ts b/packages/replay-internal/src/eventBuffer/index.ts index 741cb5dedc91..bc000da5db7e 100644 --- a/packages/replay-internal/src/eventBuffer/index.ts +++ b/packages/replay-internal/src/eventBuffer/index.ts @@ -1,7 +1,8 @@ import { getWorkerURL } from '@sentry-internal/replay-worker'; +import { DEBUG_BUILD } from '../debug-build'; import type { EventBuffer } from '../types'; -import { logInfo } from '../util/log'; +import { logger } from '../util/logger'; import { EventBufferArray } from './EventBufferArray'; import { EventBufferProxy } from './EventBufferProxy'; @@ -32,7 +33,7 @@ export function createEventBuffer({ } } - logInfo('[Replay] Using simple buffer'); + DEBUG_BUILD && logger.info('Using simple buffer'); return new EventBufferArray(); } @@ -44,11 +45,11 @@ function _loadWorker(customWorkerUrl?: string): EventBufferProxy | void { return; } - logInfo(`[Replay] Using compression worker${customWorkerUrl ? ` from ${customWorkerUrl}` : ''}`); + DEBUG_BUILD && logger.info(`Using compression worker${customWorkerUrl ? ` from ${customWorkerUrl}` : ''}`); const worker = new Worker(workerUrl); return new EventBufferProxy(worker); } catch (error) { - logInfo('[Replay] Failed to create compression worker'); + DEBUG_BUILD && logger.exception(error, 'Failed to create compression worker'); // Fall back to use simple event buffer array } } diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index f42d6ef6964a..b48ac787543b 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -1,15 +1,8 @@ /* eslint-disable max-lines */ // TODO: We might want to split this file up import { EventType, record } from '@sentry-internal/rrweb'; -import { - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, - captureException, - getActiveSpan, - getClient, - getRootSpan, - spanToJSON, -} from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getActiveSpan, getClient, getRootSpan, spanToJSON } from '@sentry/core'; import type { ReplayRecordingMode, Span } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { logger } from './util/logger'; import { BUFFER_CHECKOUT_TIME, @@ -60,7 +53,6 @@ import { debounce } from './util/debounce'; import { getHandleRecordingEmit } from './util/handleRecordingEmit'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; -import { logInfo, logInfoNextTick } from './util/log'; import { sendReplay } from './util/sendReplay'; import type { SKIPPED } from './util/throttle'; import { THROTTLED, throttle } from './util/throttle'; @@ -212,6 +204,15 @@ export class ReplayContainer implements ReplayContainerInterface { if (slowClickConfig) { this.clickDetector = new ClickDetector(this, slowClickConfig); } + + // Configure replay logger w/ experimental options + if (DEBUG_BUILD) { + const experiments = options._experiments; + logger.setConfig({ + captureExceptions: !!experiments.captureExceptions, + traceInternals: !!experiments.traceInternals, + }); + } } /** Get the event context. */ @@ -243,11 +244,7 @@ export class ReplayContainer implements ReplayContainerInterface { /** A wrapper to conditionally capture exceptions. */ public handleException(error: unknown): void { - DEBUG_BUILD && logger.error('[Replay]', error); - - if (DEBUG_BUILD && this._options._experiments && this._options._experiments.captureExceptions) { - captureException(error); - } + DEBUG_BUILD && logger.exception(error); } /** @@ -273,7 +270,7 @@ export class ReplayContainer implements ReplayContainerInterface { if (!this.session) { // This should not happen, something wrong has occurred - this.handleException(new Error('Unable to initialize and create session')); + DEBUG_BUILD && logger.exception(new Error('Unable to initialize and create session')); return; } @@ -287,10 +284,7 @@ export class ReplayContainer implements ReplayContainerInterface { // In this case, we still want to continue in `session` recording mode this.recordingMode = this.session.sampled === 'buffer' && this.session.segmentId === 0 ? 'buffer' : 'session'; - logInfoNextTick( - `[Replay] Starting replay in ${this.recordingMode} mode`, - this._options._experiments.traceInternals, - ); + DEBUG_BUILD && logger.infoTick(`Starting replay in ${this.recordingMode} mode`); this._initializeRecording(); } @@ -304,16 +298,16 @@ export class ReplayContainer implements ReplayContainerInterface { */ public start(): void { if (this._isEnabled && this.recordingMode === 'session') { - DEBUG_BUILD && logger.info('[Replay] Recording is already in progress'); + DEBUG_BUILD && logger.info('Recording is already in progress'); return; } if (this._isEnabled && this.recordingMode === 'buffer') { - DEBUG_BUILD && logger.info('[Replay] Buffering is in progress, call `flush()` to save the replay'); + DEBUG_BUILD && logger.info('Buffering is in progress, call `flush()` to save the replay'); return; } - logInfoNextTick('[Replay] Starting replay in session mode', this._options._experiments.traceInternals); + DEBUG_BUILD && logger.infoTick('Starting replay in session mode'); // Required as user activity is initially set in // constructor, so if `start()` is called after @@ -325,7 +319,6 @@ export class ReplayContainer implements ReplayContainerInterface { { maxReplayDuration: this._options.maxReplayDuration, sessionIdleExpire: this.timeouts.sessionIdleExpire, - traceInternals: this._options._experiments.traceInternals, }, { stickySession: this._options.stickySession, @@ -346,17 +339,16 @@ export class ReplayContainer implements ReplayContainerInterface { */ public startBuffering(): void { if (this._isEnabled) { - DEBUG_BUILD && logger.info('[Replay] Buffering is in progress, call `flush()` to save the replay'); + DEBUG_BUILD && logger.info('Buffering is in progress, call `flush()` to save the replay'); return; } - logInfoNextTick('[Replay] Starting replay in buffer mode', this._options._experiments.traceInternals); + DEBUG_BUILD && logger.infoTick('Starting replay in buffer mode'); const session = loadOrCreateSession( { sessionIdleExpire: this.timeouts.sessionIdleExpire, maxReplayDuration: this._options.maxReplayDuration, - traceInternals: this._options._experiments.traceInternals, }, { stickySession: this._options.stickySession, @@ -436,10 +428,7 @@ export class ReplayContainer implements ReplayContainerInterface { this._isEnabled = false; try { - logInfo( - `[Replay] Stopping Replay${reason ? ` triggered by ${reason}` : ''}`, - this._options._experiments.traceInternals, - ); + DEBUG_BUILD && logger.info(`Stopping Replay${reason ? ` triggered by ${reason}` : ''}`); this._removeListeners(); this.stopRecording(); @@ -476,7 +465,7 @@ export class ReplayContainer implements ReplayContainerInterface { this._isPaused = true; this.stopRecording(); - logInfo('[Replay] Pausing replay', this._options._experiments.traceInternals); + DEBUG_BUILD && logger.info('Pausing replay'); } /** @@ -493,7 +482,7 @@ export class ReplayContainer implements ReplayContainerInterface { this._isPaused = false; this.startRecording(); - logInfo('[Replay] Resuming replay', this._options._experiments.traceInternals); + DEBUG_BUILD && logger.info('Resuming replay'); } /** @@ -510,7 +499,7 @@ export class ReplayContainer implements ReplayContainerInterface { const activityTime = Date.now(); - logInfo('[Replay] Converting buffer to session', this._options._experiments.traceInternals); + DEBUG_BUILD && logger.info('Converting buffer to session'); // Allow flush to complete before resuming as a session recording, otherwise // the checkout from `startRecording` may be included in the payload. @@ -798,7 +787,6 @@ export class ReplayContainer implements ReplayContainerInterface { { sessionIdleExpire: this.timeouts.sessionIdleExpire, maxReplayDuration: this._options.maxReplayDuration, - traceInternals: this._options._experiments.traceInternals, previousSessionId, }, { @@ -990,7 +978,7 @@ export class ReplayContainer implements ReplayContainerInterface { // If the user has come back to the page within SESSION_IDLE_PAUSE_DURATION // ms, we will re-use the existing session, otherwise create a new // session - logInfo('[Replay] Document has become active, but session has expired'); + DEBUG_BUILD && logger.info('Document has become active, but session has expired'); return; } @@ -1106,7 +1094,7 @@ export class ReplayContainer implements ReplayContainerInterface { const replayId = this.getSessionId(); if (!this.session || !this.eventBuffer || !replayId) { - DEBUG_BUILD && logger.error('[Replay] No session or eventBuffer found to flush.'); + DEBUG_BUILD && logger.error('No session or eventBuffer found to flush.'); return; } @@ -1198,7 +1186,7 @@ export class ReplayContainer implements ReplayContainerInterface { } if (!this.checkAndHandleExpiredSession()) { - DEBUG_BUILD && logger.error('[Replay] Attempting to finish replay event after session expired.'); + DEBUG_BUILD && logger.error('Attempting to finish replay event after session expired.'); return; } @@ -1219,12 +1207,12 @@ export class ReplayContainer implements ReplayContainerInterface { const tooShort = duration < this._options.minReplayDuration; const tooLong = duration > this._options.maxReplayDuration + 5_000; if (tooShort || tooLong) { - logInfo( - `[Replay] Session duration (${Math.floor(duration / 1000)}s) is too ${ - tooShort ? 'short' : 'long' - }, not sending replay.`, - this._options._experiments.traceInternals, - ); + DEBUG_BUILD && + logger.info( + `Session duration (${Math.floor(duration / 1000)}s) is too ${ + tooShort ? 'short' : 'long' + }, not sending replay.`, + ); if (tooShort) { this._debouncedFlush(); @@ -1234,7 +1222,7 @@ export class ReplayContainer implements ReplayContainerInterface { const eventBuffer = this.eventBuffer; if (eventBuffer && this.session.segmentId === 0 && !eventBuffer.hasCheckout) { - logInfo('[Replay] Flushing initial segment without checkout.', this._options._experiments.traceInternals); + DEBUG_BUILD && logger.info('Flushing initial segment without checkout.'); // TODO FN: Evaluate if we want to stop here, or remove this again? } diff --git a/packages/replay-internal/src/session/fetchSession.ts b/packages/replay-internal/src/session/fetchSession.ts index 43e162b5f3d6..031605bfde87 100644 --- a/packages/replay-internal/src/session/fetchSession.ts +++ b/packages/replay-internal/src/session/fetchSession.ts @@ -1,13 +1,14 @@ import { REPLAY_SESSION_KEY, WINDOW } from '../constants'; +import { DEBUG_BUILD } from '../debug-build'; import type { Session } from '../types'; import { hasSessionStorage } from '../util/hasSessionStorage'; -import { logInfoNextTick } from '../util/log'; +import { logger } from '../util/logger'; import { makeSession } from './Session'; /** * Fetches a session from storage */ -export function fetchSession(traceInternals?: boolean): Session | null { +export function fetchSession(): Session | null { if (!hasSessionStorage()) { return null; } @@ -22,7 +23,7 @@ export function fetchSession(traceInternals?: boolean): Session | null { const sessionObj = JSON.parse(sessionStringFromStorage) as Session; - logInfoNextTick('[Replay] Loading existing session', traceInternals); + DEBUG_BUILD && logger.infoTick('Loading existing session'); return makeSession(sessionObj); } catch { diff --git a/packages/replay-internal/src/session/loadOrCreateSession.ts b/packages/replay-internal/src/session/loadOrCreateSession.ts index 1e1ac7664d40..d37c51590d54 100644 --- a/packages/replay-internal/src/session/loadOrCreateSession.ts +++ b/packages/replay-internal/src/session/loadOrCreateSession.ts @@ -1,5 +1,6 @@ +import { DEBUG_BUILD } from '../debug-build'; import type { Session, SessionOptions } from '../types'; -import { logInfoNextTick } from '../util/log'; +import { logger } from '../util/logger'; import { createSession } from './createSession'; import { fetchSession } from './fetchSession'; import { shouldRefreshSession } from './shouldRefreshSession'; @@ -10,23 +11,21 @@ import { shouldRefreshSession } from './shouldRefreshSession'; */ export function loadOrCreateSession( { - traceInternals, sessionIdleExpire, maxReplayDuration, previousSessionId, }: { sessionIdleExpire: number; maxReplayDuration: number; - traceInternals?: boolean; previousSessionId?: string; }, sessionOptions: SessionOptions, ): Session { - const existingSession = sessionOptions.stickySession && fetchSession(traceInternals); + const existingSession = sessionOptions.stickySession && fetchSession(); // No session exists yet, just create a new one if (!existingSession) { - logInfoNextTick('[Replay] Creating new session', traceInternals); + DEBUG_BUILD && logger.infoTick('Creating new session'); return createSession(sessionOptions, { previousSessionId }); } @@ -34,6 +33,6 @@ export function loadOrCreateSession( return existingSession; } - logInfoNextTick('[Replay] Session in sessionStorage is expired, creating new one...'); + DEBUG_BUILD && logger.infoTick('Session in sessionStorage is expired, creating new one...'); return createSession(sessionOptions, { previousSessionId: existingSession.id }); } diff --git a/packages/replay-internal/src/util/addEvent.ts b/packages/replay-internal/src/util/addEvent.ts index f397ea0564f6..700627cf954f 100644 --- a/packages/replay-internal/src/util/addEvent.ts +++ b/packages/replay-internal/src/util/addEvent.ts @@ -1,11 +1,10 @@ import { EventType } from '@sentry-internal/rrweb'; import { getClient } from '@sentry/core'; -import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; import { EventBufferSizeExceededError } from '../eventBuffer/error'; import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent, ReplayPluginOptions } from '../types'; -import { logInfoNextTick } from './log'; +import { logger } from './logger'; import { timestampToMs } from './timestamp'; function isCustomEvent(event: RecordingEvent): event is ReplayFrameEvent { @@ -109,10 +108,8 @@ export function shouldAddEvent(replay: ReplayContainer, event: RecordingEvent): // Throw out events that are +60min from the initial timestamp if (timestampInMs > replay.getContext().initialTimestamp + replay.getOptions().maxReplayDuration) { - logInfoNextTick( - `[Replay] Skipping event with timestamp ${timestampInMs} because it is after maxReplayDuration`, - replay.getOptions()._experiments.traceInternals, - ); + DEBUG_BUILD && + logger.infoTick(`Skipping event with timestamp ${timestampInMs} because it is after maxReplayDuration`); return false; } @@ -129,7 +126,7 @@ function maybeApplyCallback( } } catch (error) { DEBUG_BUILD && - logger.error('[Replay] An error occured in the `beforeAddRecordingEvent` callback, skipping the event...', error); + logger.exception(error, 'An error occured in the `beforeAddRecordingEvent` callback, skipping the event...'); return null; } diff --git a/packages/replay-internal/src/util/handleRecordingEmit.ts b/packages/replay-internal/src/util/handleRecordingEmit.ts index eaec29be261a..6b87845d793f 100644 --- a/packages/replay-internal/src/util/handleRecordingEmit.ts +++ b/packages/replay-internal/src/util/handleRecordingEmit.ts @@ -1,12 +1,11 @@ import { EventType } from '@sentry-internal/rrweb'; -import { logger } from '@sentry/utils'; import { updateClickDetectorForRecordingEvent } from '../coreHandlers/handleClick'; import { DEBUG_BUILD } from '../debug-build'; import { saveSession } from '../session/saveSession'; import type { RecordingEvent, ReplayContainer, ReplayOptionFrameEvent } from '../types'; import { addEventSync } from './addEvent'; -import { logInfo } from './log'; +import { logger } from './logger'; type RecordingEmitCallback = (event: RecordingEvent, isCheckout?: boolean) => void; @@ -21,7 +20,7 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa return (event: RecordingEvent, _isCheckout?: boolean) => { // If this is false, it means session is expired, create and a new session and wait for checkout if (!replay.checkAndHandleExpiredSession()) { - DEBUG_BUILD && logger.warn('[Replay] Received replay event after session expired.'); + DEBUG_BUILD && logger.warn('Received replay event after session expired.'); return; } @@ -82,10 +81,8 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa if (replay.recordingMode === 'buffer' && replay.session && replay.eventBuffer) { const earliestEvent = replay.eventBuffer.getEarliestTimestamp(); if (earliestEvent) { - logInfo( - `[Replay] Updating session start time to earliest event in buffer to ${new Date(earliestEvent)}`, - replay.getOptions()._experiments.traceInternals, - ); + DEBUG_BUILD && + logger.info(`Updating session start time to earliest event in buffer to ${new Date(earliestEvent)}`); replay.session.started = earliestEvent; diff --git a/packages/replay-internal/src/util/log.ts b/packages/replay-internal/src/util/log.ts deleted file mode 100644 index c847a093f02f..000000000000 --- a/packages/replay-internal/src/util/log.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { setTimeout } from '@sentry-internal/browser-utils'; -import { addBreadcrumb } from '@sentry/core'; -import { logger } from '@sentry/utils'; - -import { DEBUG_BUILD } from '../debug-build'; - -/** - * Log a message in debug mode, and add a breadcrumb when _experiment.traceInternals is enabled. - */ -export function logInfo(message: string, shouldAddBreadcrumb?: boolean): void { - if (!DEBUG_BUILD) { - return; - } - - logger.info(message); - - if (shouldAddBreadcrumb) { - addLogBreadcrumb(message); - } -} - -/** - * Log a message, and add a breadcrumb in the next tick. - * This is necessary when the breadcrumb may be added before the replay is initialized. - */ -export function logInfoNextTick(message: string, shouldAddBreadcrumb?: boolean): void { - if (!DEBUG_BUILD) { - return; - } - - logger.info(message); - - if (shouldAddBreadcrumb) { - // Wait a tick here to avoid race conditions for some initial logs - // which may be added before replay is initialized - setTimeout(() => { - addLogBreadcrumb(message); - }, 0); - } -} - -function addLogBreadcrumb(message: string): void { - addBreadcrumb( - { - category: 'console', - data: { - logger: 'replay', - }, - level: 'info', - message, - }, - { level: 'info' }, - ); -} diff --git a/packages/replay-internal/src/util/logger.ts b/packages/replay-internal/src/util/logger.ts new file mode 100644 index 000000000000..80445409164b --- /dev/null +++ b/packages/replay-internal/src/util/logger.ts @@ -0,0 +1,105 @@ +import { addBreadcrumb, captureException } from '@sentry/core'; +import type { ConsoleLevel, SeverityLevel } from '@sentry/types'; +import { logger as coreLogger } from '@sentry/utils'; + +import { DEBUG_BUILD } from '../debug-build'; + +type ReplayConsoleLevels = Extract; +const CONSOLE_LEVELS: readonly ReplayConsoleLevels[] = ['info', 'warn', 'error', 'log'] as const; +const PREFIX = '[Replay] '; + +type LoggerMethod = (...args: unknown[]) => void; +type LoggerConsoleMethods = Record; + +interface LoggerConfig { + captureExceptions: boolean; + traceInternals: boolean; +} + +interface ReplayLogger extends LoggerConsoleMethods { + /** + * Calls `logger.info` but saves breadcrumb in the next tick due to race + * conditions before replay is initialized. + */ + infoTick: LoggerMethod; + /** + * Captures exceptions (`Error`) if "capture internal exceptions" is enabled + */ + exception: LoggerMethod; + /** + * Configures the logger with additional debugging behavior + */ + setConfig(config: LoggerConfig): void; +} + +function _addBreadcrumb(message: unknown, level: SeverityLevel = 'info'): void { + addBreadcrumb( + { + category: 'console', + data: { + logger: 'replay', + }, + level, + message: `${PREFIX}${message}`, + }, + { level }, + ); +} + +function makeReplayLogger(): ReplayLogger { + let _capture = false; + let _trace = false; + + const _logger: Partial = { + exception: () => undefined, + infoTick: () => undefined, + setConfig: (opts: LoggerConfig) => { + _capture = opts.captureExceptions; + _trace = opts.traceInternals; + }, + }; + + if (DEBUG_BUILD) { + CONSOLE_LEVELS.forEach(name => { + _logger[name] = (...args: unknown[]) => { + coreLogger[name](PREFIX, ...args); + if (_trace) { + _addBreadcrumb(args[0]); + } + }; + }); + + _logger.exception = (error: unknown, ...message: unknown[]) => { + if (_logger.error) { + _logger.error(...message); + } + + coreLogger.error(PREFIX, error); + + if (_capture) { + captureException(error); + } else if (_trace) { + // No need for a breadcrumb is `_capture` is enabled since it should be + // captured as an exception + _addBreadcrumb(error); + } + }; + + _logger.infoTick = (...args: unknown[]) => { + coreLogger.info(PREFIX, ...args); + if (_trace) { + // Wait a tick here to avoid race conditions for some initial logs + // which may be added before replay is initialized + setTimeout(() => _addBreadcrumb(args[0]), 0); + } + }; + } else { + CONSOLE_LEVELS.forEach(name => { + _logger[name] = () => undefined; + }); + } + + return _logger as ReplayLogger; +} + +export const logger = makeReplayLogger(); diff --git a/packages/replay-internal/src/util/sendReplayRequest.ts b/packages/replay-internal/src/util/sendReplayRequest.ts index 03945bb479af..a623771af75b 100644 --- a/packages/replay-internal/src/util/sendReplayRequest.ts +++ b/packages/replay-internal/src/util/sendReplayRequest.ts @@ -5,9 +5,10 @@ import { resolvedSyncPromise } from '@sentry/utils'; import { isRateLimited, updateRateLimits } from '@sentry/utils'; import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants'; +import { DEBUG_BUILD } from '../debug-build'; import type { SendReplayData } from '../types'; import { createReplayEnvelope } from './createReplayEnvelope'; -import { logInfo } from './log'; +import { logger } from './logger'; import { prepareRecordingData } from './prepareRecordingData'; import { prepareReplayEvent } from './prepareReplayEvent'; @@ -57,7 +58,7 @@ export async function sendReplayRequest({ if (!replayEvent) { // Taken from baseclient's `_processEvent` method, where this is handled for errors/transactions client.recordDroppedEvent('event_processor', 'replay', baseEvent); - logInfo('An event processor returned `null`, will not send event.'); + DEBUG_BUILD && logger.info('An event processor returned `null`, will not send event.'); return resolvedSyncPromise({}); } diff --git a/packages/replay-internal/test/integration/flush.test.ts b/packages/replay-internal/test/integration/flush.test.ts index 999811de0a81..ffc0a83bb141 100644 --- a/packages/replay-internal/test/integration/flush.test.ts +++ b/packages/replay-internal/test/integration/flush.test.ts @@ -19,6 +19,7 @@ import { clearSession } from '../../src/session/clearSession'; import type { EventBuffer } from '../../src/types'; import { createPerformanceEntries } from '../../src/util/createPerformanceEntries'; import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; +import { logger } from '../../src/util/logger'; import * as SendReplay from '../../src/util/sendReplay'; import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index'; import type { DomHandler } from '../types'; @@ -335,7 +336,7 @@ describe('Integration | flush', () => { }); it('logs warning if flushing initial segment without checkout', async () => { - replay.getOptions()._experiments.traceInternals = true; + logger.setConfig({ traceInternals: true }); sessionStorage.clear(); clearSession(replay); @@ -408,11 +409,11 @@ describe('Integration | flush', () => { }, ]); - replay.getOptions()._experiments.traceInternals = false; + logger.setConfig({ traceInternals: false }); }); it('logs warning if adding event that is after maxReplayDuration', async () => { - replay.getOptions()._experiments.traceInternals = true; + logger.setConfig({ traceInternals: true }); const spyLogger = vi.spyOn(SentryUtils.logger, 'info'); @@ -440,12 +441,13 @@ describe('Integration | flush', () => { expect(mockSendReplay).toHaveBeenCalledTimes(0); expect(spyLogger).toHaveBeenLastCalledWith( - `[Replay] Skipping event with timestamp ${ + '[Replay] ', + `Skipping event with timestamp ${ BASE_TIMESTAMP + MAX_REPLAY_DURATION + 100 } because it is after maxReplayDuration`, ); - replay.getOptions()._experiments.traceInternals = false; + logger.setConfig({ traceInternals: false }); spyLogger.mockRestore(); }); From 6aeaf421c6664efbe8abcfaee00b1597c408454d Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Tue, 13 Aug 2024 03:33:40 -0400 Subject: [PATCH 12/17] test(browser): Add e2e tests for the `@sentry/browser` package (#13125) Add e2e test app for `@sentry/browser` package, testing - error - pageload span - navigation span --------- Co-authored-by: Lukas Stracke --- .github/workflows/build.yml | 1 + .../default-browser/.gitignore | 29 +++++ .../test-applications/default-browser/.npmrc | 2 + .../default-browser/build.mjs | 49 ++++++++ .../default-browser/package.json | 41 ++++++ .../default-browser/playwright.config.mjs | 7 ++ .../default-browser/public/index.html | 23 ++++ .../default-browser/src/index.js | 18 +++ .../default-browser/start-event-proxy.mjs | 6 + .../default-browser/tests/errors.test.ts | 58 +++++++++ .../default-browser/tests/performance.test.ts | 118 ++++++++++++++++++ .../default-browser/tsconfig.json | 20 +++ 12 files changed, 372 insertions(+) create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/.gitignore create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/.npmrc create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/build.mjs create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/package.json create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/playwright.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/public/index.html create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/src/index.js create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/start-event-proxy.mjs create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/tests/errors.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/tests/performance.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/default-browser/tsconfig.json diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d19c648e39b1..b18da4f6c43f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -867,6 +867,7 @@ jobs: 'create-remix-app-express', 'create-remix-app-express-legacy', 'create-remix-app-express-vite-dev', + 'default-browser', 'node-express-esm-loader', 'node-express-esm-preload', 'node-express-esm-without-loader', diff --git a/dev-packages/e2e-tests/test-applications/default-browser/.gitignore b/dev-packages/e2e-tests/test-applications/default-browser/.gitignore new file mode 100644 index 000000000000..84634c973eeb --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/.gitignore @@ -0,0 +1,29 @@ +# See https://help.github.com/articles/ignoring-files/ for more about ignoring files. + +# dependencies +/node_modules +/.pnp +.pnp.js + +# testing +/coverage + +# production +/build + +# misc +.DS_Store +.env.local +.env.development.local +.env.test.local +.env.production.local + +npm-debug.log* +yarn-debug.log* +yarn-error.log* + +/test-results/ +/playwright-report/ +/playwright/.cache/ + +!*.d.ts diff --git a/dev-packages/e2e-tests/test-applications/default-browser/.npmrc b/dev-packages/e2e-tests/test-applications/default-browser/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/default-browser/build.mjs b/dev-packages/e2e-tests/test-applications/default-browser/build.mjs new file mode 100644 index 000000000000..aeaad894bdbd --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/build.mjs @@ -0,0 +1,49 @@ +import * as path from 'path'; +import * as url from 'url'; +import HtmlWebpackPlugin from 'html-webpack-plugin'; +import TerserPlugin from 'terser-webpack-plugin'; +import webpack from 'webpack'; + +const __dirname = path.dirname(url.fileURLToPath(import.meta.url)); + +webpack( + { + entry: path.join(__dirname, 'src/index.js'), + output: { + path: path.join(__dirname, 'build'), + filename: 'app.js', + }, + optimization: { + minimize: true, + minimizer: [new TerserPlugin()], + }, + plugins: [ + new webpack.EnvironmentPlugin(['E2E_TEST_DSN']), + new HtmlWebpackPlugin({ + template: path.join(__dirname, 'public/index.html'), + }), + ], + mode: 'production', + }, + (err, stats) => { + if (err) { + console.error(err.stack || err); + if (err.details) { + console.error(err.details); + } + return; + } + + const info = stats.toJson(); + + if (stats.hasErrors()) { + console.error(info.errors); + process.exit(1); + } + + if (stats.hasWarnings()) { + console.warn(info.warnings); + process.exit(1); + } + }, +); diff --git a/dev-packages/e2e-tests/test-applications/default-browser/package.json b/dev-packages/e2e-tests/test-applications/default-browser/package.json new file mode 100644 index 000000000000..d6286c2423b6 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/package.json @@ -0,0 +1,41 @@ +{ + "name": "default-browser-test-app", + "version": "0.1.0", + "private": true, + "dependencies": { + "@sentry/browser": "latest || *", + "@types/node": "16.7.13", + "typescript": "4.9.5" + }, + "scripts": { + "start": "serve -s build", + "build": "node build.mjs", + "test": "playwright test", + "clean": "npx rimraf node_modules pnpm-lock.yaml", + "test:build": "pnpm install && npx playwright install && pnpm build", + "test:assert": "pnpm test" + }, + "browserslist": { + "production": [ + ">0.2%", + "not dead", + "not op_mini all" + ], + "development": [ + "last 1 chrome version", + "last 1 firefox version", + "last 1 safari version" + ] + }, + "devDependencies": { + "@playwright/test": "^1.44.1", + "@sentry-internal/test-utils": "link:../../../test-utils", + "webpack": "^5.91.0", + "serve": "14.0.1", + "terser-webpack-plugin": "^5.3.10", + "html-webpack-plugin": "^5.6.0" + }, + "volta": { + "extends": "../../package.json" + } +} diff --git a/dev-packages/e2e-tests/test-applications/default-browser/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/default-browser/playwright.config.mjs new file mode 100644 index 000000000000..31f2b913b58b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/playwright.config.mjs @@ -0,0 +1,7 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: `pnpm start`, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/default-browser/public/index.html b/dev-packages/e2e-tests/test-applications/default-browser/public/index.html new file mode 100644 index 000000000000..35e91be91c84 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/public/index.html @@ -0,0 +1,23 @@ + + + + + + Default Browser App + + +
+ + + + + + + + diff --git a/dev-packages/e2e-tests/test-applications/default-browser/src/index.js b/dev-packages/e2e-tests/test-applications/default-browser/src/index.js new file mode 100644 index 000000000000..d3eea216fe84 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/src/index.js @@ -0,0 +1,18 @@ +import * as Sentry from '@sentry/browser'; + +Sentry.init({ + dsn: process.env.E2E_TEST_DSN, + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: 1.0, + release: 'e2e-test', + environment: 'qa', + tunnel: 'http://localhost:3031', +}); + +document.getElementById('exception-button').addEventListener('click', () => { + throw new Error('I am an error!'); +}); + +document.getElementById('navigation-link').addEventListener('click', () => { + document.getElementById('navigation-target').scrollIntoView({ behavior: 'smooth' }); +}); diff --git a/dev-packages/e2e-tests/test-applications/default-browser/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/default-browser/start-event-proxy.mjs new file mode 100644 index 000000000000..6c84e74d541b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'default-browser', +}); diff --git a/dev-packages/e2e-tests/test-applications/default-browser/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/default-browser/tests/errors.test.ts new file mode 100644 index 000000000000..e4f2eda9a579 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/tests/errors.test.ts @@ -0,0 +1,58 @@ +import { expect, test } from '@playwright/test'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; + +test('captures an error', async ({ page }) => { + const errorEventPromise = waitForError('default-browser', event => { + return !event.type && event.exception?.values?.[0]?.value === 'I am an error!'; + }); + + await page.goto('/'); + + const exceptionButton = page.locator('id=exception-button'); + await exceptionButton.click(); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('I am an error!'); + + expect(errorEvent.transaction).toBe('/'); + + expect(errorEvent.request).toEqual({ + url: 'http://localhost:3030/', + headers: expect.any(Object), + }); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); + +test('sets correct transactionName', async ({ page }) => { + const transactionPromise = waitForTransaction('default-browser', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + const errorEventPromise = waitForError('default-browser', event => { + return !event.type && event.exception?.values?.[0]?.value === 'I am an error!'; + }); + + await page.goto('/'); + const transactionEvent = await transactionPromise; + + const exceptionButton = page.locator('id=exception-button'); + await exceptionButton.click(); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('I am an error!'); + + expect(errorEvent.transaction).toEqual('/'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: transactionEvent.contexts?.trace?.trace_id, + span_id: expect.not.stringContaining(transactionEvent.contexts?.trace?.span_id || ''), + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/default-browser/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/default-browser/tests/performance.test.ts new file mode 100644 index 000000000000..7013fb43ecef --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/tests/performance.test.ts @@ -0,0 +1,118 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test('captures a pageload transaction', async ({ page }) => { + const transactionPromise = waitForTransaction('default-browser', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/`); + + const pageLoadTransaction = await transactionPromise; + + expect(pageLoadTransaction).toEqual({ + contexts: { + trace: { + data: expect.objectContaining({ + 'sentry.idle_span_finish_reason': 'idleTimeout', + 'sentry.op': 'pageload', + 'sentry.origin': 'auto.pageload.browser', + 'sentry.sample_rate': 1, + 'sentry.source': 'url', + }), + op: 'pageload', + origin: 'auto.pageload.browser', + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + }, + environment: 'qa', + event_id: expect.stringMatching(/[a-f0-9]{32}/), + measurements: { + 'connection.rtt': { + unit: 'millisecond', + value: expect.any(Number), + }, + fcp: { + unit: 'millisecond', + value: expect.any(Number), + }, + fp: { + unit: 'millisecond', + value: expect.any(Number), + }, + lcp: { + unit: 'millisecond', + value: expect.any(Number), + }, + ttfb: { + unit: 'millisecond', + value: expect.any(Number), + }, + 'ttfb.requestTime': { + unit: 'millisecond', + value: expect.any(Number), + }, + }, + platform: 'javascript', + release: 'e2e-test', + request: { + headers: { + 'User-Agent': expect.any(String), + }, + url: 'http://localhost:3030/', + }, + sdk: { + integrations: expect.any(Array), + name: 'sentry.javascript.browser', + packages: [ + { + name: 'npm:@sentry/browser', + version: expect.any(String), + }, + ], + version: expect.any(String), + }, + spans: expect.any(Array), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + transaction: '/', + transaction_info: { + source: 'url', + }, + type: 'transaction', + }); +}); + +test('captures a navigation transaction', async ({ page }) => { + page.on('console', msg => console.log(msg.text())); + const pageLoadTransactionPromise = waitForTransaction('default-browser', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + const navigationTransactionPromise = waitForTransaction('default-browser', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; + }); + + await page.goto(`/`); + await pageLoadTransactionPromise; + + const linkElement = page.locator('id=navigation-link'); + + await linkElement.click(); + + const navigationTransaction = await navigationTransactionPromise; + + expect(navigationTransaction).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.browser', + }, + }, + transaction: '/', + transaction_info: { + source: 'url', + }, + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/default-browser/tsconfig.json b/dev-packages/e2e-tests/test-applications/default-browser/tsconfig.json new file mode 100644 index 000000000000..4cc95dc2689a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/default-browser/tsconfig.json @@ -0,0 +1,20 @@ +{ + "compilerOptions": { + "target": "es2018", + "lib": ["dom", "dom.iterable", "esnext"], + "allowJs": true, + "skipLibCheck": true, + "esModuleInterop": true, + "allowSyntheticDefaultImports": true, + "strict": true, + "forceConsistentCasingInFileNames": true, + "noFallthroughCasesInSwitch": true, + "module": "esnext", + "moduleResolution": "node", + "resolveJsonModule": true, + "isolatedModules": true, + "noEmit": true, + "jsx": "react" + }, + "include": ["src", "tests"] +} From e03df3761e081541fabfaf0ac6fa5bd5a82fb7e3 Mon Sep 17 00:00:00 2001 From: Inozemtsev Denis Date: Tue, 13 Aug 2024 14:59:12 +0700 Subject: [PATCH 13/17] feat(browser): Allow sentry in safari extension background page (#13209) Add `safari-web-extension:` to the list of allowed extension protocols. This chage should allow sentry/browser to work in safari browser extensions. --- packages/browser/src/sdk.ts | 2 +- packages/browser/test/sdk.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 04aa82b5f0e6..1a0296341341 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -93,7 +93,7 @@ function shouldShowBrowserExtensionError(): boolean { const runtimeId = extensionObject && extensionObject.runtime && extensionObject.runtime.id; const href = (WINDOW.location && WINDOW.location.href) || ''; - const extensionProtocols = ['chrome-extension:', 'moz-extension:', 'ms-browser-extension:']; + const extensionProtocols = ['chrome-extension:', 'moz-extension:', 'ms-browser-extension:', 'safari-web-extension:']; // Running the SDK in a dedicated extension page and calling Sentry.init is fine; no risk of data leakage const isDedicatedExtensionPage = diff --git a/packages/browser/test/sdk.test.ts b/packages/browser/test/sdk.test.ts index 618333532a09..d638862aba9d 100644 --- a/packages/browser/test/sdk.test.ts +++ b/packages/browser/test/sdk.test.ts @@ -199,7 +199,7 @@ describe('init', () => { consoleErrorSpy.mockRestore(); }); - it.each(['chrome-extension', 'moz-extension', 'ms-browser-extension'])( + it.each(['chrome-extension', 'moz-extension', 'ms-browser-extension', 'safari-web-extension'])( "doesn't log a browser extension error if executed inside an extension running in a dedicated page (%s)", extensionProtocol => { const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); From 40a86d5c0b58839f40e6bacec98c491b5e0e63a2 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 13 Aug 2024 09:59:37 +0200 Subject: [PATCH 14/17] chore(ci): Adjust contribution message for multiple contributors (#13335) Use plural "contributions" if multiple contributors are mentioned --- dev-packages/external-contributor-gh-action/index.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/external-contributor-gh-action/index.mjs b/dev-packages/external-contributor-gh-action/index.mjs index 7eff418e9205..ffa9369ee2df 100644 --- a/dev-packages/external-contributor-gh-action/index.mjs +++ b/dev-packages/external-contributor-gh-action/index.mjs @@ -48,7 +48,7 @@ async function run() { const newContributors = formatter.format(users); const newChangelog = changelogStr.replace( contributorMessageRegex, - `Work in this release was contributed by ${newContributors}. Thank you for your contribution!`, + `Work in this release was contributed by ${newContributors}. Thank you for your contributions!`, ); fs.writeFile(changelogFilePath, newChangelog); From 899c571b7b2062e4db6943c83be77852ec44c08d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 13 Aug 2024 10:17:25 +0200 Subject: [PATCH 15/17] ref: Add external contributor to CHANGELOG.md (#13334) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13125 --- Co-authored-by: Lukas Stracke --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 152f75a9169c..3f8110348cd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott -Work in this release was contributed by @MonstraG. Thank you for your contribution! +Work in this release was contributed by @MonstraG and @Zen-cronic. Thank you for your contributions! ## 8.25.0 From 0e7c492c8792535ed5bc469e72bc55511022354e Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 13 Aug 2024 10:21:16 +0200 Subject: [PATCH 16/17] feat(sveltekit): Always add browserTracingIntegration (#13322) closes https://github.com/getsentry/sentry-javascript/issues/13011 ref: https://github.com/getsentry/sentry-javascript/pull/13318 --- packages/sveltekit/src/client/sdk.ts | 12 +++++------- packages/sveltekit/test/client/sdk.test.ts | 14 +------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/packages/sveltekit/src/client/sdk.ts b/packages/sveltekit/src/client/sdk.ts index 98fd328c7abe..f01a033012a3 100644 --- a/packages/sveltekit/src/client/sdk.ts +++ b/packages/sveltekit/src/client/sdk.ts @@ -1,4 +1,4 @@ -import { applySdkMetadata, hasTracingEnabled } from '@sentry/core'; +import { applySdkMetadata } from '@sentry/core'; import type { BrowserOptions } from '@sentry/svelte'; import { getDefaultIntegrations as getDefaultSvelteIntegrations } from '@sentry/svelte'; import { WINDOW, init as initSvelteSdk } from '@sentry/svelte'; @@ -41,15 +41,13 @@ export function init(options: BrowserOptions): Client | undefined { } function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefined { - // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", in which case everything inside - // will get treeshaken away + // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", + // in which case everything inside will get tree-shaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { - if (hasTracingEnabled(options)) { - return [...getDefaultSvelteIntegrations(options), svelteKitBrowserTracingIntegration()]; - } + return [...getDefaultSvelteIntegrations(options), svelteKitBrowserTracingIntegration()]; } - return undefined; + return getDefaultSvelteIntegrations(options); } /** diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts index cdecffbea3a5..90593cf1f34c 100644 --- a/packages/sveltekit/test/client/sdk.test.ts +++ b/packages/sveltekit/test/client/sdk.test.ts @@ -46,6 +46,7 @@ describe('Sentry client SDK', () => { ['tracesSampleRate', { tracesSampleRate: 0 }], ['tracesSampler', { tracesSampler: () => 1.0 }], ['enableTracing', { enableTracing: true }], + ['no tracing option set', {}], ])('adds a browserTracingIntegration if tracing is enabled via %s', (_, tracingOptions) => { init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', @@ -56,19 +57,6 @@ describe('Sentry client SDK', () => { expect(browserTracing).toBeDefined(); }); - it.each([ - ['enableTracing', { enableTracing: false }], - ['no tracing option set', {}], - ])("doesn't add a browserTracingIntegration integration if tracing is disabled via %s", (_, tracingOptions) => { - init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - ...tracingOptions, - }); - - const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); - expect(browserTracing).toBeUndefined(); - }); - it("doesn't add a browserTracingIntegration if `__SENTRY_TRACING__` is set to false", () => { // This is the closest we can get to unit-testing the `__SENTRY_TRACING__` tree-shaking guard // IRL, the code to add the integration would most likely be removed by the bundler. From 70e1815c35f255598726974e86173bd98724aa74 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 13 Aug 2024 10:49:54 +0200 Subject: [PATCH 17/17] feat(nextjs): Always add browserTracingIntegration (#13324) closes https://github.com/getsentry/sentry-javascript/issues/13012 ref: https://github.com/getsentry/sentry-javascript/pull/13323 --- .size-limit.js | 2 +- packages/nextjs/src/client/index.ts | 11 ++++------- packages/nextjs/test/clientSdk.test.ts | 21 ++++++++------------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index 6369aa49e3e9..80aa4c5095ea 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -193,7 +193,7 @@ module.exports = [ import: createImport('init'), ignore: ['next/router', 'next/constants'], gzip: true, - limit: '38.03 KB', + limit: '38.05 KB', }, // SvelteKit SDK (ESM) { diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index 597cc3d4cd91..a68734a10398 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -1,4 +1,4 @@ -import { addEventProcessor, applySdkMetadata, hasTracingEnabled } from '@sentry/core'; +import { addEventProcessor, applySdkMetadata } from '@sentry/core'; import type { BrowserOptions } from '@sentry/react'; import { getDefaultIntegrations as getReactDefaultIntegrations, init as reactInit } from '@sentry/react'; import type { Client, EventProcessor, Integration } from '@sentry/types'; @@ -48,13 +48,10 @@ export function init(options: BrowserOptions): Client | undefined { function getDefaultIntegrations(options: BrowserOptions): Integration[] { const customDefaultIntegrations = getReactDefaultIntegrations(options); - - // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", in which case everything inside - // will get treeshaken away + // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", + // in which case everything inside will get tree-shaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { - if (hasTracingEnabled(options)) { - customDefaultIntegrations.push(browserTracingIntegration()); - } + customDefaultIntegrations.push(browserTracingIntegration()); } // This value is injected at build time, based on the output directory specified in the build config. Though a default diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts index 169c7cde5bfc..ac159564410b 100644 --- a/packages/nextjs/test/clientSdk.test.ts +++ b/packages/nextjs/test/clientSdk.test.ts @@ -130,33 +130,28 @@ describe('Client init()', () => { }); describe('browserTracingIntegration()', () => { - it('adds `browserTracingIntegration()` integration if `tracesSampleRate` is set', () => { + it('adds the browserTracingIntegration when `__SENTRY_TRACING__` is not set', () => { const client = init({ dsn: TEST_DSN, - tracesSampleRate: 1.0, }); const browserTracingIntegration = client?.getIntegrationByName('BrowserTracing'); - expect(browserTracingIntegration?.name).toBe('BrowserTracing'); + expect(browserTracingIntegration).toBeDefined(); }); - it('adds `browserTracingIntegration()` integration if `tracesSampler` is set', () => { - const client = init({ - dsn: TEST_DSN, - tracesSampler: () => true, - }); + it("doesn't add a browserTracingIntegration if `__SENTRY_TRACING__` is set to false", () => { + // @ts-expect-error Test setup for build-time flag + globalThis.__SENTRY_TRACING__ = false; - const browserTracingIntegration = client?.getIntegrationByName('BrowserTracing'); - expect(browserTracingIntegration?.name).toBe('BrowserTracing'); - }); - - it('does not add `browserTracingIntegration()` integration if tracing not enabled in SDK', () => { const client = init({ dsn: TEST_DSN, }); const browserTracingIntegration = client?.getIntegrationByName('BrowserTracing'); expect(browserTracingIntegration).toBeUndefined(); + + // @ts-expect-error Test setup for build-time flag + delete globalThis.__SENTRY_TRACING__; }); }); });