From cf63845fb915f850ff8b4359e2fcc67e62e3eb0f Mon Sep 17 00:00:00 2001 From: Andrei <168741329+andreiborza@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:58:24 +0200 Subject: [PATCH 01/35] chore(solidstart): Add solidstart sdk to the registry in .craft.yml (#13289) --- .craft.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.craft.yml b/.craft.yml index 185fa2fd0510..f08ee2832d25 100644 --- a/.craft.yml +++ b/.craft.yml @@ -206,6 +206,8 @@ targets: onlyIfPresent: /^sentry-remix-\d.*\.tgz$/ 'npm:@sentry/solid': onlyIfPresent: /^sentry-solid-\d.*\.tgz$/ + 'npm:@sentry/solidstart': + onlyIfPresent: /^sentry-solidstart-\d.*\.tgz$/ 'npm:@sentry/svelte': onlyIfPresent: /^sentry-svelte-\d.*\.tgz$/ 'npm:@sentry/sveltekit': From 2a1d8ee59440fddfaba99af683566f83210c54a9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 9 Aug 2024 13:32:23 +0200 Subject: [PATCH 02/35] feat(node): Add `fsInstrumentation` (#13291) --- .../fs-instrumentation/fixtures/.gitignore | 3 + .../fixtures/some-file-promises.txt | 8 + .../fixtures/some-file-promisify.txt | 8 + .../fs-instrumentation/fixtures/some-file.txt | 8 + .../suites/fs-instrumentation/server.ts | 137 +++++++++ .../suites/fs-instrumentation/test.ts | 260 ++++++++++++++++++ packages/astro/src/index.server.ts | 1 + packages/aws-serverless/src/index.ts | 1 + packages/bun/src/index.ts | 1 + packages/google-cloud-serverless/src/index.ts | 1 + packages/node/package.json | 1 + packages/node/src/index.ts | 1 + packages/node/src/integrations/fs.ts | 149 ++++++++++ yarn.lock | 8 + 14 files changed, 587 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/.gitignore create mode 100644 dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file-promises.txt create mode 100644 dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file-promisify.txt create mode 100644 dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file.txt create mode 100644 dev-packages/node-integration-tests/suites/fs-instrumentation/server.ts create mode 100644 dev-packages/node-integration-tests/suites/fs-instrumentation/test.ts create mode 100644 packages/node/src/integrations/fs.ts diff --git a/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/.gitignore b/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/.gitignore new file mode 100644 index 000000000000..814ce048cf6b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/.gitignore @@ -0,0 +1,3 @@ +some-file.txt.* +some-file-promises.txt.* +some-file-promisify.txt.* diff --git a/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file-promises.txt b/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file-promises.txt new file mode 100644 index 000000000000..00a0fd7faa1a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file-promises.txt @@ -0,0 +1,8 @@ + _____________________________ +< gimme some fs instrumentation > + ----------------------------- + \ ^__^ + \ (oo)\_______ + (__)\ )\/\\ + ||----w | + || || diff --git a/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file-promisify.txt b/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file-promisify.txt new file mode 100644 index 000000000000..00a0fd7faa1a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file-promisify.txt @@ -0,0 +1,8 @@ + _____________________________ +< gimme some fs instrumentation > + ----------------------------- + \ ^__^ + \ (oo)\_______ + (__)\ )\/\\ + ||----w | + || || diff --git a/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file.txt b/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file.txt new file mode 100644 index 000000000000..00a0fd7faa1a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/fs-instrumentation/fixtures/some-file.txt @@ -0,0 +1,8 @@ + _____________________________ +< gimme some fs instrumentation > + ----------------------------- + \ ^__^ + \ (oo)\_______ + (__)\ )\/\\ + ||----w | + || || diff --git a/dev-packages/node-integration-tests/suites/fs-instrumentation/server.ts b/dev-packages/node-integration-tests/suites/fs-instrumentation/server.ts new file mode 100644 index 000000000000..1320d7c3b4e2 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/fs-instrumentation/server.ts @@ -0,0 +1,137 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, + tracesSampleRate: 1, + integrations: [ + Sentry.fsIntegration({ + recordFilePaths: true, + recordErrorMessagesAsSpanAttributes: true, + }), + ], +}); + +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import * as util from 'util'; +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import express from 'express'; + +const app = express(); + +app.get('/readFile-error', async (_, res) => { + try { + await fs.promises.readFile(path.join(__dirname, 'fixtures', 'some-file-that-doesnt-exist.txt'), 'utf-8'); + } catch { + // noop + } + res.send('done'); +}); + +app.get('/readFile', async (_, res) => { + await new Promise(resolve => { + fs.readFile(path.join(__dirname, 'fixtures', 'some-file.txt'), 'utf-8', () => { + resolve(); + }); + }); + await fs.promises.readFile(path.join(__dirname, 'fixtures', 'some-file-promises.txt'), 'utf-8'); + await util.promisify(fs.readFile)(path.join(__dirname, 'fixtures', 'some-file-promisify.txt'), 'utf-8'); + res.send('done'); +}); + +app.get('/copyFile', async (_, res) => { + await new Promise(resolve => { + fs.copyFile( + path.join(__dirname, 'fixtures', 'some-file.txt'), + path.join(__dirname, 'fixtures', 'some-file.txt.copy'), + () => { + resolve(); + }, + ); + }); + await fs.promises.copyFile( + path.join(__dirname, 'fixtures', 'some-file-promises.txt'), + path.join(__dirname, 'fixtures', 'some-file-promises.txt.copy'), + ); + await util.promisify(fs.copyFile)( + path.join(__dirname, 'fixtures', 'some-file-promisify.txt'), + path.join(__dirname, 'fixtures', 'some-file-promisify.txt.copy'), + ); + res.send('done'); +}); + +app.get('/link', async (_, res) => { + await new Promise(resolve => { + fs.link( + path.join(__dirname, 'fixtures', 'some-file.txt'), + path.join(__dirname, 'fixtures', 'some-file.txt.link'), + () => { + resolve(); + }, + ); + }); + await fs.promises.link( + path.join(__dirname, 'fixtures', 'some-file-promises.txt'), + path.join(__dirname, 'fixtures', 'some-file-promises.txt.link'), + ); + await util.promisify(fs.link)( + path.join(__dirname, 'fixtures', 'some-file-promisify.txt'), + path.join(__dirname, 'fixtures', 'some-file-promisify.txt.link'), + ); + + await Promise.all([ + fs.promises.unlink(path.join(__dirname, 'fixtures', 'some-file.txt.link')), + fs.promises.unlink(path.join(__dirname, 'fixtures', 'some-file-promises.txt.link')), + fs.promises.unlink(path.join(__dirname, 'fixtures', 'some-file-promisify.txt.link')), + ]); + + res.send('done'); +}); + +app.get('/mkdtemp', async (_, res) => { + await new Promise(resolve => { + fs.mkdtemp(path.join(os.tmpdir(), 'foo-'), () => { + resolve(); + }); + }); + await fs.promises.mkdtemp(path.join(os.tmpdir(), 'foo-')); + await util.promisify(fs.mkdtemp)(path.join(os.tmpdir(), 'foo-')); + + res.send('done'); +}); + +app.get('/symlink', async (_, res) => { + await new Promise(resolve => { + fs.symlink( + path.join(__dirname, 'fixtures', 'some-file.txt'), + path.join(__dirname, 'fixtures', 'some-file.txt.symlink'), + () => { + resolve(); + }, + ); + }); + await fs.promises.symlink( + path.join(__dirname, 'fixtures', 'some-file-promises.txt'), + path.join(__dirname, 'fixtures', 'some-file-promises.txt.symlink'), + ); + await util.promisify(fs.symlink)( + path.join(__dirname, 'fixtures', 'some-file-promisify.txt'), + path.join(__dirname, 'fixtures', 'some-file-promisify.txt.symlink'), + ); + + await Promise.all([ + fs.promises.unlink(path.join(__dirname, 'fixtures', 'some-file.txt.symlink')), + fs.promises.unlink(path.join(__dirname, 'fixtures', 'some-file-promises.txt.symlink')), + fs.promises.unlink(path.join(__dirname, 'fixtures', 'some-file-promisify.txt.symlink')), + ]); + + res.send('done'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/fs-instrumentation/test.ts b/dev-packages/node-integration-tests/suites/fs-instrumentation/test.ts new file mode 100644 index 000000000000..05860fa1ce26 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/fs-instrumentation/test.ts @@ -0,0 +1,260 @@ +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/node'; +import { cleanupChildProcesses, createRunner } from '../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should create spans for fs operations that take target argument', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + transaction: { + transaction: 'GET /readFile-error', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'fs.readFile', + op: 'file', + status: 'unknown_error', + data: { + fs_error: expect.stringMatching('ENOENT: no such file or directory,'), + path_argument: expect.stringMatching('/fixtures/some-file-that-doesnt-exist.txt'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + ]), + }, + }) + .start(done); + + expect(runner.makeRequest('get', '/readFile-error')).resolves.toBe('done'); +}); + +test('should create spans for fs operations that take one path', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + transaction: { + transaction: 'GET /readFile', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'fs.readFile', + op: 'file', + status: 'ok', + data: { + path_argument: expect.stringMatching('/fixtures/some-file.txt'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.readFile', + op: 'file', + status: 'ok', + data: { + path_argument: expect.stringMatching('/fixtures/some-file-promises.txt'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.readFile', + op: 'file', + status: 'ok', + data: { + path_argument: expect.stringMatching('/fixtures/some-file-promisify.txt'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + ]), + }, + }) + .start(done); + + expect(runner.makeRequest('get', '/readFile')).resolves.toBe('done'); +}); + +test('should create spans for fs operations that take src and dest arguments', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + transaction: { + transaction: 'GET /copyFile', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'fs.copyFile', + op: 'file', + status: 'ok', + data: { + src_argument: expect.stringMatching('/fixtures/some-file.txt'), + dest_argument: expect.stringMatching('/fixtures/some-file.txt.copy'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.copyFile', + op: 'file', + status: 'ok', + data: { + src_argument: expect.stringMatching('/fixtures/some-file-promises.txt'), + dest_argument: expect.stringMatching('/fixtures/some-file-promises.txt.copy'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.copyFile', + op: 'file', + status: 'ok', + data: { + src_argument: expect.stringMatching('/fixtures/some-file-promisify.txt'), + dest_argument: expect.stringMatching('/fixtures/some-file-promisify.txt.copy'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + ]), + }, + }) + .start(done); + + expect(runner.makeRequest('get', '/copyFile')).resolves.toBe('done'); +}); + +test('should create spans for fs operations that take existing path and new path arguments', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + transaction: { + transaction: 'GET /link', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'fs.link', + op: 'file', + status: 'ok', + data: { + existing_path_argument: expect.stringMatching('/fixtures/some-file.txt'), + new_path_argument: expect.stringMatching('/fixtures/some-file.txt.link'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.link', + op: 'file', + status: 'ok', + data: { + existing_path_argument: expect.stringMatching('/some-file-promises.txt'), + new_path_argument: expect.stringMatching('/some-file-promises.txt.link'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.link', + op: 'file', + status: 'ok', + data: { + existing_path_argument: expect.stringMatching('/fixtures/some-file-promisify.txt'), + new_path_argument: expect.stringMatching('/fixtures/some-file-promisify.txt.link'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + ]), + }, + }) + .start(done); + + expect(runner.makeRequest('get', '/link')).resolves.toBe('done'); +}); + +test('should create spans for fs operations that take prefix argument', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + transaction: { + transaction: 'GET /mkdtemp', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'fs.mkdtemp', + op: 'file', + status: 'ok', + data: { + prefix_argument: expect.stringMatching('/foo-'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.mkdtemp', + op: 'file', + status: 'ok', + data: { + prefix_argument: expect.stringMatching('/foo-'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.mkdtemp', + op: 'file', + status: 'ok', + data: { + prefix_argument: expect.stringMatching('/foo-'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + ]), + }, + }) + .start(done); + + expect(runner.makeRequest('get', '/mkdtemp')).resolves.toBe('done'); +}); + +test('should create spans for fs operations that take target argument', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + transaction: { + transaction: 'GET /symlink', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'fs.symlink', + op: 'file', + status: 'ok', + data: { + target_argument: expect.stringMatching('/some-file-promisify.txt'), + path_argument: expect.stringMatching('/some-file-promisify.txt.symlink'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.symlink', + op: 'file', + status: 'ok', + data: { + target_argument: expect.stringMatching('/fixtures/some-file-promisify.txt'), + path_argument: expect.stringMatching('/fixtures/some-file-promisify.txt.symlink'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + expect.objectContaining({ + description: 'fs.symlink', + op: 'file', + status: 'ok', + data: { + target_argument: expect.stringMatching('/fixtures/some-file-promisify.txt'), + path_argument: expect.stringMatching('/fixtures/some-file-promisify.txt.symlink'), + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }, + }), + ]), + }, + }) + .start(done); + + expect(runner.makeRequest('get', '/symlink')).resolves.toBe('done'); +}); diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 7f52ad0dc0bb..be3f002dcbb8 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -40,6 +40,7 @@ export { extraErrorDataIntegration, fastifyIntegration, flush, + fsIntegration, functionToStringIntegration, generateInstrumentOnce, getActiveSpan, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 20ef9eeaf09f..7b05f8df3a86 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -86,6 +86,7 @@ export { connectIntegration, setupConnectErrorHandler, fastifyIntegration, + fsIntegration, graphqlIntegration, mongoIntegration, mongooseIntegration, diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index 4de55fd1c5f7..e49e4163af31 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -77,6 +77,7 @@ export { nodeContextIntegration, localVariablesIntegration, requestDataIntegration, + fsIntegration, functionToStringIntegration, inboundFiltersIntegration, linkedErrorsIntegration, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index 73f24f9cf39e..9a501307e79f 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -57,6 +57,7 @@ export { nodeContextIntegration, localVariablesIntegration, requestDataIntegration, + fsIntegration, functionToStringIntegration, inboundFiltersIntegration, linkedErrorsIntegration, diff --git a/packages/node/package.json b/packages/node/package.json index 523fc9a5dcf1..9ec4c8a7e389 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -72,6 +72,7 @@ "@opentelemetry/instrumentation-connect": "0.38.0", "@opentelemetry/instrumentation-express": "0.41.1", "@opentelemetry/instrumentation-fastify": "0.38.0", + "@opentelemetry/instrumentation-fs": "0.14.0", "@opentelemetry/instrumentation-graphql": "0.42.0", "@opentelemetry/instrumentation-hapi": "0.40.0", "@opentelemetry/instrumentation-http": "0.52.1", diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 1fdc32d3d77a..9ef89ab42fb7 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -1,5 +1,6 @@ export { httpIntegration } from './integrations/http'; export { nativeNodeFetchIntegration } from './integrations/node-fetch'; +export { fsIntegration } from './integrations/fs'; export { consoleIntegration } from './integrations/console'; export { nodeContextIntegration } from './integrations/context'; diff --git a/packages/node/src/integrations/fs.ts b/packages/node/src/integrations/fs.ts new file mode 100644 index 000000000000..2288096dad43 --- /dev/null +++ b/packages/node/src/integrations/fs.ts @@ -0,0 +1,149 @@ +import { FsInstrumentation } from '@opentelemetry/instrumentation-fs'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration } from '@sentry/core'; +import { generateInstrumentOnce } from '../otel/instrument'; + +const INTEGRATION_NAME = 'FileSystem'; + +/** + * This integration will create spans for `fs` API operations, like reading and writing files. + * + * **WARNING:** This integration may add significant overhead to your application. Especially in scenarios with a lot of + * file I/O, like for example when running a framework dev server, including this integration can massively slow down + * your application. + * + * @param options Configuration for this integration. + */ +export const fsIntegration = defineIntegration( + ( + options: { + /** + * Setting this option to `true` will include any filepath arguments from your `fs` API calls as span attributes. + * + * Defaults to `false`. + */ + recordFilePaths?: boolean; + + /** + * Setting this option to `true` will include the error messages of failed `fs` API calls as a span attribute. + * + * Defaults to `false`. + */ + recordErrorMessagesAsSpanAttributes?: boolean; + } = {}, + ) => { + return { + name: INTEGRATION_NAME, + setupOnce() { + generateInstrumentOnce( + INTEGRATION_NAME, + () => + new FsInstrumentation({ + requireParentSpan: true, + endHook(functionName, { args, span, error }) { + span.updateName(`fs.${functionName}`); + + span.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'file', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.file.fs', + }); + + if (options.recordErrorMessagesAsSpanAttributes) { + if (typeof args[0] === 'string' && FS_OPERATIONS_WITH_PATH_ARG.includes(functionName)) { + span.setAttribute('path_argument', args[0]); + } else if ( + typeof args[0] === 'string' && + typeof args[1] === 'string' && + FS_OPERATIONS_WITH_TARGET_PATH.includes(functionName) + ) { + span.setAttribute('target_argument', args[0]); + span.setAttribute('path_argument', args[1]); + } else if (typeof args[0] === 'string' && FS_OPERATIONS_WITH_PREFIX.includes(functionName)) { + span.setAttribute('prefix_argument', args[0]); + } else if ( + typeof args[0] === 'string' && + typeof args[1] === 'string' && + FS_OPERATIONS_WITH_EXISTING_PATH_NEW_PATH.includes(functionName) + ) { + span.setAttribute('existing_path_argument', args[0]); + span.setAttribute('new_path_argument', args[1]); + } else if ( + typeof args[0] === 'string' && + typeof args[1] === 'string' && + FS_OPERATIONS_WITH_SRC_DEST.includes(functionName) + ) { + span.setAttribute('src_argument', args[0]); + span.setAttribute('dest_argument', args[1]); + } else if ( + typeof args[0] === 'string' && + typeof args[1] === 'string' && + FS_OPERATIONS_WITH_OLD_PATH_NEW_PATH.includes(functionName) + ) { + span.setAttribute('old_path_argument', args[0]); + span.setAttribute('new_path_argument', args[1]); + } + } + + if (error && options.recordErrorMessagesAsSpanAttributes) { + span.setAttribute('fs_error', error.message); + } + }, + }), + )(); + }, + }; + }, +); + +const FS_OPERATIONS_WITH_OLD_PATH_NEW_PATH = ['rename', 'renameSync']; +const FS_OPERATIONS_WITH_SRC_DEST = ['copyFile', 'cp', 'copyFileSync', 'cpSync']; +const FS_OPERATIONS_WITH_EXISTING_PATH_NEW_PATH = ['link', 'linkSync']; +const FS_OPERATIONS_WITH_PREFIX = ['mkdtemp', 'mkdtempSync']; +const FS_OPERATIONS_WITH_TARGET_PATH = ['symlink', 'symlinkSync']; +const FS_OPERATIONS_WITH_PATH_ARG = [ + 'access', + 'appendFile', + 'chmod', + 'chown', + 'exists', + 'mkdir', + 'lchown', + 'lstat', + 'lutimes', + 'open', + 'opendir', + 'readdir', + 'readFile', + 'readlink', + 'realpath', + 'realpath.native', + 'rm', + 'rmdir', + 'stat', + 'truncate', + 'unlink', + 'utimes', + 'writeFile', + 'accessSync', + 'appendFileSync', + 'chmodSync', + 'chownSync', + 'existsSync', + 'lchownSync', + 'lstatSync', + 'lutimesSync', + 'opendirSync', + 'mkdirSync', + 'openSync', + 'readdirSync', + 'readFileSync', + 'readlinkSync', + 'realpathSync', + 'realpathSync.native', + 'rmdirSync', + 'rmSync', + 'statSync', + 'truncateSync', + 'unlinkSync', + 'utimesSync', + 'writeFileSync', +]; diff --git a/yarn.lock b/yarn.lock index 660c53cce594..08a7a3eb7487 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7113,6 +7113,14 @@ "@opentelemetry/instrumentation" "^0.52.0" "@opentelemetry/semantic-conventions" "^1.22.0" +"@opentelemetry/instrumentation-fs@0.14.0": + version "0.14.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-fs/-/instrumentation-fs-0.14.0.tgz#19f1cb38a8c2d05f3b96af67f1c8d43f0af2829b" + integrity sha512-pVc8P5AgliC1DphyyBUgsxXlm2XaPH4BpYvt7rAZDMIqUpRk8gs19SioABtKqqxvFzg5jPtgJfJsdxq0Y+maLw== + dependencies: + "@opentelemetry/core" "^1.8.0" + "@opentelemetry/instrumentation" "^0.52.0" + "@opentelemetry/instrumentation-graphql@0.42.0": version "0.42.0" resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-graphql/-/instrumentation-graphql-0.42.0.tgz#588a18c39e3b3f655bc09243566172ab0b638d35" From 28a8237e50054b21216deb71afe0a747fbf188a6 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Fri, 9 Aug 2024 10:17:48 -0400 Subject: [PATCH 03/35] feat(tracing): make long animation frames opt-out (#13255) Enable long animation frames by default. If long animation frames are not supported by the browser, Sentry will fallback to using long tasks where enabled. --- .../interactions/test.ts | 20 +++++++++++-------- .../long-tasks-disabled/init.js | 4 +++- .../long-tasks-enabled/init.js | 1 + .../src/tracing/browserTracingIntegration.ts | 9 +++++++-- packages/ember/tests/helpers/utils.ts | 8 ++++++-- packages/utils/src/worldwide.ts | 1 + 6 files changed, 30 insertions(+), 13 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions/test.ts index bb219eda38c7..4ebea3457af6 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions/test.ts @@ -33,9 +33,11 @@ sentryTest('should capture interaction transaction. @firefox', async ({ browserN expect(eventData.contexts).toMatchObject({ trace: { op: 'ui.action.click' } }); expect(eventData.platform).toBe('javascript'); expect(eventData.type).toBe('transaction'); - expect(eventData.spans).toHaveLength(1); - const interactionSpan = eventData.spans![0]; + const spans = eventData.spans?.filter(span => !span.op?.startsWith('ui.long-animation-frame')); + expect(spans).toHaveLength(1); + + const interactionSpan = spans![0]; expect(interactionSpan.op).toBe('ui.interaction.click'); expect(interactionSpan.description).toBe('body > button.clicked'); expect(interactionSpan.timestamp).toBeDefined(); @@ -63,7 +65,8 @@ sentryTest( await page.waitForTimeout(1000); await page.locator('[data-test-id=interaction-button]').click(); const envelope = await envelopePromise; - expect(envelope[0].spans).toHaveLength(1); + const spans = envelope[0].spans?.filter(span => !span.op?.startsWith('ui.long-animation-frame')); + expect(spans).toHaveLength(1); } }, ); @@ -89,10 +92,10 @@ sentryTest( const envelopes = await envelopePromise; expect(envelopes).toHaveLength(1); const eventData = envelopes[0]; + const spans = eventData.spans?.filter(span => !span.op?.startsWith('ui.long-animation-frame')); + expect(spans).toHaveLength(1); - expect(eventData.spans).toHaveLength(1); - - const interactionSpan = eventData.spans![0]; + const interactionSpan = spans![0]; expect(interactionSpan.op).toBe('ui.interaction.click'); expect(interactionSpan.description).toBe('body > AnnotatedButton'); }, @@ -120,9 +123,10 @@ sentryTest( expect(envelopes).toHaveLength(1); const eventData = envelopes[0]; - expect(eventData.spans).toHaveLength(1); + const spans = eventData.spans?.filter(span => !span.op?.startsWith('ui.long-animation-frame')); + expect(spans).toHaveLength(1); - const interactionSpan = eventData.spans![0]; + const interactionSpan = spans![0]; expect(interactionSpan.op).toBe('ui.interaction.click'); expect(interactionSpan.description).toBe('body > StyledButton'); }, diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/long-tasks-disabled/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/long-tasks-disabled/init.js index bde12a1304ed..e1b3f6b13b01 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/long-tasks-disabled/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/long-tasks-disabled/init.js @@ -4,6 +4,8 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [Sentry.browserTracingIntegration({ enableLongTask: false, idleTimeout: 9000 })], + integrations: [ + Sentry.browserTracingIntegration({ enableLongTask: false, enableLongAnimationFrame: false, idleTimeout: 9000 }), + ], tracesSampleRate: 1, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/long-tasks-enabled/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/long-tasks-enabled/init.js index ad1d8832b228..319dfaadd4a8 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/long-tasks-enabled/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/long-tasks-enabled/init.js @@ -7,6 +7,7 @@ Sentry.init({ integrations: [ Sentry.browserTracingIntegration({ idleTimeout: 9000, + enableLongAnimationFrame: false, }), ], tracesSampleRate: 1, diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index d8f6796b5b1b..491c7aaae88d 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -28,6 +28,7 @@ import { import type { Client, IntegrationFn, StartSpanOptions, TransactionSource } from '@sentry/types'; import type { Span } from '@sentry/types'; import { + GLOBAL_OBJ, browserPerformanceTimeOrigin, generatePropagationContext, getDomElement, @@ -168,7 +169,7 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { instrumentPageLoad: true, markBackgroundSpan: true, enableLongTask: true, - enableLongAnimationFrame: false, + enableLongAnimationFrame: true, enableInp: true, _experiments: {}, ...defaultRequestInstrumentationOptions, @@ -213,7 +214,11 @@ export const browserTracingIntegration = ((_options: Partial { const op = span.op; - return !op?.startsWith('ui.ember.runloop.') && !op?.startsWith('ui.long-task'); + return ( + !op?.startsWith('ui.ember.runloop.') && + !op?.startsWith('ui.long-task') && + !op?.startsWith('ui.long-animation-frame') + ); }) .map(spanJson => { return `${spanJson.op} | ${spanJson.description}`; diff --git a/packages/utils/src/worldwide.ts b/packages/utils/src/worldwide.ts index 7e428444e21d..e323f12034a2 100644 --- a/packages/utils/src/worldwide.ts +++ b/packages/utils/src/worldwide.ts @@ -49,6 +49,7 @@ type BackwardsCompatibleSentryCarrier = SentryCarrier & { export type InternalGlobal = { navigator?: { userAgent?: string }; console: Console; + PerformanceObserver?: any; Sentry?: any; onerror?: { (event: object | string, source?: string, lineno?: number, colno?: number, error?: Error): any; From dd0501b50bc680c1a251cb2359a217d9af901b7c Mon Sep 17 00:00:00 2001 From: Andrei <168741329+andreiborza@users.noreply.github.com> Date: Fri, 9 Aug 2024 17:01:46 +0200 Subject: [PATCH 04/35] fix(solidstart): Remove public DSN from README (#13294) --- packages/solidstart/README.md | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/solidstart/README.md b/packages/solidstart/README.md index b654b9bdf744..b5d775781875 100644 --- a/packages/solidstart/README.md +++ b/packages/solidstart/README.md @@ -66,7 +66,7 @@ Create an instrument file named `instrument.server.mjs` and add your initializat import * as Sentry from '@sentry/solidstart'; Sentry.init({ - dsn: 'https://0e67f7dd5326d51506e92d7f1eff887a@o447951.ingest.us.sentry.io/4507459091824640', + dsn: '__PUBLIC_DSN__', tracesSampleRate: 1.0, // Capture 100% of the transactions }); ``` @@ -145,11 +145,6 @@ JS `ErrorBoundary` component with `Sentry.withSentryErrorBoundary`. import * as Sentry from '@sentry/solidstart'; import { ErrorBoundary } from 'solid-js'; -Sentry.init({ - dsn: '__PUBLIC_DSN__', - tracesSampleRate: 1.0, // Capture 100% of the transactions -}); - const SentryErrorBoundary = Sentry.withSentryErrorBoundary(ErrorBoundary); render( From a6fda4c1cbc3cea44b1c0386ebeb08c4d43b6a24 Mon Sep 17 00:00:00 2001 From: Arseny Garelyshev Date: Fri, 9 Aug 2024 17:17:54 +0200 Subject: [PATCH 05/35] feat(nextjs): export SentryBuildOptions (#13296) Refs: #13295 --- packages/nextjs/src/config/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/nextjs/src/config/index.ts b/packages/nextjs/src/config/index.ts index c4032ee44d46..d191fb9673e2 100644 --- a/packages/nextjs/src/config/index.ts +++ b/packages/nextjs/src/config/index.ts @@ -1 +1,2 @@ export { withSentryConfig } from './withSentryConfig'; +export type { SentryBuildOptions } from './types'; From 88fef5b01a4fdd043a2d0d9787cd524ac4d95085 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 9 Aug 2024 18:20:40 +0200 Subject: [PATCH 06/35] ref: Add external contributor to CHANGELOG.md (#13298) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13296 --------- Co-authored-by: andreiborza <168741329+andreiborza@users.noreply.github.com> Co-authored-by: Andrei Borza --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e8d141efd95..152f75a9169c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +Work in this release was contributed by @MonstraG. Thank you for your contribution! + ## 8.25.0 ### Important Changes From a55e4d5b0077294ee3238693b8a243c8a0acec9f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Aug 2024 11:29:00 +0200 Subject: [PATCH 07/35] test: Fix node-integration-test timeouts & cleanup (#13280) This PR streamlines and fixes some timing/cleanup issues we had in integration tests, which sometimes lead to issues. One problem was introduced here: https://github.com/getsentry/sentry-javascript/pull/13253 This change lead to the server being shut down (because `done` is called) before the HTTP requests are finished, leading to error logs. Another problem was that in some cases we had leaking processes, where we did not properly close servers we started - this was everywhere we used `createTestServer`. I also moved some code from the node-integration-tests package to the remix package, that was only used there (and not properly depended/imported on). For future debugging, this was shown by running tests with `--detectOpenHandles`. --- dev-packages/node-integration-tests/README.md | 8 - .../node-integration-tests/jest.setup.js | 4 +- .../suites/express/multiple-init/server.ts | 8 +- .../suites/express/multiple-init/test.ts | 11 + .../suites/public-api/LocalVariables/test.ts | 4 + .../suites/tracing/connect/scenario.js | 4 +- .../suites/tracing/connect/test.ts | 2 - .../suites/tracing/hapi/scenario.js | 4 +- .../suites/tracing/hapi/test.ts | 2 - .../suites/tracing/mongodb/test.ts | 2 - .../suites/tracing/mongoose/test.ts | 2 - .../suites/tracing/mysql2/test.ts | 3 + .../nestjs-errors-no-express/scenario.ts | 4 +- .../tracing/nestjs-errors-no-express/test.ts | 2 - .../suites/tracing/nestjs-errors/scenario.ts | 4 +- .../suites/tracing/nestjs-errors/test.ts | 2 - .../tracing/nestjs-no-express/scenario.ts | 4 +- .../suites/tracing/nestjs-no-express/test.ts | 2 - .../suites/tracing/nestjs/scenario.ts | 4 +- .../suites/tracing/nestjs/test.ts | 2 - .../suites/tracing/postgres/test.ts | 3 + .../suites/tracing/redis-cache/test.ts | 3 + .../suites/tracing/redis/test.ts | 3 + .../requests/fetch-breadcrumbs/test.ts | 4 +- .../tracing/requests/fetch-no-tracing/test.ts | 4 +- .../fetch-sampled-no-active-span/test.ts | 4 +- .../tracing/requests/fetch-unsampled/test.ts | 4 +- .../tracing/requests/http-breadcrumbs/test.ts | 4 +- .../tracing/requests/http-no-tracing/test.ts | 4 +- .../http-sampled-no-active-span/test.ts | 4 +- .../tracing/requests/http-sampled/test.ts | 4 +- .../tracing/requests/http-unsampled/test.ts | 4 +- .../suites/tracing/spans/test.ts | 4 +- .../tracing/tracePropagationTargets/test.ts | 4 +- .../node-integration-tests/utils/index.ts | 251 +---------------- .../node-integration-tests/utils/runner.ts | 30 +- .../node-integration-tests/utils/server.ts | 22 +- .../integration/test/server/utils/helpers.ts | 259 +++++++++++++++++- 38 files changed, 369 insertions(+), 324 deletions(-) diff --git a/dev-packages/node-integration-tests/README.md b/dev-packages/node-integration-tests/README.md index 35b3c10883b7..ab1ce5e834de 100644 --- a/dev-packages/node-integration-tests/README.md +++ b/dev-packages/node-integration-tests/README.md @@ -38,14 +38,6 @@ requests, and assertions. Test server, interceptors and assertions are all run o `utils/` contains helpers and Sentry-specific assertions that can be used in (`test.ts`). -`TestEnv` class contains methods to create and execute requests on a test server instance. `TestEnv.init()` which starts -a test server and returns a `TestEnv` instance must be called by each test. The test server is automatically shut down -after each test, if a data collection helper method such as `getEnvelopeRequest` and `getAPIResponse` is used. Tests -that do not use those helper methods will need to end the server manually. - -`TestEnv` instance has two public properties: `url` and `server`. The `url` property is the base URL for the server. The -`http.Server` instance is used to finish the server eventually. - Nock interceptors are internally used to capture envelope requests by `getEnvelopeRequest` and `getMultipleEnvelopeRequest` helpers. After capturing required requests, the interceptors are removed. Nock can manually be used inside the test cases to intercept requests but should be removed before the test ends, as not to cause diff --git a/dev-packages/node-integration-tests/jest.setup.js b/dev-packages/node-integration-tests/jest.setup.js index b0c26e5b05f2..88492fc5d945 100644 --- a/dev-packages/node-integration-tests/jest.setup.js +++ b/dev-packages/node-integration-tests/jest.setup.js @@ -1,7 +1,7 @@ const { cleanupChildProcesses } = require('./utils/runner'); -// Increases test timeout from 5s to 45s -jest.setTimeout(45000); +// Default timeout: 15s +jest.setTimeout(15000); afterEach(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts b/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts index 4d1625035ebf..39d56710f043 100644 --- a/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts +++ b/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts @@ -54,15 +54,19 @@ app.get('/test/error/:id', (req, res) => { setTimeout(() => { // We flush to ensure we are sending exceptions in a certain order - Sentry.flush(3000).then( + Sentry.flush(1000).then( () => { + // We send this so we can wait for this, to know the test is ended & server can be closed + if (id === '3') { + Sentry.captureException(new Error('Final exception was captured')); + } res.send({}); }, () => { res.send({}); }, ); - }, 1000); + }, 1); }); Sentry.setupExpressErrorHandler(app); diff --git a/dev-packages/node-integration-tests/suites/express/multiple-init/test.ts b/dev-packages/node-integration-tests/suites/express/multiple-init/test.ts index f654fc361442..b80669a7c432 100644 --- a/dev-packages/node-integration-tests/suites/express/multiple-init/test.ts +++ b/dev-packages/node-integration-tests/suites/express/multiple-init/test.ts @@ -48,6 +48,17 @@ test('allows to call init multiple times', done => { }, }, }) + .expect({ + event: { + exception: { + values: [ + { + value: 'Final exception was captured', + }, + ], + }, + }, + }) .start(done); runner diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts index 7076388b9d29..2640ecf94461 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts @@ -3,6 +3,10 @@ import * as path from 'path'; import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +// This test takes some time because it connects the debugger etc. +// So we increase the timeout here +jest.setTimeout(45_000); + const EXPECTED_LOCAL_VARIABLES_EVENT = { exception: { values: [ diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js b/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js index b4013cd20434..db95fad457b2 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js @@ -13,7 +13,7 @@ Sentry.init({ const connect = require('connect'); const http = require('http'); -const init = async () => { +const run = async () => { const app = connect(); app.use('/', function (req, res, next) { @@ -34,4 +34,4 @@ const init = async () => { sendPortToRunner(port); }; -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts index ad49a4e4532d..dd14c2277f7b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts @@ -1,7 +1,5 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - describe('connect auto-instrumentation', () => { afterAll(async () => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js index 184dcb3b8ea1..f3171eb085e0 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js @@ -13,7 +13,7 @@ const Boom = require('@hapi/boom'); const port = 5999; -const init = async () => { +const run = async () => { const server = Hapi.server({ host: 'localhost', port, @@ -65,4 +65,4 @@ const init = async () => { sendPortToRunner(port); }; -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts index 903b3b0b6fa8..8bb3bfdb0796 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts @@ -1,7 +1,5 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - describe('hapi auto-instrumentation', () => { afterAll(async () => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts b/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts index 2f79385521d3..59c50d32ebdc 100644 --- a/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts @@ -2,8 +2,6 @@ import { MongoMemoryServer } from 'mongodb-memory-server-global'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - describe('MongoDB experimental Test', () => { let mongoServer: MongoMemoryServer; diff --git a/dev-packages/node-integration-tests/suites/tracing/mongoose/test.ts b/dev-packages/node-integration-tests/suites/tracing/mongoose/test.ts index 050a3ffc9e12..30eeb62ffe31 100644 --- a/dev-packages/node-integration-tests/suites/tracing/mongoose/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/mongoose/test.ts @@ -2,8 +2,6 @@ import { MongoMemoryServer } from 'mongodb-memory-server-global'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - describe('Mongoose experimental Test', () => { let mongoServer: MongoMemoryServer; diff --git a/dev-packages/node-integration-tests/suites/tracing/mysql2/test.ts b/dev-packages/node-integration-tests/suites/tracing/mysql2/test.ts index db056fd222e8..60070308c7db 100644 --- a/dev-packages/node-integration-tests/suites/tracing/mysql2/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/mysql2/test.ts @@ -1,5 +1,8 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +// When running docker compose, we need a larger timeout, as this takes some time... +jest.setTimeout(75000); + describe('mysql2 auto instrumentation', () => { afterAll(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/scenario.ts index 295eba00fd9a..51173004b2f8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/scenario.ts @@ -47,7 +47,7 @@ class AppController { }) class AppModule {} -async function init(): Promise { +async function run(): Promise { const app = await NestFactory.create(AppModule); const { httpAdapter } = app.get(HttpAdapterHost); Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter)); @@ -56,4 +56,4 @@ async function init(): Promise { } // eslint-disable-next-line @typescript-eslint/no-floating-promises -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/test.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/test.ts index f38550469446..84b1aeef40c4 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/test.ts @@ -1,8 +1,6 @@ import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - const { TS_VERSION } = process.env; const isOldTS = TS_VERSION && TS_VERSION.startsWith('3.'); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/scenario.ts index 09a59eb8c7c7..11a0bb831c36 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/scenario.ts @@ -45,7 +45,7 @@ class AppController { }) class AppModule {} -async function init(): Promise { +async function run(): Promise { const app = await NestFactory.create(AppModule); const { httpAdapter } = app.get(HttpAdapterHost); Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter)); @@ -54,4 +54,4 @@ async function init(): Promise { } // eslint-disable-next-line @typescript-eslint/no-floating-promises -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/test.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/test.ts index 264cbe1482cc..1b366307eac6 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/test.ts @@ -1,8 +1,6 @@ import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - const { TS_VERSION } = process.env; const isOldTS = TS_VERSION && TS_VERSION.startsWith('3.'); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/scenario.ts index 209f517193dc..b6a6e4c0dca7 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/scenario.ts @@ -47,7 +47,7 @@ class AppController { }) class AppModule {} -async function init(): Promise { +async function run(): Promise { const app = await NestFactory.create(AppModule); const { httpAdapter } = app.get(HttpAdapterHost); Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter)); @@ -56,4 +56,4 @@ async function init(): Promise { } // eslint-disable-next-line @typescript-eslint/no-floating-promises -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/test.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/test.ts index 70eb9e9aaa26..59532ef989da 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/test.ts @@ -1,8 +1,6 @@ import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - const { TS_VERSION } = process.env; const isOldTS = TS_VERSION && TS_VERSION.startsWith('3.'); 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 19ec6c04c3e3..b75ff4d8a9ef 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts @@ -45,11 +45,11 @@ class AppController { }) class AppModule {} -async function init(): Promise { +async function run(): Promise { const app = await NestFactory.create(AppModule); await app.listen(port); sendPortToRunner(port); } // eslint-disable-next-line @typescript-eslint/no-floating-promises -init(); +run(); 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 2b42f23c857a..686c93e1cad6 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts @@ -1,8 +1,6 @@ import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - const { TS_VERSION } = process.env; const isOldTS = TS_VERSION && TS_VERSION.startsWith('3.'); diff --git a/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts b/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts index d83f992638d1..f2549c70eb90 100644 --- a/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts @@ -1,5 +1,8 @@ import { createRunner } from '../../../utils/runner'; +// When running docker compose, we need a larger timeout, as this takes some time... +jest.setTimeout(75000); + describe('postgres auto instrumentation', () => { test('should auto-instrument `pg` package', done => { const EXPECTED_TRANSACTION = { diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts index 0c0807c8f480..8d494986ab3b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts @@ -1,5 +1,8 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +// When running docker compose, we need a larger timeout, as this takes some time... +jest.setTimeout(75000); + describe('redis cache auto instrumentation', () => { afterAll(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/redis/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis/test.ts index f68c14499a13..5a801a2f6a5b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis/test.ts @@ -1,5 +1,8 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +// When running docker compose, we need a larger timeout, as this takes some time... +jest.setTimeout(75000); + describe('redis auto instrumentation', () => { afterAll(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts index 2c238c9ecd15..c0d783aaa594 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts @@ -6,7 +6,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { test('outgoing fetch requests create breadcrumbs', done => { createTestServer(done) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .ensureNoErrorOutput() @@ -72,7 +72,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { }, }, }) - .start(done); + .start(closeTestServer); }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing/test.ts index f9ad7f92d3f1..9c732d899cde 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing/test.ts @@ -26,7 +26,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .ensureNoErrorOutput() @@ -42,7 +42,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { }, }, }) - .start(done); + .start(closeTestServer); }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled-no-active-span/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled-no-active-span/test.ts index 8b9ff1486565..fde1c787829a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled-no-active-span/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled-no-active-span/test.ts @@ -26,7 +26,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -41,7 +41,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { }, }, }) - .start(done); + .start(closeTestServer); }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/test.ts index 016881f47399..d288e9a03fbf 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/test.ts @@ -26,7 +26,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -41,7 +41,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { }, }, }) - .start(done); + .start(closeTestServer); }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/test.ts index aab8338d0a35..812dbbb4ae60 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/test.ts @@ -4,7 +4,7 @@ import { createTestServer } from '../../../../utils/server'; test('outgoing http requests create breadcrumbs', done => { createTestServer(done) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .ensureNoErrorOutput() @@ -70,6 +70,6 @@ test('outgoing http requests create breadcrumbs', done => { }, }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts index 308e0c6676e2..d0b570625c2b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts @@ -24,7 +24,7 @@ test('outgoing http requests are correctly instrumented with tracing disabled', expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .ensureNoErrorOutput() @@ -40,6 +40,6 @@ test('outgoing http requests are correctly instrumented with tracing disabled', }, }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-no-active-span/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-no-active-span/test.ts index 83d8774dbd46..2baff11a5faf 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-no-active-span/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-no-active-span/test.ts @@ -24,7 +24,7 @@ test('outgoing sampled http requests without active span are correctly instrumen expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -39,6 +39,6 @@ test('outgoing sampled http requests without active span are correctly instrumen }, }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/test.ts index fd939bc4458c..38dfa6524019 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/test.ts @@ -24,7 +24,7 @@ test('outgoing sampled http requests are correctly instrumented', done => { expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -32,6 +32,6 @@ test('outgoing sampled http requests are correctly instrumented', done => { // we're not too concerned with the actual transaction here since this is tested elsewhere }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts index ed5d30631f31..3d2e0e421863 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts @@ -24,7 +24,7 @@ test('outgoing http requests are correctly instrumented when not sampled', done expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -39,6 +39,6 @@ test('outgoing http requests are correctly instrumented when not sampled', done }, }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/spans/test.ts index 7def81fbe952..e349622d39f8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/spans/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/spans/test.ts @@ -18,7 +18,7 @@ test('should capture spans for outgoing http requests', done => { 404, ) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -43,6 +43,6 @@ test('should capture spans for outgoing http requests', done => { ]), }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/test.ts b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/test.ts index c43b5607ef52..e6081bedd8ea 100644 --- a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/test.ts @@ -24,7 +24,7 @@ test('HttpIntegration should instrument correct requests when tracePropagationTa expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -32,6 +32,6 @@ test('HttpIntegration should instrument correct requests when tracePropagationTa // we're not too concerned with the actual transaction here since this is tested elsewhere }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/utils/index.ts b/dev-packages/node-integration-tests/utils/index.ts index 52223f4417a2..8c12ec72e0d2 100644 --- a/dev-packages/node-integration-tests/utils/index.ts +++ b/dev-packages/node-integration-tests/utils/index.ts @@ -1,16 +1,6 @@ -import * as http from 'http'; -import type { AddressInfo } from 'net'; -import * as path from 'path'; -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ -import * as Sentry from '@sentry/node'; +import type * as http from 'http'; import type { EnvelopeItemType } from '@sentry/types'; -import { logger, parseSemver } from '@sentry/utils'; -import type { AxiosRequestConfig } from 'axios'; -import axios from 'axios'; -import type { Express } from 'express'; -import type { HttpTerminator } from 'http-terminator'; -import { createHttpTerminator } from 'http-terminator'; -import nock from 'nock'; +import { parseSemver } from '@sentry/utils'; const NODE_VERSION = parseSemver(process.versions.node).major; @@ -93,240 +83,3 @@ export const assertSentryTransaction = (actual: Record, expecte export const parseEnvelope = (body: string): Array> => { return body.split('\n').map(e => JSON.parse(e)); }; - -/** - * Sends a get request to given URL. - * Flushes the Sentry event queue. - * - * @param {string} url - * @return {*} {Promise} - */ -export async function runScenario(url: string): Promise { - await axios.get(url); - await Sentry.flush(); -} - -async function makeRequest( - method: 'get' | 'post' = 'get', - url: string, - axiosConfig?: AxiosRequestConfig, -): Promise { - try { - if (method === 'get') { - await axios.get(url, axiosConfig); - } else { - await axios.post(url, axiosConfig); - } - } catch (e) { - // We sometimes expect the request to fail, but not the test. - // So, we do nothing. - logger.warn(e); - } -} - -export class TestEnv { - private _axiosConfig: AxiosRequestConfig | undefined = undefined; - private _terminator: HttpTerminator; - - public constructor(public readonly server: http.Server, public readonly url: string) { - this.server = server; - this.url = url; - this._terminator = createHttpTerminator({ server: this.server, gracefulTerminationTimeout: 0 }); - } - - /** - * Starts a test server and returns the TestEnv instance - * - * @param {string} testDir - * @param {string} [serverPath] - * @param {string} [scenarioPath] - * @return {*} {Promise} - */ - public static async init(testDir: string, serverPath?: string, scenarioPath?: string): Promise { - const defaultServerPath = path.resolve(process.cwd(), 'utils', 'defaults', 'server'); - - const [server, url] = await new Promise<[http.Server, string]>(resolve => { - // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-unsafe-member-access - const app = require(serverPath || defaultServerPath).default as Express; - - app.get('/test', (_req, res) => { - try { - require(scenarioPath || `${testDir}/scenario`); - } finally { - res.status(200).end(); - } - }); - - const server = app.listen(0, () => { - const url = `http://localhost:${(server.address() as AddressInfo).port}/test`; - resolve([server, url]); - }); - }); - - return new TestEnv(server, url); - } - - /** - * Intercepts and extracts up to a number of requests containing Sentry envelopes. - * - * @param {DataCollectorOptions} options - * @returns The intercepted envelopes. - */ - public async getMultipleEnvelopeRequest(options: DataCollectorOptions): Promise[][]> { - const envelopeTypeArray = - typeof options.envelopeType === 'string' - ? [options.envelopeType] - : options.envelopeType || (['event'] as EnvelopeItemType[]); - - const resProm = this.setupNock( - options.count || 1, - typeof options.endServer === 'undefined' ? true : options.endServer, - envelopeTypeArray, - ); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - makeRequest(options.method, options.url || this.url, this._axiosConfig); - return resProm; - } - - /** - * Intercepts and extracts a single request containing a Sentry envelope - * - * @param {DataCollectorOptions} options - * @returns The extracted envelope. - */ - public async getEnvelopeRequest(options?: DataCollectorOptions): Promise>> { - const requests = await this.getMultipleEnvelopeRequest({ ...options, count: 1 }); - - if (!requests[0]) { - throw new Error('No requests found'); - } - - return requests[0]; - } - - /** - * Sends a get request to given URL, with optional headers. Returns the response. - * Ends the server instance and flushes the Sentry event queue. - * - * @param {Record} [headers] - * @return {*} {Promise} - */ - public async getAPIResponse( - url?: string, - headers: Record = {}, - endServer: boolean = true, - ): Promise { - try { - const { data } = await axios.get(url || this.url, { - headers, - // KeepAlive false to work around a Node 20 bug with ECONNRESET: https://github.com/axios/axios/issues/5929 - httpAgent: new http.Agent({ keepAlive: false }), - }); - return data; - } finally { - await Sentry.flush(); - - if (endServer) { - this.server.close(); - } - } - } - - public async setupNock( - count: number, - endServer: boolean, - envelopeType: EnvelopeItemType[], - ): Promise[][]> { - return new Promise(resolve => { - const envelopes: Record[][] = []; - const mock = nock('https://dsn.ingest.sentry.io') - .persist() - .post('/api/1337/envelope/', body => { - const envelope = parseEnvelope(body); - - if (envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) { - envelopes.push(envelope); - } else { - return false; - } - - if (count === envelopes.length) { - nock.removeInterceptor(mock); - - if (endServer) { - // Cleaning nock only before the server is closed, - // not to break tests that use simultaneous requests to the server. - // Ex: Remix scope bleed tests. - nock.cleanAll(); - - // Abort all pending requests to nock to prevent hanging / flakes. - // See: https://github.com/nock/nock/issues/1118#issuecomment-544126948 - nock.abortPendingRequests(); - - this._closeServer() - .catch(e => { - logger.warn(e); - }) - .finally(() => { - resolve(envelopes); - }); - } else { - resolve(envelopes); - } - } - - return true; - }); - - mock - .query(true) // accept any query params - used for sentry_key param - .reply(200); - }); - } - - public setAxiosConfig(axiosConfig: AxiosRequestConfig): void { - this._axiosConfig = axiosConfig; - } - - public async countEnvelopes(options: { - url?: string; - timeout?: number; - envelopeType: EnvelopeItemType | EnvelopeItemType[]; - }): Promise { - return new Promise(resolve => { - let reqCount = 0; - - const mock = nock('https://dsn.ingest.sentry.io') - .persist() - .post('/api/1337/envelope/', body => { - const envelope = parseEnvelope(body); - - if (options.envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) { - reqCount++; - return true; - } - - return false; - }); - - setTimeout( - () => { - nock.removeInterceptor(mock); - - nock.cleanAll(); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this._closeServer().then(() => { - resolve(reqCount); - }); - }, - options.timeout || 1000, - ); - }); - } - - private _closeServer(): Promise { - return this._terminator.terminate(); - } -} diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index ae0451f0d576..cb4ab58347e7 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -110,7 +110,10 @@ async function runDockerCompose(options: DockerOptions): Promise { return new Promise((resolve, reject) => { const cwd = join(...options.workingDirectory); const close = (): void => { - spawnSync('docker', ['compose', 'down', '--volumes'], { cwd }); + spawnSync('docker', ['compose', 'down', '--volumes'], { + cwd, + stdio: process.env.DEBUG ? 'inherit' : undefined, + }); }; // ensure we're starting fresh @@ -126,6 +129,9 @@ async function runDockerCompose(options: DockerOptions): Promise { function newData(data: Buffer): void { const text = data.toString('utf8'); + // eslint-disable-next-line no-console + if (process.env.DEBUG) console.log(text); + for (const match of options.readyMatches) { if (text.includes(match)) { child.stdout.removeAllListeners(); @@ -359,19 +365,29 @@ export function createRunner(...paths: string[]) { } } - const serverStartup: Promise = withSentryServer + // We need to properly define & pass these types around for TS 3.8, + // which otherwise fails to infer these correctly :( + type ServerStartup = [number | undefined, (() => void) | undefined]; + type DockerStartup = VoidFunction | undefined; + + const serverStartup: Promise = withSentryServer ? createBasicSentryServer(newEnvelope) - : Promise.resolve(undefined); + : Promise.resolve([undefined, undefined]); - const dockerStartup: Promise = dockerOptions + const dockerStartup: Promise = dockerOptions ? runDockerCompose(dockerOptions) : Promise.resolve(undefined); - const startup = Promise.all([dockerStartup, serverStartup]); + const startup = Promise.all([dockerStartup, serverStartup]) as Promise<[DockerStartup, ServerStartup]>; - // eslint-disable-next-line @typescript-eslint/no-floating-promises startup - .then(([dockerChild, mockServerPort]) => { + .then(([dockerChild, [mockServerPort, mockServerClose]]) => { + if (mockServerClose) { + CLEANUP_STEPS.add(() => { + mockServerClose(); + }); + } + if (dockerChild) { CLEANUP_STEPS.add(dockerChild); } diff --git a/dev-packages/node-integration-tests/utils/server.ts b/dev-packages/node-integration-tests/utils/server.ts index f68f1a9c80d4..71a7adf9798f 100644 --- a/dev-packages/node-integration-tests/utils/server.ts +++ b/dev-packages/node-integration-tests/utils/server.ts @@ -9,8 +9,9 @@ import express from 'express'; * This does no checks on the envelope, it just calls the callback if it managed to parse an envelope from the raw POST * body data. */ -export function createBasicSentryServer(onEnvelope: (env: Envelope) => void): Promise { +export function createBasicSentryServer(onEnvelope: (env: Envelope) => void): Promise<[number, () => void]> { const app = express(); + app.use(express.raw({ type: () => true, inflate: true, limit: '100mb' })); app.post('/api/:id/envelope/', (req, res) => { try { @@ -27,7 +28,12 @@ export function createBasicSentryServer(onEnvelope: (env: Envelope) => void): Pr return new Promise(resolve => { const server = app.listen(0, () => { const address = server.address() as AddressInfo; - resolve(address.port); + resolve([ + address.port, + () => { + server.close(); + }, + ]); }); }); } @@ -36,7 +42,7 @@ type HeaderAssertCallback = (headers: Record void) { +export function createTestServer(done: (error?: unknown) => void) { const gets: Array<[string, HeaderAssertCallback, number]> = []; return { @@ -44,7 +50,7 @@ export function createTestServer(done: (error: unknown) => void) { gets.push([path, callback, result]); return this; }, - start: async (): Promise => { + start: async (): Promise<[string, () => void]> => { const app = express(); for (const [path, callback, result] of gets) { @@ -62,7 +68,13 @@ export function createTestServer(done: (error: unknown) => void) { return new Promise(resolve => { const server = app.listen(0, () => { const address = server.address() as AddressInfo; - resolve(`http://localhost:${address.port}`); + resolve([ + `http://localhost:${address.port}`, + () => { + server.close(); + done(); + }, + ]); }); }); }, diff --git a/packages/remix/test/integration/test/server/utils/helpers.ts b/packages/remix/test/integration/test/server/utils/helpers.ts index eccda209fb48..981be12f314a 100644 --- a/packages/remix/test/integration/test/server/utils/helpers.ts +++ b/packages/remix/test/integration/test/server/utils/helpers.ts @@ -1,11 +1,264 @@ import * as http from 'http'; import { AddressInfo } from 'net'; +import * as path from 'path'; import { createRequestHandler } from '@remix-run/express'; +/* eslint-disable @typescript-eslint/no-unsafe-member-access */ +import * as Sentry from '@sentry/node'; +import type { EnvelopeItemType } from '@sentry/types'; +import { logger } from '@sentry/utils'; +import type { AxiosRequestConfig } from 'axios'; +import axios from 'axios'; import express from 'express'; -import { TestEnv } from '../../../../../../../dev-packages/node-integration-tests/utils'; +import type { Express } from 'express'; +import type { HttpTerminator } from 'http-terminator'; +import { createHttpTerminator } from 'http-terminator'; +import nock from 'nock'; export * from '../../../../../../../dev-packages/node-integration-tests/utils'; +type DataCollectorOptions = { + // Optional custom URL + url?: string; + + // The expected amount of requests to the envelope endpoint. + // If the amount of sent requests is lower than `count`, this function will not resolve. + count?: number; + + // The method of the request. + method?: 'get' | 'post'; + + // Whether to stop the server after the requests have been intercepted + endServer?: boolean; + + // Type(s) of the envelopes to capture + envelopeType?: EnvelopeItemType | EnvelopeItemType[]; +}; + +async function makeRequest( + method: 'get' | 'post' = 'get', + url: string, + axiosConfig?: AxiosRequestConfig, +): Promise { + try { + if (method === 'get') { + await axios.get(url, axiosConfig); + } else { + await axios.post(url, axiosConfig); + } + } catch (e) { + // We sometimes expect the request to fail, but not the test. + // So, we do nothing. + logger.warn(e); + } +} + +class TestEnv { + private _axiosConfig: AxiosRequestConfig | undefined = undefined; + private _terminator: HttpTerminator; + + public constructor(public readonly server: http.Server, public readonly url: string) { + this.server = server; + this.url = url; + this._terminator = createHttpTerminator({ server: this.server, gracefulTerminationTimeout: 0 }); + } + + /** + * Starts a test server and returns the TestEnv instance + * + * @param {string} testDir + * @param {string} [serverPath] + * @param {string} [scenarioPath] + * @return {*} {Promise} + */ + public static async init(testDir: string, serverPath?: string, scenarioPath?: string): Promise { + const defaultServerPath = path.resolve(process.cwd(), 'utils', 'defaults', 'server'); + + const [server, url] = await new Promise<[http.Server, string]>(resolve => { + // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-unsafe-member-access + const app = require(serverPath || defaultServerPath).default as Express; + + app.get('/test', (_req, res) => { + try { + require(scenarioPath || `${testDir}/scenario`); + } finally { + res.status(200).end(); + } + }); + + const server = app.listen(0, () => { + const url = `http://localhost:${(server.address() as AddressInfo).port}/test`; + resolve([server, url]); + }); + }); + + return new TestEnv(server, url); + } + + /** + * Intercepts and extracts up to a number of requests containing Sentry envelopes. + * + * @param {DataCollectorOptions} options + * @returns The intercepted envelopes. + */ + public async getMultipleEnvelopeRequest(options: DataCollectorOptions): Promise[][]> { + const envelopeTypeArray = + typeof options.envelopeType === 'string' + ? [options.envelopeType] + : options.envelopeType || (['event'] as EnvelopeItemType[]); + + const resProm = this.setupNock( + options.count || 1, + typeof options.endServer === 'undefined' ? true : options.endServer, + envelopeTypeArray, + ); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + makeRequest(options.method, options.url || this.url, this._axiosConfig); + return resProm; + } + + /** + * Intercepts and extracts a single request containing a Sentry envelope + * + * @param {DataCollectorOptions} options + * @returns The extracted envelope. + */ + public async getEnvelopeRequest(options?: DataCollectorOptions): Promise>> { + const requests = await this.getMultipleEnvelopeRequest({ ...options, count: 1 }); + + if (!requests[0]) { + throw new Error('No requests found'); + } + + return requests[0]; + } + + /** + * Sends a get request to given URL, with optional headers. Returns the response. + * Ends the server instance and flushes the Sentry event queue. + * + * @param {Record} [headers] + * @return {*} {Promise} + */ + public async getAPIResponse( + url?: string, + headers: Record = {}, + endServer: boolean = true, + ): Promise { + try { + const { data } = await axios.get(url || this.url, { + headers, + // KeepAlive false to work around a Node 20 bug with ECONNRESET: https://github.com/axios/axios/issues/5929 + httpAgent: new http.Agent({ keepAlive: false }), + }); + return data; + } finally { + await Sentry.flush(); + + if (endServer) { + this.server.close(); + } + } + } + + public async setupNock( + count: number, + endServer: boolean, + envelopeType: EnvelopeItemType[], + ): Promise[][]> { + return new Promise(resolve => { + const envelopes: Record[][] = []; + const mock = nock('https://dsn.ingest.sentry.io') + .persist() + .post('/api/1337/envelope/', body => { + const envelope = parseEnvelope(body); + + if (envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) { + envelopes.push(envelope); + } else { + return false; + } + + if (count === envelopes.length) { + nock.removeInterceptor(mock); + + if (endServer) { + // Cleaning nock only before the server is closed, + // not to break tests that use simultaneous requests to the server. + // Ex: Remix scope bleed tests. + nock.cleanAll(); + + // Abort all pending requests to nock to prevent hanging / flakes. + // See: https://github.com/nock/nock/issues/1118#issuecomment-544126948 + nock.abortPendingRequests(); + + this._closeServer() + .catch(e => { + logger.warn(e); + }) + .finally(() => { + resolve(envelopes); + }); + } else { + resolve(envelopes); + } + } + + return true; + }); + + mock + .query(true) // accept any query params - used for sentry_key param + .reply(200); + }); + } + + public setAxiosConfig(axiosConfig: AxiosRequestConfig): void { + this._axiosConfig = axiosConfig; + } + + public async countEnvelopes(options: { + url?: string; + timeout?: number; + envelopeType: EnvelopeItemType | EnvelopeItemType[]; + }): Promise { + return new Promise(resolve => { + let reqCount = 0; + + const mock = nock('https://dsn.ingest.sentry.io') + .persist() + .post('/api/1337/envelope/', body => { + const envelope = parseEnvelope(body); + + if (options.envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) { + reqCount++; + return true; + } + + return false; + }); + + setTimeout( + () => { + nock.removeInterceptor(mock); + + nock.cleanAll(); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._closeServer().then(() => { + resolve(reqCount); + }); + }, + options.timeout || 1000, + ); + }); + } + + private _closeServer(): Promise { + return this._terminator.terminate(); + } +} + export class RemixTestEnv extends TestEnv { private constructor(public readonly server: http.Server, public readonly url: string) { super(server, url); @@ -27,3 +280,7 @@ export class RemixTestEnv extends TestEnv { return new RemixTestEnv(server, `http://localhost:${serverPort}`); } } + +const parseEnvelope = (body: string): Array> => { + return body.split('\n').map(e => JSON.parse(e)); +}; From b29d771f65bdeae81ff29b4213d8e835445a470a Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 12 Aug 2024 11:30:55 +0200 Subject: [PATCH 08/35] feat(nuxt): Always add tracing meta tags (#13273) Making sure tracing without performance works. ref (SvelteKit): https://github.com/getsentry/sentry-javascript/pull/13231 --- packages/nuxt/src/runtime/utils.ts | 17 ++--- .../nuxt/test/runtime/plugins/server.test.ts | 65 +++++-------------- 2 files changed, 21 insertions(+), 61 deletions(-) diff --git a/packages/nuxt/src/runtime/utils.ts b/packages/nuxt/src/runtime/utils.ts index b7c038ddd505..585387f59003 100644 --- a/packages/nuxt/src/runtime/utils.ts +++ b/packages/nuxt/src/runtime/utils.ts @@ -1,8 +1,6 @@ -import { getActiveSpan, getRootSpan, spanToTraceHeader } from '@sentry/core'; -import { getDynamicSamplingContextFromSpan } from '@sentry/opentelemetry'; +import { getTraceMetaTags } from '@sentry/core'; import type { Context } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; -import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import type { CapturedErrorContext } from 'nitropack'; import type { NuxtRenderHTMLContext } from 'nuxt/app'; @@ -37,16 +35,9 @@ export function extractErrorContext(errorContext: CapturedErrorContext): Context * Exported only for testing */ export function addSentryTracingMetaTags(head: NuxtRenderHTMLContext['head']): void { - const activeSpan = getActiveSpan(); - const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; + const metaTags = getTraceMetaTags(); - if (rootSpan) { - const traceParentData = spanToTraceHeader(rootSpan); - const dynamicSamplingContext = dynamicSamplingContextToSentryBaggageHeader( - getDynamicSamplingContextFromSpan(rootSpan), - ); - - head.push(``); - head.push(``); + if (metaTags) { + head.push(metaTags); } } diff --git a/packages/nuxt/test/runtime/plugins/server.test.ts b/packages/nuxt/test/runtime/plugins/server.test.ts index 518b20026cbd..5750f0f9495f 100644 --- a/packages/nuxt/test/runtime/plugins/server.test.ts +++ b/packages/nuxt/test/runtime/plugins/server.test.ts @@ -1,64 +1,33 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { getTraceMetaTags } from '@sentry/core'; +import { type Mock, afterEach, describe, expect, it, vi } from 'vitest'; import { addSentryTracingMetaTags } from '../../../src/runtime/utils'; -const mockReturns = vi.hoisted(() => { - return { - traceHeader: 'trace-header', - baggageHeader: 'baggage-header', - }; -}); - -vi.mock('@sentry/core', async () => { - const actual = await vi.importActual('@sentry/core'); - - return { - ...actual, - getActiveSpan: vi.fn().mockReturnValue({ spanId: '123' }), - getRootSpan: vi.fn().mockReturnValue({ spanId: 'root123' }), - spanToTraceHeader: vi.fn(() => mockReturns.traceHeader), - }; -}); - -vi.mock('@sentry/opentelemetry', async () => { - const actual = await vi.importActual('@sentry/opentelemetry'); - - return { - ...actual, - getDynamicSamplingContextFromSpan: vi.fn().mockReturnValue('contextValue'), - }; -}); - -vi.mock('@sentry/utils', async () => { - const actual = await vi.importActual('@sentry/utils'); - - return { - ...actual, - dynamicSamplingContextToSentryBaggageHeader: vi.fn().mockReturnValue(mockReturns.baggageHeader), - }; -}); +vi.mock('@sentry/core', () => ({ + getTraceMetaTags: vi.fn(), +})); describe('addSentryTracingMetaTags', () => { afterEach(() => { vi.resetAllMocks(); }); - it('should add meta tags when there is an active root span', () => { + it('should add meta tags to the head array', () => { + const mockMetaTags = [ + '', + '', + ].join('\n'); + + // return value is mocked here as return values of `getTraceMetaTags` are tested separately (in @sentry/core) + (getTraceMetaTags as Mock).mockReturnValue(mockMetaTags); + const head: string[] = []; addSentryTracingMetaTags(head); - expect(head).toContain(``); - expect(head).toContain(``); + expect(head).toContain(mockMetaTags); }); - it('should not add meta tags when there is no active root span', () => { - vi.doMock('@sentry/core', async () => { - const actual = await vi.importActual('@sentry/core'); - - return { - ...actual, - getActiveSpan: vi.fn().mockReturnValue(undefined), - }; - }); + it('should handle empty meta tags', () => { + (getTraceMetaTags as Mock).mockReturnValue(''); const head: string[] = []; addSentryTracingMetaTags(head); From 16996bbb62b5c5dd90bb662f09089a3f06ddf0f0 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Aug 2024 12:47:27 +0200 Subject: [PATCH 09/35] 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 10/35] 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 11/35] 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 12/35] 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 13/35] 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 14/35] 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 15/35] 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 16/35] 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 17/35] 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 18/35] 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 19/35] 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 20/35] 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 21/35] 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 22/35] 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 23/35] 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 24/35] 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 25/35] 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__; }); }); }); From 431164432b7ddca6a3c5a18906ded6660a147275 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 13 Aug 2024 12:58:14 +0200 Subject: [PATCH 26/35] ref: Add external contributor to CHANGELOG.md (#13336) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13209 --- Co-authored-by: Lukas Stracke --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f8110348cd6..931d1462da47 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 and @Zen-cronic. Thank you for your contributions! +Work in this release was contributed by @MonstraG, @undead-voron and @Zen-cronic. Thank you for your contributions! ## 8.25.0 From 5af8eb8c3a9028cbd541a23eb08ef49236a5a550 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 13 Aug 2024 13:00:48 +0200 Subject: [PATCH 27/35] feat(browser): Send CLS as standalone span (experimental) (#13056) Add an experimental feature to `browserTracingIntegration` to no longer tie CLS reporting to the ongoing pageload span but instead send a standalone CLS span similarly to how we report INP. The big advantage of this reporting strategy is that layout shifts happening after the pageload idle span ended, will also get reported. This should give users more accurate CLS values in the web vitals performance insights module. --- .size-limit.js | 8 +- .../web-vitals-cls-standalone-spans/init.js | 17 + .../subject.js | 17 + .../template.html | 12 + .../web-vitals-cls-standalone-spans/test.ts | 455 ++++++++++++++++++ .../src/metrics/browserMetrics.ts | 50 +- packages/browser-utils/src/metrics/cls.ts | 122 +++++ packages/browser-utils/src/metrics/inp.ts | 57 +-- packages/browser-utils/src/metrics/utils.ts | 82 +++- .../src/metrics/web-vitals/README.md | 6 +- .../src/tracing/browserTracingIntegration.ts | 8 +- 11 files changed, 760 insertions(+), 74 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts create mode 100644 packages/browser-utils/src/metrics/cls.ts diff --git a/.size-limit.js b/.size-limit.js index 80aa4c5095ea..859ce741cc3d 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -55,7 +55,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'feedbackIntegration'), gzip: true, - limit: '90 KB', + limit: '91 KB', }, { name: '@sentry/browser (incl. Tracing, Replay, Feedback, metrics)', @@ -143,7 +143,7 @@ module.exports = [ name: 'CDN Bundle (incl. Tracing)', path: createCDNPath('bundle.tracing.min.js'), gzip: true, - limit: '37 KB', + limit: '38 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay)', @@ -170,7 +170,7 @@ module.exports = [ path: createCDNPath('bundle.tracing.min.js'), gzip: false, brotli: false, - limit: '110 KB', + limit: '111 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay) - uncompressed', @@ -193,7 +193,7 @@ module.exports = [ import: createImport('init'), ignore: ['next/router', 'next/constants'], gzip: true, - limit: '38.05 KB', + limit: '39 KB', }, // SvelteKit SDK (ESM) { diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/init.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/init.js new file mode 100644 index 000000000000..32fbb07fbbae --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/init.js @@ -0,0 +1,17 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + Sentry.browserTracingIntegration({ + idleTimeout: 9000, + _experiments: { + enableStandaloneClsSpans: true, + }, + }), + ], + tracesSampleRate: 1, + debug: true, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/subject.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/subject.js new file mode 100644 index 000000000000..ed1b9b790bb9 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/subject.js @@ -0,0 +1,17 @@ +import { simulateCLS } from '../../../../utils/web-vitals/cls.ts'; + +// Simulate Layout shift right at the beginning of the page load, depending on the URL hash +// don't run if expected CLS is NaN +const expectedCLS = Number(location.hash.slice(1)); +if (expectedCLS && expectedCLS >= 0) { + simulateCLS(expectedCLS).then(() => window.dispatchEvent(new Event('cls-done'))); +} + +// Simulate layout shift whenever the trigger-cls event is dispatched +// Cannot trigger cia a button click because expected layout shift after +// an interaction doesn't contribute to CLS. +window.addEventListener('trigger-cls', () => { + simulateCLS(0.1).then(() => { + window.dispatchEvent(new Event('cls-done')); + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html new file mode 100644 index 000000000000..487683893a7f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html @@ -0,0 +1,12 @@ + + + + + + +
+

+ Some content +

+ + diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts new file mode 100644 index 000000000000..cdf1e6837ef4 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts @@ -0,0 +1,455 @@ +import type { Page } from '@playwright/test'; +import { expect } from '@playwright/test'; +import type { Event as SentryEvent, EventEnvelope, SpanEnvelope } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { + getFirstSentryEnvelopeRequest, + getMultipleSentryEnvelopeRequests, + properFullEnvelopeRequestParser, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; + +sentryTest.beforeEach(async ({ browserName, page }) => { + if (shouldSkipTracingTest() || browserName !== 'chromium') { + sentryTest.skip(); + } + + await page.setViewportSize({ width: 800, height: 1200 }); +}); + +function waitForLayoutShift(page: Page): Promise { + return page.evaluate(() => { + return new Promise(resolve => { + window.addEventListener('cls-done', () => resolve()); + }); + }); +} + +function triggerAndWaitForLayoutShift(page: Page): Promise { + return page.evaluate(() => { + window.dispatchEvent(new CustomEvent('trigger-cls')); + return new Promise(resolve => { + window.addEventListener('cls-done', () => resolve()); + }); + }); +} + +function hidePage(page: Page): Promise { + return page.evaluate(() => { + window.dispatchEvent(new Event('pagehide')); + }); +} + +sentryTest('captures a "GOOD" CLS vital with its source as a standalone span', async ({ getLocalTestPath, page }) => { + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(`${url}#0.05`); + + await waitForLayoutShift(page); + + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': 0, + 'sentry.op': 'ui.webvital.cls', + 'sentry.origin': 'auto.http.browser.cls', + transaction: expect.stringContaining('index.html'), + 'user_agent.original': expect.stringContaining('Chrome'), + 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), + }, + description: expect.stringContaining('body > div#content > p'), + exclusive_time: 0, + measurements: { + cls: { + unit: '', + value: expect.any(Number), // better check below, + }, + }, + op: 'ui.webvital.cls', + origin: 'auto.http.browser.cls', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + segment_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: spanEnvelopeItem.start_timestamp, + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }); + + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.03); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.07); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: spanEnvelopeItem.trace_id, + // no transaction, because span source is URL + }, + }); +}); + +sentryTest('captures a "MEH" CLS vital with its source as a standalone span', async ({ getLocalTestPath, page }) => { + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(`${url}#0.21`); + + await waitForLayoutShift(page); + + // Page hide to trigger CLS emission + await page.evaluate(() => { + window.dispatchEvent(new Event('pagehide')); + }); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': 0, + 'sentry.op': 'ui.webvital.cls', + 'sentry.origin': 'auto.http.browser.cls', + transaction: expect.stringContaining('index.html'), + 'user_agent.original': expect.stringContaining('Chrome'), + 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), + }, + description: expect.stringContaining('body > div#content > p'), + exclusive_time: 0, + measurements: { + cls: { + unit: '', + value: expect.any(Number), // better check below, + }, + }, + op: 'ui.webvital.cls', + origin: 'auto.http.browser.cls', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + segment_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: spanEnvelopeItem.start_timestamp, + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }); + + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.18); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.23); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: spanEnvelopeItem.trace_id, + // no transaction, because span source is URL + }, + }); +}); + +sentryTest('captures a "POOR" CLS vital with its source as a standalone span.', async ({ getLocalTestPath, page }) => { + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(`${url}#0.35`); + + await waitForLayoutShift(page); + + // Page hide to trigger CLS emission + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': 0, + 'sentry.op': 'ui.webvital.cls', + 'sentry.origin': 'auto.http.browser.cls', + transaction: expect.stringContaining('index.html'), + 'user_agent.original': expect.stringContaining('Chrome'), + 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), + }, + description: expect.stringContaining('body > div#content > p'), + exclusive_time: 0, + measurements: { + cls: { + unit: '', + value: expect.any(Number), // better check below, + }, + }, + op: 'ui.webvital.cls', + origin: 'auto.http.browser.cls', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + segment_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: spanEnvelopeItem.start_timestamp, + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }); + + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.33); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.38); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: spanEnvelopeItem.trace_id, + // no transaction, because span source is URL + }, + }); +}); + +sentryTest( + 'captures a 0 CLS vital as a standalone span if no layout shift occurred', + async ({ getLocalTestPath, page }) => { + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(url); + + await page.waitForTimeout(1000); + + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': 0, + 'sentry.op': 'ui.webvital.cls', + 'sentry.origin': 'auto.http.browser.cls', + transaction: expect.stringContaining('index.html'), + 'user_agent.original': expect.stringContaining('Chrome'), + 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), + }, + description: 'Layout shift', + exclusive_time: 0, + measurements: { + cls: { + unit: '', + value: 0, + }, + }, + op: 'ui.webvital.cls', + origin: 'auto.http.browser.cls', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + segment_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: spanEnvelopeItem.start_timestamp, + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: spanEnvelopeItem.trace_id, + // no transaction, because span source is URL + }, + }); + }, +); + +sentryTest( + 'captures CLS increases after the pageload span ended, when page is hidden', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.type).toBe('transaction'); + expect(eventData.contexts?.trace?.op).toBe('pageload'); + + const pageloadSpanId = eventData.contexts?.trace?.span_id; + const pageloadTraceId = eventData.contexts?.trace?.trace_id; + + expect(pageloadSpanId).toMatch(/[a-f0-9]{16}/); + expect(pageloadTraceId).toMatch(/[a-f0-9]{32}/); + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await triggerAndWaitForLayoutShift(page); + + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.05); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.15); + + // Ensure the CLS span is connected to the pageload span and trace + expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toBe(pageloadSpanId); + expect(spanEnvelopeItem.trace_id).toEqual(pageloadTraceId); + }, +); + +sentryTest('sends CLS of the initial page when soft-navigating to a new page', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.type).toBe('transaction'); + expect(eventData.contexts?.trace?.op).toBe('pageload'); + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await triggerAndWaitForLayoutShift(page); + + await page.goto(`${url}#soft-navigation`); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.05); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.15); + expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toMatch(/[a-f0-9]{16}/); +}); + +sentryTest("doesn't send further CLS after the first navigation", async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.type).toBe('transaction'); + expect(eventData.contexts?.trace?.op).toBe('pageload'); + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await triggerAndWaitForLayoutShift(page); + + await page.goto(`${url}#soft-navigation`); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0); + + getMultipleSentryEnvelopeRequests(page, 1, { envelopeType: 'span' }, () => { + throw new Error('Unexpected span - This should not happen!'); + }); + + const navigationTxnPromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'transaction' }, + properFullEnvelopeRequestParser, + ); + + // activate both CLS emission triggers: + await page.goto(`${url}#soft-navigation-2`); + await hidePage(page); + + // assumption: If we would send another CLS span on the 2nd navigation, it would be sent before the navigation + // transaction ends. This isn't 100% safe to ensure we don't send something but otherwise we'd need to wait for + // a timeout or something similar. + await navigationTxnPromise; +}); + +sentryTest("doesn't send further CLS after the first page hide", async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.type).toBe('transaction'); + expect(eventData.contexts?.trace?.op).toBe('pageload'); + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await triggerAndWaitForLayoutShift(page); + + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0); + + getMultipleSentryEnvelopeRequests(page, 1, { envelopeType: 'span' }, () => { + throw new Error('Unexpected span - This should not happen!'); + }); + + const navigationTxnPromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'transaction' }, + properFullEnvelopeRequestParser, + ); + + // activate both CLS emission triggers: + await page.goto(`${url}#soft-navigation-2`); + await hidePage(page); + + // assumption: If we would send another CLS span on the 2nd navigation, it would be sent before the navigation + // transaction ends. This isn't 100% safe to ensure we don't send something but otherwise we'd need to wait for + // a timeout or something similar. + await navigationTxnPromise; +}); diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index 43ea45dd4a08..b71f80df1ff2 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -7,6 +7,7 @@ import { browserPerformanceTimeOrigin, getComponentName, htmlTreeAsString, logge import { spanToJSON } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; import { WINDOW } from '../types'; +import { trackClsAsStandaloneSpan } from './cls'; import { type PerformanceLongAnimationFrameTiming, addClsInstrumentationHandler, @@ -65,29 +66,33 @@ let _measurements: Measurements = {}; let _lcpEntry: LargestContentfulPaint | undefined; let _clsEntry: LayoutShift | undefined; +interface StartTrackingWebVitalsOptions { + recordClsStandaloneSpans: boolean; +} + /** * Start tracking web vitals. * The callback returned by this function can be used to stop tracking & ensure all measurements are final & captured. * * @returns A function that forces web vitals collection */ -export function startTrackingWebVitals(): () => void { +export function startTrackingWebVitals({ recordClsStandaloneSpans }: StartTrackingWebVitalsOptions): () => void { const performance = getBrowserPerformanceAPI(); if (performance && browserPerformanceTimeOrigin) { // @ts-expect-error we want to make sure all of these are available, even if TS is sure they are if (performance.mark) { WINDOW.performance.mark('sentry-tracing-init'); } - const fidCallback = _trackFID(); - const clsCallback = _trackCLS(); - const lcpCallback = _trackLCP(); - const ttfbCallback = _trackTtfb(); + const fidCleanupCallback = _trackFID(); + const lcpCleanupCallback = _trackLCP(); + const ttfbCleanupCallback = _trackTtfb(); + const clsCleanupCallback = recordClsStandaloneSpans ? trackClsAsStandaloneSpan() : _trackCLS(); return (): void => { - fidCallback(); - clsCallback(); - lcpCallback(); - ttfbCallback(); + fidCleanupCallback(); + lcpCleanupCallback(); + ttfbCleanupCallback(); + clsCleanupCallback && clsCleanupCallback(); }; } @@ -211,17 +216,19 @@ export function startTrackingInteractions(): void { export { startTrackingINP, registerInpInteractionListener } from './inp'; -/** Starts tracking the Cumulative Layout Shift on the current page. */ +/** + * Starts tracking the Cumulative Layout Shift on the current page and collects the value and last entry + * to the `_measurements` object which ultimately is applied to the pageload span's measurements. + */ function _trackCLS(): () => void { return addClsInstrumentationHandler(({ metric }) => { - const entry = metric.entries[metric.entries.length - 1]; + const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined; if (!entry) { return; } - - DEBUG_BUILD && logger.log('[Measurements] Adding CLS'); + DEBUG_BUILD && logger.log(`[Measurements] Adding CLS ${metric.value}`); _measurements['cls'] = { value: metric.value, unit: '' }; - _clsEntry = entry as LayoutShift; + _clsEntry = entry; }, true); } @@ -267,8 +274,16 @@ function _trackTtfb(): () => void { }); } +interface AddPerformanceEntriesOptions { + /** + * Flag to determine if CLS should be recorded as a measurement on the span or + * sent as a standalone span instead. + */ + recordClsOnPageloadSpan: boolean; +} + /** Add performance related spans to a transaction */ -export function addPerformanceEntries(span: Span): void { +export function addPerformanceEntries(span: Span, options: AddPerformanceEntriesOptions): void { const performance = getBrowserPerformanceAPI(); if (!performance || !WINDOW.performance.getEntries || !browserPerformanceTimeOrigin) { // Gatekeeper if performance API not available @@ -286,7 +301,7 @@ export function addPerformanceEntries(span: Span): void { performanceEntries.slice(_performanceCursor).forEach((entry: Record) => { const startTime = msToSec(entry.startTime); const duration = msToSec( - // Inexplicibly, Chrome sometimes emits a negative duration. We need to work around this. + // Inexplicably, Chrome sometimes emits a negative duration. We need to work around this. // There is a SO post attempting to explain this, but it leaves one with open questions: https://stackoverflow.com/questions/23191918/peformance-getentries-and-negative-duration-display // The way we clamp the value is probably not accurate, since we have observed this happen for things that may take a while to load, like for example the replay worker. // TODO: Investigate why this happens and how to properly mitigate. For now, this is a workaround to prevent transactions being dropped due to negative duration spans. @@ -375,7 +390,8 @@ export function addPerformanceEntries(span: Span): void { // If FCP is not recorded we should not record the cls value // according to the new definition of CLS. - if (!('fcp' in _measurements)) { + // TODO: Check if the first condition is still necessary: `onCLS` already only fires once `onFCP` was called. + if (!('fcp' in _measurements) || !options.recordClsOnPageloadSpan) { delete _measurements.cls; } diff --git a/packages/browser-utils/src/metrics/cls.ts b/packages/browser-utils/src/metrics/cls.ts new file mode 100644 index 000000000000..aa25a54754a1 --- /dev/null +++ b/packages/browser-utils/src/metrics/cls.ts @@ -0,0 +1,122 @@ +import { + SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, + SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT, + SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + getActiveSpan, + getClient, + getCurrentScope, + getRootSpan, + spanToJSON, +} from '@sentry/core'; +import type { SpanAttributes } from '@sentry/types'; +import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/utils'; +import { DEBUG_BUILD } from '../debug-build'; +import { addClsInstrumentationHandler } from './instrument'; +import { msToSec, startStandaloneWebVitalSpan } from './utils'; +import { onHidden } from './web-vitals/lib/onHidden'; + +/** + * Starts tracking the Cumulative Layout Shift on the current page and collects the value once + * + * - the page visibility is hidden + * - a navigation span is started (to stop CLS measurement for SPA soft navigations) + * + * Once either of these events triggers, the CLS value is sent as a standalone span and we stop + * measuring CLS. + */ +export function trackClsAsStandaloneSpan(): void { + let standaloneCLsValue = 0; + let standaloneClsEntry: LayoutShift | undefined; + let pageloadSpanId: string | undefined; + + if (!supportsLayoutShift()) { + return; + } + + let sentSpan = false; + function _collectClsOnce() { + if (sentSpan) { + return; + } + sentSpan = true; + if (pageloadSpanId) { + sendStandaloneClsSpan(standaloneCLsValue, standaloneClsEntry, pageloadSpanId); + } + cleanupClsHandler(); + } + + const cleanupClsHandler = addClsInstrumentationHandler(({ metric }) => { + const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined; + if (!entry) { + return; + } + standaloneCLsValue = metric.value; + standaloneClsEntry = entry; + }, true); + + // use pagehide event from web-vitals + onHidden(() => { + _collectClsOnce(); + }); + + // Since the call chain of this function is synchronous and evaluates before the SDK client is created, + // we need to wait with subscribing to a client hook until the client is created. Therefore, we defer + // to the next tick after the SDK setup. + setTimeout(() => { + const client = getClient(); + + const unsubscribeStartNavigation = client?.on('startNavigationSpan', () => { + _collectClsOnce(); + unsubscribeStartNavigation && unsubscribeStartNavigation(); + }); + + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan && getRootSpan(activeSpan); + const spanJSON = rootSpan && spanToJSON(rootSpan); + if (spanJSON && spanJSON.op === 'pageload') { + pageloadSpanId = rootSpan.spanContext().spanId; + } + }, 0); +} + +function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined, pageloadSpanId: string) { + DEBUG_BUILD && logger.log(`Sending CLS span (${clsValue})`); + + const startTime = msToSec(browserPerformanceTimeOrigin as number) + (entry?.startTime || 0); + const duration = msToSec(entry?.duration || 0); + const routeName = getCurrentScope().getScopeData().transactionName; + + const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift'; + + const attributes: SpanAttributes = dropUndefinedKeys({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls', + [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0, + // attach the pageload span id to the CLS span so that we can link them in the UI + 'sentry.pageload.span_id': pageloadSpanId, + }); + + const span = startStandaloneWebVitalSpan({ + name, + transaction: routeName, + attributes, + startTime, + }); + + span?.addEvent('cls', { + [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: '', + [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: clsValue, + }); + + span?.end(startTime + duration); +} + +function supportsLayoutShift(): boolean { + try { + return PerformanceObserver.supportedEntryTypes?.includes('layout-shift'); + } catch { + return false; + } +} diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index c4186a20f17e..5814b139bd2d 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -2,23 +2,21 @@ import { SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT, SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE, + SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, getActiveSpan, - getClient, getCurrentScope, getRootSpan, spanToJSON, - startInactiveSpan, } from '@sentry/core'; -import type { Integration, Span, SpanAttributes } from '@sentry/types'; +import type { Span, SpanAttributes } from '@sentry/types'; import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString } from '@sentry/utils'; -import { WINDOW } from '../types'; import { addInpInstrumentationHandler, addPerformanceInstrumentationHandler, isPerformanceEventTiming, } from './instrument'; -import { getBrowserPerformanceAPI, msToSec } from './utils'; +import { getBrowserPerformanceAPI, msToSec, startStandaloneWebVitalSpan } from './utils'; const LAST_INTERACTIONS: number[] = []; const INTERACTIONS_SPAN_MAP = new Map(); @@ -71,8 +69,7 @@ const INP_ENTRY_MAP: Record = { /** Starts tracking the Interaction to Next Paint on the current page. */ function _trackINP(): () => void { return addInpInstrumentationHandler(({ metric }) => { - const client = getClient(); - if (!client || metric.value == undefined) { + if (metric.value == undefined) { return; } @@ -85,11 +82,9 @@ function _trackINP(): () => void { const { interactionId } = entry; const interactionType = INP_ENTRY_MAP[entry.name]; - const options = client.getOptions(); /** Build the INP span, create an envelope from the span, and then send the envelope */ const startTime = msToSec((browserPerformanceTimeOrigin as number) + entry.startTime); const duration = msToSec(metric.value); - const scope = getCurrentScope(); const activeSpan = getActiveSpan(); const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; @@ -101,56 +96,28 @@ function _trackINP(): () => void { // Else, we try to use the active span. // Finally, we fall back to look at the transactionName on the scope - const routeName = spanToUse ? spanToJSON(spanToUse).description : scope.getScopeData().transactionName; - - const user = scope.getUser(); - - // We need to get the replay, user, and activeTransaction from the current scope - // so that we can associate replay id, profile id, and a user display to the span - const replay = client.getIntegrationByName string }>('Replay'); - - const replayId = replay && replay.getReplayId(); - - const userDisplay = user !== undefined ? user.email || user.id || user.ip_address : undefined; - let profileId: string | undefined = undefined; - try { - // @ts-expect-error skip optional chaining to save bundle size with try catch - profileId = scope.getScopeData().contexts.profile.profile_id; - } catch { - // do nothing - } + const routeName = spanToUse ? spanToJSON(spanToUse).description : getCurrentScope().getScopeData().transactionName; const name = htmlTreeAsString(entry.target); const attributes: SpanAttributes = dropUndefinedKeys({ - release: options.release, - environment: options.environment, - transaction: routeName, - [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: metric.value, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.inp', - user: userDisplay || undefined, - profile_id: profileId || undefined, - replay_id: replayId || undefined, - // INP score calculation in the sentry backend relies on the user agent - // to account for different INP values being reported from different browsers - 'user_agent.original': WINDOW.navigator && WINDOW.navigator.userAgent, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `ui.interaction.${interactionType}`, + [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry.duration, }); - const span = startInactiveSpan({ + const span = startStandaloneWebVitalSpan({ name, - op: `ui.interaction.${interactionType}`, + transaction: routeName, attributes, - startTime: startTime, - experimental: { - standalone: true, - }, + startTime, }); - span.addEvent('inp', { + span?.addEvent('inp', { [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: 'millisecond', [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: metric.value, }); - span.end(startTime + duration); + span?.end(startTime + duration); }); } diff --git a/packages/browser-utils/src/metrics/utils.ts b/packages/browser-utils/src/metrics/utils.ts index d46cb2cfe35c..5f9d0de4d4ab 100644 --- a/packages/browser-utils/src/metrics/utils.ts +++ b/packages/browser-utils/src/metrics/utils.ts @@ -1,6 +1,6 @@ import type { SentrySpan } from '@sentry/core'; -import { spanToJSON, startInactiveSpan, withActiveSpan } from '@sentry/core'; -import type { Span, SpanTimeInput, StartSpanOptions } from '@sentry/types'; +import { getClient, getCurrentScope, spanToJSON, startInactiveSpan, withActiveSpan } from '@sentry/core'; +import type { Integration, Span, SpanAttributes, SpanTimeInput, StartSpanOptions } from '@sentry/types'; import { WINDOW } from '../types'; /** @@ -44,6 +44,84 @@ export function startAndEndSpan( }); } +interface StandaloneWebVitalSpanOptions { + name: string; + transaction?: string; + attributes: SpanAttributes; + startTime: number; +} + +/** + * Starts an inactive, standalone span used to send web vital values to Sentry. + * DO NOT use this for arbitrary spans, as these spans require special handling + * during ingestion to extract metrics. + * + * This function adds a bunch of attributes and data to the span that's shared + * by all web vital standalone spans. However, you need to take care of adding + * the actual web vital value as an event to the span. Also, you need to assign + * a transaction name and some other values that are specific to the web vital. + * + * Ultimately, you also need to take care of ending the span to send it off. + * + * @param options + * + * @returns an inactive, standalone and NOT YET ended span + */ +export function startStandaloneWebVitalSpan(options: StandaloneWebVitalSpanOptions): Span | undefined { + const client = getClient(); + if (!client) { + return; + } + + const { name, transaction, attributes: passedAttributes, startTime } = options; + + const { release, environment } = client.getOptions(); + // We need to get the replay, user, and activeTransaction from the current scope + // so that we can associate replay id, profile id, and a user display to the span + const replay = client.getIntegrationByName string }>('Replay'); + const replayId = replay && replay.getReplayId(); + + const scope = getCurrentScope(); + + const user = scope.getUser(); + const userDisplay = user !== undefined ? user.email || user.id || user.ip_address : undefined; + + let profileId: string | undefined = undefined; + try { + // @ts-expect-error skip optional chaining to save bundle size with try catch + profileId = scope.getScopeData().contexts.profile.profile_id; + } catch { + // do nothing + } + + const attributes: SpanAttributes = { + release, + environment, + + user: userDisplay || undefined, + profile_id: profileId || undefined, + replay_id: replayId || undefined, + + transaction, + + // Web vital score calculation relies on the user agent to account for different + // browsers setting different thresholds for what is considered a good/meh/bad value. + // For example: Chrome vs. Chrome Mobile + 'user_agent.original': WINDOW.navigator && WINDOW.navigator.userAgent, + + ...passedAttributes, + }; + + return startInactiveSpan({ + name, + attributes, + startTime, + experimental: { + standalone: true, + }, + }); +} + /** Get the browser performance API. */ export function getBrowserPerformanceAPI(): Performance | undefined { // @ts-expect-error we want to make sure all of these are available, even if TS is sure they are diff --git a/packages/browser-utils/src/metrics/web-vitals/README.md b/packages/browser-utils/src/metrics/web-vitals/README.md index 653ee22c7ff1..d779969dbe5d 100644 --- a/packages/browser-utils/src/metrics/web-vitals/README.md +++ b/packages/browser-utils/src/metrics/web-vitals/README.md @@ -17,9 +17,9 @@ Current vendored web vitals are: ## Notable Changes from web-vitals library -This vendored web-vitals library is meant to be used in conjunction with the `@sentry/tracing` `BrowserTracing` -integration. As such, logic around `BFCache` and multiple reports were removed from the library as our web-vitals only -report once per pageload. +This vendored web-vitals library is meant to be used in conjunction with the `@sentry/browser` +`browserTracingIntegration`. As such, logic around `BFCache` and multiple reports were removed from the library as our +web-vitals only report once per pageload. ## License diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 491c7aaae88d..ff5201878cff 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -146,6 +146,7 @@ export interface BrowserTracingOptions { */ _experiments: Partial<{ enableInteractions: boolean; + enableStandaloneClsSpans: boolean; }>; /** @@ -191,7 +192,7 @@ export const browserTracingIntegration = ((_options: Partial { _collectWebVitals(); - addPerformanceEntries(span); + addPerformanceEntries(span, { recordClsOnPageloadSpan: !enableStandaloneClsSpans }); }, }); @@ -298,6 +299,7 @@ export const browserTracingIntegration = ((_options: Partial Date: Tue, 13 Aug 2024 13:13:08 +0200 Subject: [PATCH 28/35] fix(nuxt): Add import line for disabled `autoImport` (#13342) Adds explicit imports in case `autoImport` is disabled (Nuxt docs [here](https://nuxt.com/docs/guide/concepts/auto-imports#disabling-auto-imports)). Disabled `autoImport` in the E2E test to verify. fixes https://github.com/getsentry/sentry-javascript/issues/13302 --- dev-packages/e2e-tests/test-applications/nuxt-3/app.vue | 2 +- .../e2e-tests/test-applications/nuxt-3/nuxt.config.ts | 3 +++ .../test-applications/nuxt-3/pages/fetch-server-error.vue | 4 +++- .../test-applications/nuxt-3/pages/test-param/[param].vue | 2 ++ .../nuxt-3/server/api/param-error/[param].ts | 2 ++ .../test-applications/nuxt-3/server/api/server-error.ts | 2 ++ .../test-applications/nuxt-3/server/api/test-param/[param].ts | 2 ++ packages/nuxt/src/module.ts | 2 ++ 8 files changed, 17 insertions(+), 2 deletions(-) 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 4e7954ceb4af..23283a522546 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/app.vue +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/app.vue @@ -4,7 +4,7 @@ diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/nuxt.config.ts b/dev-packages/e2e-tests/test-applications/nuxt-3/nuxt.config.ts index 87cff074ccd9..69b31a4214ec 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/nuxt.config.ts +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/nuxt.config.ts @@ -1,4 +1,7 @@ // https://nuxt.com/docs/api/configuration/nuxt-config export default defineNuxtConfig({ modules: ['@sentry/nuxt/module'], + imports: { + autoImport: false, + }, }); 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 index 4643f045582e..8cb2a9997e58 100644 --- 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 @@ -5,7 +5,9 @@ \ 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 4b2b7e35a83e..2ac1b9095a0f 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 @@ -6,6 +6,8 @@