diff --git a/CHANGELOG.md b/CHANGELOG.md index 50c7c9202486..4f73317a2392 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,51 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 10.4.0 + +### Important Changes + +- **fix(browser): Ensure IP address is only inferred by Relay if `sendDefaultPii` is `true`** + +This release includes a fix for a [behaviour change](https://docs.sentry.io/platforms/javascript/migration/v8-to-v9/#behavior-changes) +that was originally introduced with v9 of the SDK: User IP Addresses should only be added to Sentry events automatically, +if `sendDefaultPii` was set to `true`. + +However, the change in v9 required further internal adjustment, which should have been included in v10 of the SDK. +Unfortunately, the change did not make it into the initial v10 version but is now applied with `10.4.0`. +There is _no API_ breakage involved and hence it is safe to update. +However, after updating the SDK, events (errors, traces, replays, etc.) sent from the browser, will only include +user IP addresses, if you set `sendDefaultPii: true` in your `Sentry.init` options. + +We apologize for any inconvenience caused! + +- **feat(node): Add `ignoreStaticAssets` ([#17370](https://github.com/getsentry/sentry-javascript/pull/17370))** + +This release adds a new option to `httpIntegration` to ignore requests for static assets (e.g. `favicon.xml` or `robots.txt`). The option defaults to `true`, meaning that going forward, such requests will not be traced by default. You can still enable tracing for these requests by setting the option to `false`: + +```js +Sentry.init({ + integrations: [ + Sentry.httpIntegration({ + // defaults to true, set to false to enable traces for static assets + ignoreStaticAssets: false, + }), + ], +}); +``` + +### Other Changes + +- fix(nuxt): Do not drop parametrized routes ([#17357](https://github.com/getsentry/sentry-javascript/pull/17357)) + +
+ Internal Changes + +- ref(node): Split up incoming & outgoing http handling ([#17358](https://github.com/getsentry/sentry-javascript/pull/17358)) +- test(node): Enable additionalDependencies in integration runner ([#17361](https://github.com/getsentry/sentry-javascript/pull/17361)) + +
+ ## 10.3.0 - feat(core): MCP Server - Capture prompt results from prompt function calls (#17284) diff --git a/MIGRATION.md b/MIGRATION.md index ceaa6578e8eb..84d4e63da562 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -80,6 +80,18 @@ The removal entails **no breaking API changes**. However, in rare cases, you mig - If you set up Sentry Alerts that depend on FID, be aware that these could trigger once you upgrade the SDK, due to a lack of new values. To replace them, adjust your alerts (or dashbaords) to use INP. +### Update: User IP Address collection gated by `sendDefaultPii` + +Version `10.4.0` introduced a change that should have ideally been introduced with `10.0.0` of the SDK. +Originally destined for [version `9.0.0`](https://docs.sentry.io/platforms/javascript/migration/v8-to-v9/#behavior-changes), but having not the desired effect until v10, +SDKs will now control IP address inference of user IP addresses depending on the value of the top level `sendDefaultPii` init option. + +- If `sendDefaultPii` is `true`, Sentry will infer the IP address of users' devices to events (errors, traces, replays, etc) in all browser-based SDKs. +- If `sendDefaultPii` is `false` or not set, Sentry will not infer or collect IP address data. + +Given that this was already the advertised behaviour since v9, we classify the change [as a fix](https://github.com/getsentry/sentry-javascript/pull/17364), +though we recognize the potential impact of it. We apologize for any inconvenience caused. + ## No Version Support Timeline Version support timelines are stressful for everybody using the SDK, so we won't be defining one. diff --git a/dev-packages/browser-integration-tests/suites/feedback/attachTo/test.ts b/dev-packages/browser-integration-tests/suites/feedback/attachTo/test.ts index c93ce0453f83..ffc00cc8258c 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/attachTo/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/attachTo/test.ts @@ -61,6 +61,9 @@ sentryTest('should capture feedback with custom button', async ({ getLocalTestUr version: expect.any(String), name: 'sentry.javascript.browser', packages: expect.anything(), + settings: { + infer_ip: 'never', + }, }, request: { url: `${TEST_HOST}/index.html`, diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts index e6eb920f64a5..9d6cf1a8a1f1 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts @@ -61,6 +61,9 @@ sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => { version: expect.any(String), name: 'sentry.javascript.browser', packages: expect.anything(), + settings: { + infer_ip: 'never', + }, }, request: { url: `${TEST_HOST}/index.html`, diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts index 66653ce68a82..d76fd2089413 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts @@ -95,6 +95,9 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestUrl version: expect.any(String), name: 'sentry.javascript.browser', packages: expect.anything(), + settings: { + infer_ip: 'never', + }, }, request: { url: `${TEST_HOST}/index.html`, diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts index 69c715654921..cb683ce4fa36 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts @@ -61,6 +61,9 @@ sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => { version: expect.any(String), name: 'sentry.javascript.browser', packages: expect.anything(), + settings: { + infer_ip: 'never', + }, }, request: { url: `${TEST_HOST}/index.html`, diff --git a/dev-packages/browser-integration-tests/suites/manual-client/browser-context/test.ts b/dev-packages/browser-integration-tests/suites/manual-client/browser-context/test.ts index 7ba25a3899b0..4637fcc5555d 100644 --- a/dev-packages/browser-integration-tests/suites/manual-client/browser-context/test.ts +++ b/dev-packages/browser-integration-tests/suites/manual-client/browser-context/test.ts @@ -41,6 +41,9 @@ sentryTest('allows to setup a client manually & capture exceptions', async ({ ge name: 'sentry.javascript.browser', version: expect.any(String), packages: [{ name: expect.any(String), version: expect.any(String) }], + settings: { + infer_ip: 'never', + }, }, contexts: { trace: { trace_id: expect.stringMatching(/[a-f0-9]{32}/), span_id: expect.stringMatching(/[a-f0-9]{16}/) }, diff --git a/dev-packages/browser-integration-tests/suites/public-api/captureException/simpleError/test.ts b/dev-packages/browser-integration-tests/suites/public-api/captureException/simpleError/test.ts index e5c38d738ec7..e8474472af35 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/captureException/simpleError/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/captureException/simpleError/test.ts @@ -39,5 +39,8 @@ sentryTest('should capture correct SDK metadata', async ({ getLocalTestUrl, page version: SDK_VERSION, }, ], + settings: { + infer_ip: 'never', + }, }); }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/errors/test.ts index 06a5ede3215a..453dbafa0c37 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/errors/test.ts @@ -1,10 +1,12 @@ import { expect } from '@playwright/test'; -import type { Event } from '@sentry/core'; import { sentryTest } from '../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; +import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers'; -sentryTest('should default user to {{auto}} on errors when sendDefaultPii: true', async ({ getLocalTestUrl, page }) => { - const url = await getLocalTestUrl({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); - expect(eventData.user?.ip_address).toBe('{{auto}}'); -}); +sentryTest( + 'sets sdk.settings.infer_ip to "auto" on errors when sendDefaultPii: true', + async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + const eventData = await envelopeRequestParser(await waitForErrorRequestOnUrl(page, url)); + expect(eventData.sdk?.settings?.infer_ip).toBe('auto'); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/performance/test.ts b/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/performance/test.ts index 6e1f20826548..e624afcd2117 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/performance/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/performance/test.ts @@ -7,7 +7,7 @@ import { } from '../../../../utils/helpers'; sentryTest( - 'should default user to {{auto}} on transactions when sendDefaultPii: true', + 'sets user.ip_address to "auto" on transactions when sendDefaultPii: true', async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); @@ -16,6 +16,6 @@ sentryTest( const url = await getLocalTestUrl({ testDir: __dirname }); const req = await waitForTransactionRequestOnUrl(page, url); const transaction = envelopeRequestParser(req); - expect(transaction.user?.ip_address).toBe('{{auto}}'); + expect(transaction.sdk?.settings?.infer_ip).toBe('auto'); }, ); diff --git a/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/replay/test.ts b/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/replay/test.ts index 59bc021ce0a1..a24d5e44ae04 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/replay/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/replay/test.ts @@ -3,7 +3,7 @@ import { sentryTest } from '../../../../utils/fixtures'; import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; sentryTest( - 'replay recording should contain default performance spans', + 'sets sdk.settings.infer_ip to "auto" on replay events when sendDefaultPii: true', async ({ getLocalTestUrl, page, browserName }) => { // We only test this against the NPM package and replay bundles // and only on chromium as most performance entries are only available in chromium @@ -18,6 +18,6 @@ sentryTest( await page.goto(url); const replayEvent = getReplayEvent(await reqPromise0); - expect(replayEvent.user?.ip_address).toBe('{{auto}}'); + expect(replayEvent.sdk?.settings?.infer_ip).toBe('auto'); }, ); diff --git a/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/sessions/test.ts b/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/sessions/test.ts index 7f047337dae3..4c963a737e97 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/sessions/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/sessions/test.ts @@ -3,7 +3,7 @@ import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; sentryTest( - 'should default user to {{auto}} on sessions when sendDefaultPii: true', + 'sets attrs.ip_address user to {{auto}} on sessions when sendDefaultPii: true', async ({ getLocalTestUrl, page }) => { const url = await getLocalTestUrl({ testDir: __dirname }); const session = await getFirstSentryEnvelopeRequest(page, url); diff --git a/dev-packages/browser-integration-tests/suites/public-api/setUser/unset_user/test.ts b/dev-packages/browser-integration-tests/suites/public-api/setUser/unset_user/test.ts index 13e6786a6a60..8166d35df2d0 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/setUser/unset_user/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/setUser/unset_user/test.ts @@ -11,7 +11,7 @@ sentryTest('should unset user', async ({ getLocalTestUrl, page }) => { expect(eventData[0].message).toBe('no_user'); // because sendDefaultPii: true - expect(eventData[0].user).toEqual({ ip_address: '{{auto}}' }); + expect(eventData[0].sdk?.settings?.infer_ip).toBe('auto'); expect(eventData[1].message).toBe('user'); expect(eventData[1].user).toEqual({ @@ -23,7 +23,5 @@ sentryTest('should unset user', async ({ getLocalTestUrl, page }) => { expect(eventData[2].message).toBe('unset_user'); // because sendDefaultPii: true - expect(eventData[2].user).toEqual({ - ip_address: '{{auto}}', - }); + expect(eventData[2].sdk?.settings?.infer_ip).toBe('auto'); }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/setUser/update_user/test.ts b/dev-packages/browser-integration-tests/suites/public-api/setUser/update_user/test.ts index bf26aae4d61a..19b6b7f75576 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/setUser/update_user/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/setUser/update_user/test.ts @@ -13,10 +13,11 @@ sentryTest('should update user', async ({ getLocalTestUrl, page }) => { id: 'foo', ip_address: 'bar', }); + expect(eventData[0].sdk?.settings?.infer_ip).toBe('auto'); expect(eventData[1].message).toBe('second_user'); expect(eventData[1].user).toEqual({ id: 'baz', - ip_address: '{{auto}}', }); + expect(eventData[1].sdk?.settings?.infer_ip).toBe('auto'); }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/withScope/nested_scopes/test.ts b/dev-packages/browser-integration-tests/suites/public-api/withScope/nested_scopes/test.ts index 75a459ef69d6..5ec77ed3938f 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/withScope/nested_scopes/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/withScope/nested_scopes/test.ts @@ -11,34 +11,28 @@ sentryTest('should allow nested scoping', async ({ getLocalTestUrl, page }) => { expect(eventData[0].message).toBe('root_before'); expect(eventData[0].user).toEqual({ id: 'qux', - ip_address: '{{auto}}', // because sendDefaultPii: true }); expect(eventData[0].tags).toBeUndefined(); expect(eventData[1].message).toBe('outer_before'); expect(eventData[1].user).toEqual({ id: 'qux', - ip_address: '{{auto}}', // because sendDefaultPii: true }); expect(eventData[1].tags).toMatchObject({ foo: false }); expect(eventData[2].message).toBe('inner'); - expect(eventData[2].user).toEqual({ - ip_address: '{{auto}}', // because sendDefaultPii: true - }); + expect(eventData[2].user).toEqual({}); expect(eventData[2].tags).toMatchObject({ foo: false, bar: 10 }); expect(eventData[3].message).toBe('outer_after'); expect(eventData[3].user).toEqual({ id: 'baz', - ip_address: '{{auto}}', // because sendDefaultPii: true }); expect(eventData[3].tags).toMatchObject({ foo: false }); expect(eventData[4].message).toBe('root_after'); expect(eventData[4].user).toEqual({ id: 'qux', - ip_address: '{{auto}}', // because sendDefaultPii: true }); expect(eventData[4].tags).toBeUndefined(); }); diff --git a/dev-packages/browser-integration-tests/suites/replay/captureReplay/test.ts b/dev-packages/browser-integration-tests/suites/replay/captureReplay/test.ts index 85dd7c27440a..8167552fb0d6 100644 --- a/dev-packages/browser-integration-tests/suites/replay/captureReplay/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/captureReplay/test.ts @@ -47,6 +47,9 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT ]), version: SDK_VERSION, name: 'sentry.javascript.browser', + settings: { + infer_ip: 'never', + }, }, request: { url: `${TEST_HOST}/index.html`, @@ -85,6 +88,9 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT ]), version: SDK_VERSION, name: 'sentry.javascript.browser', + settings: { + infer_ip: 'never', + }, }, request: { url: `${TEST_HOST}/index.html`, diff --git a/dev-packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts b/dev-packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts index 9b44ba6b218a..f6c3dcf17b23 100644 --- a/dev-packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts @@ -47,6 +47,9 @@ sentryTest('should capture replays (@sentry-internal/replay export)', async ({ g ]), version: SDK_VERSION, name: 'sentry.javascript.browser', + settings: { + infer_ip: 'never', + }, }, request: { url: `${TEST_HOST}/index.html`, @@ -85,6 +88,9 @@ sentryTest('should capture replays (@sentry-internal/replay export)', async ({ g ]), version: SDK_VERSION, name: 'sentry.javascript.browser', + settings: { + infer_ip: 'never', + }, }, request: { url: `${TEST_HOST}/index.html`, diff --git a/dev-packages/browser-integration-tests/utils/replayEventTemplates.ts b/dev-packages/browser-integration-tests/utils/replayEventTemplates.ts index 97787c2de26e..53a9e733a908 100644 --- a/dev-packages/browser-integration-tests/utils/replayEventTemplates.ts +++ b/dev-packages/browser-integration-tests/utils/replayEventTemplates.ts @@ -30,6 +30,9 @@ const DEFAULT_REPLAY_EVENT = { ]), version: SDK_VERSION, name: 'sentry.javascript.browser', + settings: { + infer_ip: 'never', + }, }, request: { url: expect.stringContaining('/index.html'), diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 47bd4e2faa1b..5bde8d4c5d0b 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -14,7 +14,7 @@ "build:dev": "yarn build", "build:transpile": "rollup -c rollup.npm.config.mjs", "build:types": "tsc -p tsconfig.types.json", - "clean": "rimraf -g **/node_modules && run-p clean:script", + "clean": "rimraf -g suites/**/node_modules suites/**/tmp_* && run-p clean:script", "clean:script": "node scripts/clean.js", "lint": "eslint . --format stylish", "fix": "eslint . --format stylish --fix", diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreStaticAssets.js b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreStaticAssets.js new file mode 100644 index 000000000000..bcc47257a2f1 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreStaticAssets.js @@ -0,0 +1,39 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, + // Test default for ignoreStaticAssets: true + integrations: [Sentry.httpIntegration()], +}); + +const express = require('express'); +const cors = require('cors'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/test', (_req, res) => { + res.send({ response: 'ok' }); +}); + +app.get('/favicon.ico', (_req, res) => { + res.type('image/x-icon').send(Buffer.from([0])); +}); + +app.get('/robots.txt', (_req, res) => { + res.type('text/plain').send('User-agent: *\nDisallow:\n'); +}); + +app.get('/assets/app.js', (_req, res) => { + res.type('application/javascript').send('/* js */'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-traceStaticAssets.js b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-traceStaticAssets.js new file mode 100644 index 000000000000..743d1b48e21f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-traceStaticAssets.js @@ -0,0 +1,38 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, + integrations: [Sentry.httpIntegration({ ignoreStaticAssets: false })], +}); + +const express = require('express'); +const cors = require('cors'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.use(cors()); + +app.get('/test', (_req, res) => { + res.send({ response: 'ok' }); +}); + +app.get('/favicon.ico', (_req, res) => { + res.type('image/x-icon').send(Buffer.from([0])); +}); + +app.get('/robots.txt', (_req, res) => { + res.type('text/plain').send('User-agent: *\nDisallow:\n'); +}); + +app.get('/assets/app.js', (_req, res) => { + res.type('application/javascript').send('/* js */'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts index 9682f4aa28ac..97043c998814 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts @@ -185,4 +185,44 @@ describe('httpIntegration', () => { closeTestServer(); }); }); + + test('ignores static asset requests by default', async () => { + const runner = createRunner(__dirname, 'server-ignoreStaticAssets.js') + .expect({ + transaction: event => { + expect(event.transaction).toBe('GET /test'); + expect(event.contexts?.trace?.data?.url).toMatch(/\/test$/); + expect(event.contexts?.trace?.op).toBe('http.server'); + expect(event.contexts?.trace?.status).toBe('ok'); + }, + }) + .start(); + + // These should be ignored by default + await runner.makeRequest('get', '/favicon.ico'); + await runner.makeRequest('get', '/robots.txt'); + await runner.makeRequest('get', '/assets/app.js'); + + // This one should be traced + await runner.makeRequest('get', '/test'); + + await runner.completed(); + }); + + test('traces static asset requests when ignoreStaticAssets is false', async () => { + const runner = createRunner(__dirname, 'server-traceStaticAssets.js') + .expect({ + transaction: event => { + expect(event.transaction).toBe('GET /favicon.ico'); + expect(event.contexts?.trace?.data?.url).toMatch(/\/favicon.ico$/); + expect(event.contexts?.trace?.op).toBe('http.server'); + expect(event.contexts?.trace?.status).toBe('ok'); + }, + }) + .start(); + + await runner.makeRequest('get', '/favicon.ico'); + + await runner.completed(); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/vercelai/scenario-v5.mjs b/dev-packages/node-integration-tests/suites/tracing/vercelai/scenario-v5.mjs new file mode 100644 index 000000000000..8cfe6d64ad05 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/vercelai/scenario-v5.mjs @@ -0,0 +1,75 @@ +import * as Sentry from '@sentry/node'; +import { generateText } from 'ai'; +import { MockLanguageModelV2 } from 'ai/test'; +import { z } from 'zod'; + +async function run() { + await Sentry.startSpan({ op: 'function', name: 'main' }, async () => { + await generateText({ + model: new MockLanguageModelV2({ + doGenerate: async () => ({ + finishReason: 'stop', + usage: { inputTokens: 10, outputTokens: 20, totalTokens: 30 }, + content: [{ type: 'text', text: 'First span here!' }], + }), + }), + prompt: 'Where is the first span?', + }); + + // This span should have input and output prompts attached because telemetry is explicitly enabled. + await generateText({ + experimental_telemetry: { isEnabled: true }, + model: new MockLanguageModelV2({ + doGenerate: async () => ({ + finishReason: 'stop', + usage: { inputTokens: 10, outputTokens: 20, totalTokens: 30 }, + content: [{ type: 'text', text: 'Second span here!' }], + }), + }), + prompt: 'Where is the second span?', + }); + + // This span should include tool calls and tool results + await generateText({ + model: new MockLanguageModelV2({ + doGenerate: async () => ({ + finishReason: 'tool-calls', + usage: { inputTokens: 15, outputTokens: 25, totalTokens: 40 }, + content: [{ type: 'text', text: 'Tool call completed!' }], + toolCalls: [ + { + toolCallType: 'function', + toolCallId: 'call-1', + toolName: 'getWeather', + args: '{ "location": "San Francisco" }', + }, + ], + }), + }), + tools: { + getWeather: { + parameters: z.object({ location: z.string() }), + execute: async args => { + return `Weather in ${args.location}: Sunny, 72°F`; + }, + }, + }, + prompt: 'What is the weather in San Francisco?', + }); + + // This span should not be captured because we've disabled telemetry + await generateText({ + experimental_telemetry: { isEnabled: false }, + model: new MockLanguageModelV2({ + doGenerate: async () => ({ + finishReason: 'stop', + usage: { inputTokens: 10, outputTokens: 20, totalTokens: 30 }, + content: [{ type: 'text', text: 'Third span here!' }], + }), + }), + prompt: 'Where is the third span?', + }); + }); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/vercelai/test.ts b/dev-packages/node-integration-tests/suites/tracing/vercelai/test.ts index 94fd0dde8486..720345cc7d86 100644 --- a/dev-packages/node-integration-tests/suites/tracing/vercelai/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/vercelai/test.ts @@ -197,6 +197,73 @@ describe('Vercel AI integration', () => { ]), }; + // Todo: Add missing attribute spans for v5 + // Right now only second span is recorded as it's manually opted in via explicit telemetry option + const EXPECTED_TRANSACTION_DEFAULT_PII_FALSE_V5 = { + transaction: 'main', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'vercel.ai.model.id': 'mock-model-id', + 'vercel.ai.model.provider': 'mock-provider', + 'vercel.ai.operationId': 'ai.generateText', + 'vercel.ai.pipeline.name': 'generateText', + 'vercel.ai.prompt': '{"prompt":"Where is the second span?"}', + 'vercel.ai.response.finishReason': 'stop', + 'gen_ai.response.text': expect.any(String), + 'vercel.ai.settings.maxRetries': 2, + // 'vercel.ai.settings.maxSteps': 1, + 'vercel.ai.streaming': false, + 'gen_ai.prompt': '{"prompt":"Where is the second span?"}', + 'gen_ai.response.model': 'mock-model-id', + 'gen_ai.usage.input_tokens': 10, + 'gen_ai.usage.output_tokens': 20, + 'gen_ai.usage.total_tokens': 30, + 'operation.name': 'ai.generateText', + 'sentry.op': 'gen_ai.invoke_agent', + 'sentry.origin': 'auto.vercelai.otel', + }, + description: 'generateText', + op: 'gen_ai.invoke_agent', + origin: 'auto.vercelai.otel', + status: 'ok', + }), + // doGenerate + expect.objectContaining({ + data: { + 'sentry.origin': 'auto.vercelai.otel', + 'sentry.op': 'gen_ai.generate_text', + 'operation.name': 'ai.generateText.doGenerate', + 'vercel.ai.operationId': 'ai.generateText.doGenerate', + 'vercel.ai.model.provider': 'mock-provider', + 'vercel.ai.model.id': 'mock-model-id', + 'vercel.ai.settings.maxRetries': 2, + 'gen_ai.system': 'mock-provider', + 'gen_ai.request.model': 'mock-model-id', + 'vercel.ai.pipeline.name': 'generateText.doGenerate', + 'vercel.ai.streaming': false, + 'vercel.ai.response.finishReason': 'stop', + 'vercel.ai.response.model': 'mock-model-id', + 'vercel.ai.response.id': expect.any(String), + 'gen_ai.response.text': 'Second span here!', + 'vercel.ai.response.timestamp': expect.any(String), + // 'vercel.ai.prompt.format': expect.any(String), + 'gen_ai.request.messages': expect.any(String), + 'gen_ai.response.finish_reasons': ['stop'], + 'gen_ai.usage.input_tokens': 10, + 'gen_ai.usage.output_tokens': 20, + 'gen_ai.response.id': expect.any(String), + 'gen_ai.response.model': 'mock-model-id', + 'gen_ai.usage.total_tokens': 30, + }, + description: 'generate_text mock-model-id', + op: 'gen_ai.generate_text', + origin: 'auto.vercelai.otel', + status: 'ok', + }), + ]), + }; + const EXPECTED_TRANSACTION_DEFAULT_PII_TRUE = { transaction: 'main', spans: expect.arrayContaining([ @@ -538,6 +605,23 @@ describe('Vercel AI integration', () => { }); }); + // Test with specific Vercel AI v5 version + createEsmAndCjsTests( + __dirname, + 'scenario-v5.mjs', + 'instrument.mjs', + (createRunner, test) => { + test('creates ai related spans with v5', async () => { + await createRunner().expect({ transaction: EXPECTED_TRANSACTION_DEFAULT_PII_FALSE_V5 }).start().completed(); + }); + }, + { + additionalDependencies: { + ai: '^5.0.0', + }, + }, + ); + createEsmAndCjsTests(__dirname, 'scenario-error-in-tool-express.mjs', 'instrument.mjs', (createRunner, test) => { test('captures error in tool in express server', async () => { const expectedTransaction = { diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 44118747c45c..f4a176688280 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -14,10 +14,10 @@ import type { import { normalize } from '@sentry/core'; import { createBasicSentryServer } from '@sentry-internal/test-utils'; import { execSync, spawn, spawnSync } from 'child_process'; -import { existsSync, readFileSync, unlinkSync, writeFileSync } from 'fs'; -import { join } from 'path'; +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs'; +import { basename, join } from 'path'; import { inspect } from 'util'; -import { afterAll, beforeAll, describe, test } from 'vitest'; +import { afterAll, describe, test } from 'vitest'; import { assertEnvelopeHeader, assertSentryCheckIn, @@ -174,7 +174,10 @@ export function createEsmAndCjsTests( testFn: typeof test | typeof test.fails, mode: 'esm' | 'cjs', ) => void, - options?: { failsOnCjs?: boolean; failsOnEsm?: boolean }, + // `additionalDependencies` to install in a tmp dir for the esm and cjs tests + // This could be used to override packages that live in the parent package.json for the specific run of the test + // e.g. `{ ai: '^5.0.0' }` to test Vercel AI v5 + options?: { failsOnCjs?: boolean; failsOnEsm?: boolean; additionalDependencies?: Record }, ): void { const mjsScenarioPath = join(cwd, scenarioPath); const mjsInstrumentPath = join(cwd, instrumentPath); @@ -187,36 +190,107 @@ export function createEsmAndCjsTests( throw new Error(`Instrument file not found: ${mjsInstrumentPath}`); } - const cjsScenarioPath = join(cwd, `tmp_${scenarioPath.replace('.mjs', '.cjs')}`); - const cjsInstrumentPath = join(cwd, `tmp_${instrumentPath.replace('.mjs', '.cjs')}`); + // Create a dedicated tmp directory that includes copied ESM & CJS scenario/instrument files. + // If additionalDependencies are provided, we also create a nested package.json and install them there. + const uniqueId = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`; + const tmpDirPath = join(cwd, `tmp_${uniqueId}`); + mkdirSync(tmpDirPath); + + // Copy ESM files as-is into tmp dir + const esmScenarioBasename = basename(scenarioPath); + const esmInstrumentBasename = basename(instrumentPath); + const esmScenarioPathForRun = join(tmpDirPath, esmScenarioBasename); + const esmInstrumentPathForRun = join(tmpDirPath, esmInstrumentBasename); + writeFileSync(esmScenarioPathForRun, readFileSync(mjsScenarioPath, 'utf8')); + writeFileSync(esmInstrumentPathForRun, readFileSync(mjsInstrumentPath, 'utf8')); + + // Pre-create CJS converted files inside tmp dir + const cjsScenarioPath = join(tmpDirPath, esmScenarioBasename.replace('.mjs', '.cjs')); + const cjsInstrumentPath = join(tmpDirPath, esmInstrumentBasename.replace('.mjs', '.cjs')); + convertEsmFileToCjs(esmScenarioPathForRun, cjsScenarioPath); + convertEsmFileToCjs(esmInstrumentPathForRun, cjsInstrumentPath); + + // Create a minimal package.json with requested dependencies (if any) and install them + const additionalDependencies = options?.additionalDependencies ?? {}; + if (Object.keys(additionalDependencies).length > 0) { + const packageJson = { + name: 'tmp-integration-test', + private: true, + version: '0.0.0', + dependencies: additionalDependencies, + } as const; + + writeFileSync(join(tmpDirPath, 'package.json'), JSON.stringify(packageJson, null, 2)); + + try { + const deps = Object.entries(additionalDependencies).map(([name, range]) => { + if (!range || typeof range !== 'string') { + throw new Error(`Invalid version range for "${name}": ${String(range)}`); + } + return `${name}@${range}`; + }); - describe('esm', () => { - const testFn = options?.failsOnEsm ? test.fails : test; - callback(() => createRunner(mjsScenarioPath).withFlags('--import', mjsInstrumentPath), testFn, 'esm'); - }); + if (deps.length > 0) { + // Prefer npm for temp installs to avoid Yarn engine strictness; see https://github.com/vercel/ai/issues/7777 + // We rely on the generated package.json dependencies and run a plain install. + const result = spawnSync('npm', ['install', '--silent', '--no-audit', '--no-fund'], { + cwd: tmpDirPath, + encoding: 'utf8', + }); + + if (process.env.DEBUG) { + // eslint-disable-next-line no-console + console.log('[additionalDependencies via npm]', deps.join(' ')); + // eslint-disable-next-line no-console + console.log('[npm stdout]', result.stdout); + // eslint-disable-next-line no-console + console.log('[npm stderr]', result.stderr); + } - describe('cjs', () => { - beforeAll(() => { - // For the CJS runner, we create some temporary files... - convertEsmFileToCjs(mjsScenarioPath, cjsScenarioPath); - convertEsmFileToCjs(mjsInstrumentPath, cjsInstrumentPath); + if (result.error) { + throw new Error(`Failed to install additionalDependencies in tmp dir ${tmpDirPath}: ${result.error.message}`); + } + if (typeof result.status === 'number' && result.status !== 0) { + throw new Error( + `Failed to install additionalDependencies in tmp dir ${tmpDirPath} (exit ${result.status}):\n${ + result.stderr || result.stdout || '(no output)' + }`, + ); + } + } + } catch (e) { + // eslint-disable-next-line no-console + console.error('Failed to install additionalDependencies:', e); + throw e; + } + } + + describe('esm/cjs', () => { + const esmTestFn = options?.failsOnEsm ? test.fails : test; + describe('esm', () => { + callback( + () => createRunner(esmScenarioPathForRun).withFlags('--import', esmInstrumentPathForRun), + esmTestFn, + 'esm', + ); + }); + + const cjsTestFn = options?.failsOnCjs ? test.fails : test; + describe('cjs', () => { + callback(() => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath), cjsTestFn, 'cjs'); }); + // Clean up the tmp directory after both esm and cjs suites have run afterAll(() => { try { - unlinkSync(cjsInstrumentPath); - } catch { - // Ignore errors here - } - try { - unlinkSync(cjsScenarioPath); + rmSync(tmpDirPath, { recursive: true, force: true }); } catch { - // Ignore errors here + if (process.env.DEBUG) { + // eslint-disable-next-line no-console + console.error(`Failed to remove tmp dir: ${tmpDirPath}`); + } } }); - - const testFn = options?.failsOnCjs ? test.fails : test; - callback(() => createRunner(cjsScenarioPath).withFlags('--require', cjsInstrumentPath), testFn, 'cjs'); }); } diff --git a/packages/astro/test/client/sdk.test.ts b/packages/astro/test/client/sdk.test.ts index a537013f7c22..20cad73269e5 100644 --- a/packages/astro/test/client/sdk.test.ts +++ b/packages/astro/test/client/sdk.test.ts @@ -41,6 +41,9 @@ describe('Sentry client SDK', () => { { name: 'npm:@sentry/astro', version: SDK_VERSION }, { name: 'npm:@sentry/browser', version: SDK_VERSION }, ], + settings: { + infer_ip: 'never', + }, }, }, }), diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index b6c05afe70f7..65561c29e9de 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -12,7 +12,6 @@ import type { import { _INTERNAL_flushLogsBuffer, addAutoIpAddressToSession, - addAutoIpAddressToUser, applySdkMetadata, Client, getSDKSource, @@ -83,6 +82,15 @@ export class BrowserClient extends Client { const sdkSource = WINDOW.SENTRY_SDK_SOURCE || getSDKSource(); applySdkMetadata(opts, 'browser', ['browser'], sdkSource); + // Only allow IP inferral by Relay if sendDefaultPii is true + if (opts._metadata?.sdk) { + opts._metadata.sdk.settings = { + infer_ip: opts.sendDefaultPii ? 'auto' : 'never', + // purposefully allowing already passed settings to override the default + ...opts._metadata.sdk.settings, + }; + } + super(opts); const { sendDefaultPii, sendClientReports, enableLogs } = this._options; @@ -117,7 +125,6 @@ export class BrowserClient extends Client { } if (sendDefaultPii) { - this.on('postprocessEvent', addAutoIpAddressToUser); this.on('beforeSendSession', addAutoIpAddressToSession); } } diff --git a/packages/browser/test/client.test.ts b/packages/browser/test/client.test.ts index 2197b6ed03c0..c6cbc735c8a1 100644 --- a/packages/browser/test/client.test.ts +++ b/packages/browser/test/client.test.ts @@ -184,3 +184,93 @@ describe('applyDefaultOptions', () => { WINDOW.SENTRY_RELEASE = releaseBefore; }); }); + +describe('SDK metadata', () => { + describe('sdk.settings', () => { + it('sets infer_ipto "never" by default', () => { + const options = getDefaultBrowserClientOptions({}); + const client = new BrowserClient(options); + + expect(client.getOptions()._metadata?.sdk?.settings?.infer_ip).toBe('never'); + }); + + it('sets infer_ip to "never" if sendDefaultPii is false', () => { + const options = getDefaultBrowserClientOptions({ + sendDefaultPii: false, + }); + const client = new BrowserClient(options); + + expect(client.getOptions()._metadata?.sdk?.settings?.infer_ip).toBe('never'); + }); + + it('sets infer_ip to "auto" if sendDefaultPii is true', () => { + const options = getDefaultBrowserClientOptions({ + sendDefaultPii: true, + }); + const client = new BrowserClient(options); + + expect(client.getOptions()._metadata?.sdk?.settings?.infer_ip).toBe('auto'); + }); + + it("doesn't override already set sdk metadata settings", () => { + const options = getDefaultBrowserClientOptions({ + sendDefaultPii: true, + _metadata: { + sdk: { + settings: { + infer_ip: 'never', + // @ts-expect-error -- not typed but let's test anyway + other_random_setting: 'some value', + }, + }, + }, + }); + const client = new BrowserClient(options); + + expect(client.getOptions()._metadata?.sdk?.settings).toEqual({ + infer_ip: 'never', + other_random_setting: 'some value', + }); + }); + + it('still sets infer_ip if other SDK metadata was already passed in', () => { + const options = getDefaultBrowserClientOptions({ + _metadata: { + sdk: { + name: 'sentry.javascript.angular', + }, + }, + }); + const client = new BrowserClient(options); + + expect(client.getOptions()._metadata?.sdk).toEqual({ + name: 'sentry.javascript.angular', + settings: { + infer_ip: 'never', + }, + }); + }); + }); + + describe('sdk data', () => { + it('sets sdk.name to "sentry.javascript.browser" by default', () => { + const options = getDefaultBrowserClientOptions({}); + const client = new BrowserClient(options); + + expect(client.getOptions()._metadata?.sdk?.name).toBe('sentry.javascript.browser'); + }); + + it("doesn't override already set sdk metadata", () => { + const options = getDefaultBrowserClientOptions({ + _metadata: { + sdk: { + name: 'sentry.javascript.angular', + }, + }, + }); + const client = new BrowserClient(options); + + expect(client.getOptions()._metadata?.sdk?.name).toBe('sentry.javascript.angular'); + }); + }); +}); diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 6ae8fa718638..875056890e0e 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -32,16 +32,31 @@ import { showSpanDropWarning, spanToJSON } from './utils/spanUtils'; /** * Apply SdkInfo (name, version, packages, integrations) to the corresponding event key. * Merge with existing data if any. + * + * @internal, exported only for testing **/ -function enhanceEventWithSdkInfo(event: Event, sdkInfo?: SdkInfo): Event { - if (!sdkInfo) { +export function _enhanceEventWithSdkInfo(event: Event, newSdkInfo?: SdkInfo): Event { + if (!newSdkInfo) { return event; } - event.sdk = event.sdk || {}; - event.sdk.name = event.sdk.name || sdkInfo.name; - event.sdk.version = event.sdk.version || sdkInfo.version; - event.sdk.integrations = [...(event.sdk.integrations || []), ...(sdkInfo.integrations || [])]; - event.sdk.packages = [...(event.sdk.packages || []), ...(sdkInfo.packages || [])]; + + const eventSdkInfo = event.sdk || {}; + + event.sdk = { + ...eventSdkInfo, + name: eventSdkInfo.name || newSdkInfo.name, + version: eventSdkInfo.version || newSdkInfo.version, + integrations: [...(event.sdk?.integrations || []), ...(newSdkInfo.integrations || [])], + packages: [...(event.sdk?.packages || []), ...(newSdkInfo.packages || [])], + settings: + event.sdk?.settings || newSdkInfo.settings + ? { + ...event.sdk?.settings, + ...newSdkInfo.settings, + } + : undefined, + }; + return event; } @@ -85,7 +100,7 @@ export function createEventEnvelope( */ const eventType = event.type && event.type !== 'replay_event' ? event.type : 'event'; - enhanceEventWithSdkInfo(event, metadata?.sdk); + _enhanceEventWithSdkInfo(event, metadata?.sdk); const envelopeHeaders = createEventEnvelopeHeaders(event, sdkInfo, tunnel, dsn); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0747258113a9..f81a6937d89c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -63,7 +63,10 @@ export { hasSpansEnabled } from './utils/hasSpansEnabled'; export { isSentryRequestUrl } from './utils/isSentryRequestUrl'; export { handleCallbackErrors } from './utils/handleCallbackErrors'; export { parameterize, fmt } from './utils/parameterize'; -export { addAutoIpAddressToSession, addAutoIpAddressToUser } from './utils/ipAddress'; + +export { addAutoIpAddressToSession } from './utils/ipAddress'; +// eslint-disable-next-line deprecation/deprecation +export { addAutoIpAddressToUser } from './utils/ipAddress'; export { convertSpanLinksForEnvelope, spanToTraceHeader, diff --git a/packages/core/src/types-hoist/sdkinfo.ts b/packages/core/src/types-hoist/sdkinfo.ts index b287ef0674f5..5750bdb37760 100644 --- a/packages/core/src/types-hoist/sdkinfo.ts +++ b/packages/core/src/types-hoist/sdkinfo.ts @@ -1,8 +1,14 @@ import type { Package } from './package'; +/** + * See https://develop.sentry.dev/sdk/data-model/event-payloads/sdk/#attributes + */ export interface SdkInfo { name?: string; version?: string; integrations?: string[]; packages?: Package[]; + settings?: { + infer_ip?: 'auto' | 'never'; + }; } diff --git a/packages/core/src/utils/ipAddress.ts b/packages/core/src/utils/ipAddress.ts index c481cd866e81..8c71835c9800 100644 --- a/packages/core/src/utils/ipAddress.ts +++ b/packages/core/src/utils/ipAddress.ts @@ -7,6 +7,7 @@ import type { User } from '../types-hoist/user'; /** * @internal + * @deprecated -- set ip inferral via via SDK metadata options on client instead. */ export function addAutoIpAddressToUser(objWithMaybeUser: { user?: User | null }): void { if (objWithMaybeUser.user?.ip_address === undefined) { diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index d0882524dc67..c5d246973842 100644 --- a/packages/core/test/lib/envelope.test.ts +++ b/packages/core/test/lib/envelope.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { DsnComponents } from '../../build/types/types-hoist/dsn'; import type { DynamicSamplingContext } from '../../build/types/types-hoist/envelope'; -import type { Client } from '../../src'; +import type { Client, SdkInfo } from '../../src'; import { getCurrentScope, getIsolationScope, @@ -10,7 +10,7 @@ import { setAsyncContextStrategy, setCurrentClient, } from '../../src'; -import { createEventEnvelope, createSpanEnvelope } from '../../src/envelope'; +import { _enhanceEventWithSdkInfo, createEventEnvelope, createSpanEnvelope } from '../../src/envelope'; import type { Event } from '../../src/types-hoist/event'; import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; @@ -261,3 +261,139 @@ describe('createSpanEnvelope', () => { }); }); }); + +describe('_enhanceEventWithSdkInfo', () => { + it('does nothing if no new sdk info is provided', () => { + const event: Event = { + sdk: { name: 'original', version: '1.0.0' }, + }; + const enhancedEvent = _enhanceEventWithSdkInfo(event, undefined); + expect(enhancedEvent.sdk).toEqual({ name: 'original', version: '1.0.0' }); + }); + + /** + * Note LS: I'm not sure if this is intended behaviour, but this is how it was before + * I made implementation changes for the `settings` object. Documenting behaviour for now, + * we can revisit it if it turns out this is not intended. + */ + it('prefers original version and name over newSdkInfo', () => { + const event: Event = { + sdk: { + name: 'original', + version: '1.0.0', + integrations: ['integration1', 'integration2'], + packages: [{ name: '@sentry/browser', version: '10.0.0' }], + }, + }; + const newSdkInfo: SdkInfo = { name: 'newName', version: '2.0.0' }; + + const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo); + + expect(enhancedEvent.sdk).toEqual({ + name: 'original', + version: '1.0.0', + integrations: ['integration1', 'integration2'], + packages: [{ name: '@sentry/browser', version: '10.0.0' }], + }); + }); + + describe('integrations and packages', () => { + it('merges integrations and packages of original and newSdkInfo', () => { + const event: Event = { + sdk: { + name: 'original', + version: '1.0.0', + integrations: ['integration1', 'integration2'], + packages: [{ name: '@sentry/browser', version: '10.0.0' }], + }, + }; + + const newSdkInfo: SdkInfo = { + name: 'newName', + version: '2.0.0', + integrations: ['integration3', 'integration4'], + packages: [{ name: '@sentry/node', version: '11.0.0' }], + }; + + const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo); + + expect(enhancedEvent.sdk).toEqual({ + name: 'original', + version: '1.0.0', + integrations: ['integration1', 'integration2', 'integration3', 'integration4'], + packages: [ + { name: '@sentry/browser', version: '10.0.0' }, + { name: '@sentry/node', version: '11.0.0' }, + ], + }); + }); + + it('creates empty integrations and packages arrays if no original or newSdkInfo are provided', () => { + const event: Event = { + sdk: { + name: 'original', + version: '1.0.0', + }, + }; + + const newSdkInfo: SdkInfo = {}; + + const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo); + expect(enhancedEvent.sdk).toEqual({ + name: 'original', + version: '1.0.0', + integrations: [], + packages: [], + }); + }); + }); + + describe('settings', () => { + it('prefers newSdkInfo settings over original settings', () => { + const event: Event = { + sdk: { + name: 'original', + version: '1.0.0', + integrations: ['integration1', 'integration2'], + packages: [{ name: '@sentry/browser', version: '10.0.0' }], + settings: { infer_ip: 'auto' }, + }, + }; + const newSdkInfo: SdkInfo = { + settings: { infer_ip: 'never' }, + }; + + const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo); + + expect(enhancedEvent.sdk).toEqual({ + name: 'original', + version: '1.0.0', + integrations: ['integration1', 'integration2'], + packages: [{ name: '@sentry/browser', version: '10.0.0' }], + settings: { infer_ip: 'never' }, + }); + }); + + it("doesn't create a `settings` object if no settings are provided", () => { + const event: Event = { + sdk: { + name: 'original', + version: '1.0.0', + }, + }; + + const newSdkInfo: SdkInfo = { + packages: [{ name: '@sentry/browser', version: '10.0.0' }], + }; + + const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo); + expect(enhancedEvent.sdk).toEqual({ + name: 'original', + version: '1.0.0', + packages: [{ name: '@sentry/browser', version: '10.0.0' }], + integrations: [], + settings: undefined, // undefined is fine because JSON.stringify omits undefined values anyways + }); + }); + }); +}); diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts index 4ecd436b709a..1975487363f7 100644 --- a/packages/nextjs/test/clientSdk.test.ts +++ b/packages/nextjs/test/clientSdk.test.ts @@ -65,6 +65,9 @@ describe('Client init()', () => { version: expect.any(String), }, ], + settings: { + infer_ip: 'never', + }, }, }, environment: 'test', diff --git a/packages/node-core/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node-core/src/integrations/http/SentryHttpInstrumentation.ts index daee4440e40c..805fb275047c 100644 --- a/packages/node-core/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node-core/src/integrations/http/SentryHttpInstrumentation.ts @@ -1,39 +1,21 @@ -/* eslint-disable max-lines */ import type { ChannelListener } from 'node:diagnostics_channel'; import { subscribe, unsubscribe } from 'node:diagnostics_channel'; import type * as http from 'node:http'; import type * as https from 'node:https'; -import type { EventEmitter } from 'node:stream'; -import { context, propagation } from '@opentelemetry/api'; +import { context } from '@opentelemetry/api'; import { isTracingSuppressed } from '@opentelemetry/core'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; -import type { AggregationCounts, Client, SanitizedRequestData, Scope } from '@sentry/core'; -import { - addBreadcrumb, - addNonEnumerableProperty, - debug, - generateSpanId, - getBreadcrumbLogLevelFromHttpStatusCode, - getClient, - getCurrentScope, - getIsolationScope, - getSanitizedUrlString, - getTraceData, - httpRequestToRequestData, - isError, - LRUMap, - parseUrl, - SDK_VERSION, - stripUrlQueryAndFragment, - withIsolationScope, -} from '@sentry/core'; -import { shouldPropagateTraceForUrl } from '@sentry/opentelemetry'; +import { debug, LRUMap, SDK_VERSION } from '@sentry/core'; import { DEBUG_BUILD } from '../../debug-build'; -import { mergeBaggageHeaders } from '../../utils/baggage'; import { getRequestUrl } from '../../utils/getRequestUrl'; - -const INSTRUMENTATION_NAME = '@sentry/instrumentation-http'; +import { INSTRUMENTATION_NAME } from './constants'; +import { instrumentServer } from './incoming-requests'; +import { + addRequestBreadcrumb, + addTracePropagationHeadersToOutgoingRequest, + getRequestOptions, +} from './outgoing-requests'; type Http = typeof http; type Https = typeof https; @@ -116,9 +98,6 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & { sessionFlushingDelayMS?: number; }; -// We only want to capture request bodies up to 1mb. -const MAX_BODY_BYTE_LENGTH = 1024 * 1024; - /** * This custom HTTP instrumentation is used to isolate incoming requests and annotate them with additional information. * It does not emit any spans. @@ -151,7 +130,12 @@ export class SentryHttpInstrumentation extends InstrumentationBase { const data = _data as { server: http.Server }; - this._patchServerEmitOnce(data.server); + instrumentServer(data.server, { + ignoreIncomingRequestBody: this.getConfig().ignoreIncomingRequestBody, + maxIncomingRequestBodySize: this.getConfig().maxIncomingRequestBodySize, + trackIncomingRequestsAsSessions: this.getConfig().trackIncomingRequestsAsSessions, + sessionFlushingDelayMS: this.getConfig().sessionFlushingDelayMS ?? 60_000, + }); }) satisfies ChannelListener; const onHttpClientResponseFinish = ((_data: unknown) => { @@ -245,142 +229,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase { - // Set a new propagationSpanId for this request - // We rely on the fact that `withIsolationScope()` will implicitly also fork the current scope - // This way we can save an "unnecessary" `withScope()` invocation - getCurrentScope().getPropagationContext().propagationSpanId = generateSpanId(); - - // If we don't want to extract the trace from the header, we can skip this - if (!instrumentation.getConfig().extractIncomingTraceFromHeader) { - return target.apply(thisArg, args); - } - - const ctx = propagation.extract(context.active(), normalizedRequest.headers); - return context.with(ctx, () => { - return target.apply(thisArg, args); - }); - }); - }, - }); - - addNonEnumerableProperty(newEmit, '__sentry_patched__', true); - - server.emit = newEmit; + addTracePropagationHeadersToOutgoingRequest(request, this._propagationDecisionMap); } /** @@ -402,262 +251,3 @@ export class SentryHttpInstrumentation extends InstrumentationBase { - try { - // `request.host` does not contain the port, but the host header does - const host = request.getHeader('host') || request.host; - const url = new URL(request.path, `${request.protocol}//${host}`); - const parsedUrl = parseUrl(url.toString()); - - const data: Partial = { - url: getSanitizedUrlString(parsedUrl), - 'http.method': request.method || 'GET', - }; - - if (parsedUrl.search) { - data['http.query'] = parsedUrl.search; - } - if (parsedUrl.hash) { - data['http.fragment'] = parsedUrl.hash; - } - - return data; - } catch { - return {}; - } -} - -/** - * This method patches the request object to capture the body. - * Instead of actually consuming the streamed body ourselves, which has potential side effects, - * we monkey patch `req.on('data')` to intercept the body chunks. - * This way, we only read the body if the user also consumes the body, ensuring we do not change any behavior in unexpected ways. - */ -function patchRequestToCaptureBody( - req: http.IncomingMessage, - isolationScope: Scope, - maxIncomingRequestBodySize: 'small' | 'medium' | 'always', -): void { - let bodyByteLength = 0; - const chunks: Buffer[] = []; - - DEBUG_BUILD && debug.log(INSTRUMENTATION_NAME, 'Patching request.on'); - - /** - * We need to keep track of the original callbacks, in order to be able to remove listeners again. - * Since `off` depends on having the exact same function reference passed in, we need to be able to map - * original listeners to our wrapped ones. - */ - const callbackMap = new WeakMap(); - - const maxBodySize = - maxIncomingRequestBodySize === 'small' - ? 1_000 - : maxIncomingRequestBodySize === 'medium' - ? 10_000 - : MAX_BODY_BYTE_LENGTH; - - try { - // eslint-disable-next-line @typescript-eslint/unbound-method - req.on = new Proxy(req.on, { - apply: (target, thisArg, args: Parameters) => { - const [event, listener, ...restArgs] = args; - - if (event === 'data') { - DEBUG_BUILD && - debug.log(INSTRUMENTATION_NAME, `Handling request.on("data") with maximum body size of ${maxBodySize}b`); - - const callback = new Proxy(listener, { - apply: (target, thisArg, args: Parameters) => { - try { - const chunk = args[0] as Buffer | string; - const bufferifiedChunk = Buffer.from(chunk); - - if (bodyByteLength < maxBodySize) { - chunks.push(bufferifiedChunk); - bodyByteLength += bufferifiedChunk.byteLength; - } else if (DEBUG_BUILD) { - debug.log( - INSTRUMENTATION_NAME, - `Dropping request body chunk because maximum body length of ${maxBodySize}b is exceeded.`, - ); - } - } catch (err) { - DEBUG_BUILD && debug.error(INSTRUMENTATION_NAME, 'Encountered error while storing body chunk.'); - } - - return Reflect.apply(target, thisArg, args); - }, - }); - - callbackMap.set(listener, callback); - - return Reflect.apply(target, thisArg, [event, callback, ...restArgs]); - } - - return Reflect.apply(target, thisArg, args); - }, - }); - - // Ensure we also remove callbacks correctly - // eslint-disable-next-line @typescript-eslint/unbound-method - req.off = new Proxy(req.off, { - apply: (target, thisArg, args: Parameters) => { - const [, listener] = args; - - const callback = callbackMap.get(listener); - if (callback) { - callbackMap.delete(listener); - - const modifiedArgs = args.slice(); - modifiedArgs[1] = callback; - return Reflect.apply(target, thisArg, modifiedArgs); - } - - return Reflect.apply(target, thisArg, args); - }, - }); - - req.on('end', () => { - try { - const body = Buffer.concat(chunks).toString('utf-8'); - if (body) { - // Using Buffer.byteLength here, because the body may contain characters that are not 1 byte long - const bodyByteLength = Buffer.byteLength(body, 'utf-8'); - const truncatedBody = - bodyByteLength > maxBodySize - ? `${Buffer.from(body) - .subarray(0, maxBodySize - 3) - .toString('utf-8')}...` - : body; - - isolationScope.setSDKProcessingMetadata({ normalizedRequest: { data: truncatedBody } }); - } - } catch (error) { - if (DEBUG_BUILD) { - debug.error(INSTRUMENTATION_NAME, 'Error building captured request body', error); - } - } - }); - } catch (error) { - if (DEBUG_BUILD) { - debug.error(INSTRUMENTATION_NAME, 'Error patching request to capture body', error); - } - } -} - -function getRequestOptions(request: http.ClientRequest): http.RequestOptions { - return { - method: request.method, - protocol: request.protocol, - host: request.host, - hostname: request.host, - path: request.path, - headers: request.getHeaders(), - }; -} - -/** - * Starts a session and tracks it in the context of a given isolation scope. - * When the passed response is finished, the session is put into a task and is - * aggregated with other sessions that may happen in a certain time window - * (sessionFlushingDelayMs). - * - * The sessions are always aggregated by the client that is on the current scope - * at the time of ending the response (if there is one). - */ -// Exported for unit tests -export function recordRequestSession({ - requestIsolationScope, - response, - sessionFlushingDelayMS, -}: { - requestIsolationScope: Scope; - response: EventEmitter; - sessionFlushingDelayMS?: number; -}): void { - requestIsolationScope.setSDKProcessingMetadata({ - requestSession: { status: 'ok' }, - }); - response.once('close', () => { - // We need to grab the client off the current scope instead of the isolation scope because the isolation scope doesn't hold any client out of the box. - const client = getClient(); - const requestSession = requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession; - - if (client && requestSession) { - DEBUG_BUILD && debug.log(`Recorded request session with status: ${requestSession.status}`); - - const roundedDate = new Date(); - roundedDate.setSeconds(0, 0); - const dateBucketKey = roundedDate.toISOString(); - - const existingClientAggregate = clientToRequestSessionAggregatesMap.get(client); - const bucket = existingClientAggregate?.[dateBucketKey] || { exited: 0, crashed: 0, errored: 0 }; - bucket[({ ok: 'exited', crashed: 'crashed', errored: 'errored' } as const)[requestSession.status]]++; - - if (existingClientAggregate) { - existingClientAggregate[dateBucketKey] = bucket; - } else { - DEBUG_BUILD && debug.log('Opened new request session aggregate.'); - const newClientAggregate = { [dateBucketKey]: bucket }; - clientToRequestSessionAggregatesMap.set(client, newClientAggregate); - - const flushPendingClientAggregates = (): void => { - clearTimeout(timeout); - unregisterClientFlushHook(); - clientToRequestSessionAggregatesMap.delete(client); - - const aggregatePayload: AggregationCounts[] = Object.entries(newClientAggregate).map( - ([timestamp, value]) => ({ - started: timestamp, - exited: value.exited, - errored: value.errored, - crashed: value.crashed, - }), - ); - client.sendSession({ aggregates: aggregatePayload }); - }; - - const unregisterClientFlushHook = client.on('flush', () => { - DEBUG_BUILD && debug.log('Sending request session aggregate due to client flush'); - flushPendingClientAggregates(); - }); - const timeout = setTimeout(() => { - DEBUG_BUILD && debug.log('Sending request session aggregate due to flushing schedule'); - flushPendingClientAggregates(); - }, sessionFlushingDelayMS).unref(); - } - } - }); -} - -const clientToRequestSessionAggregatesMap = new Map< - Client, - { [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } } ->(); diff --git a/packages/node-core/src/integrations/http/constants.ts b/packages/node-core/src/integrations/http/constants.ts new file mode 100644 index 000000000000..6ad7b4319758 --- /dev/null +++ b/packages/node-core/src/integrations/http/constants.ts @@ -0,0 +1,4 @@ +export const INSTRUMENTATION_NAME = '@sentry/instrumentation-http'; + +/** We only want to capture request bodies up to 1mb. */ +export const MAX_BODY_BYTE_LENGTH = 1024 * 1024; diff --git a/packages/node-core/src/integrations/http/incoming-requests.ts b/packages/node-core/src/integrations/http/incoming-requests.ts new file mode 100644 index 000000000000..2d18d1c064c4 --- /dev/null +++ b/packages/node-core/src/integrations/http/incoming-requests.ts @@ -0,0 +1,304 @@ +import { context, propagation } from '@opentelemetry/api'; +import type { AggregationCounts, Client, Scope } from '@sentry/core'; +import { + addNonEnumerableProperty, + debug, + generateSpanId, + getClient, + getCurrentScope, + getIsolationScope, + httpRequestToRequestData, + stripUrlQueryAndFragment, + withIsolationScope, +} from '@sentry/core'; +import type EventEmitter from 'events'; +import type { IncomingMessage, OutgoingMessage, Server } from 'http'; +import { DEBUG_BUILD } from '../../debug-build'; +import { INSTRUMENTATION_NAME, MAX_BODY_BYTE_LENGTH } from './constants'; + +const clientToRequestSessionAggregatesMap = new Map< + Client, + { [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } } +>(); + +/** + * Instrument a server to capture incoming requests. + * + */ +export function instrumentServer( + server: Server, + { + ignoreIncomingRequestBody, + maxIncomingRequestBodySize = 'medium', + trackIncomingRequestsAsSessions = true, + sessionFlushingDelayMS, + }: { + ignoreIncomingRequestBody?: (url: string, request: IncomingMessage) => boolean; + maxIncomingRequestBodySize?: 'small' | 'medium' | 'always' | 'none'; + trackIncomingRequestsAsSessions?: boolean; + sessionFlushingDelayMS: number; + }, +): void { + // eslint-disable-next-line @typescript-eslint/unbound-method + const originalEmit = server.emit; + + // This means it was already patched, do nothing + if ((originalEmit as { __sentry_patched__?: boolean }).__sentry_patched__) { + return; + } + + const newEmit = new Proxy(originalEmit, { + apply(target, thisArg, args: [event: string, ...args: unknown[]]) { + // Only traces request events + if (args[0] !== 'request') { + return target.apply(thisArg, args); + } + + DEBUG_BUILD && debug.log(INSTRUMENTATION_NAME, 'Handling incoming request'); + + const isolationScope = getIsolationScope().clone(); + const request = args[1] as IncomingMessage; + const response = args[2] as OutgoingMessage; + + const normalizedRequest = httpRequestToRequestData(request); + + // request.ip is non-standard but some frameworks set this + const ipAddress = (request as { ip?: string }).ip || request.socket?.remoteAddress; + + const url = request.url || '/'; + if (!ignoreIncomingRequestBody?.(url, request) && maxIncomingRequestBodySize !== 'none') { + patchRequestToCaptureBody(request, isolationScope, maxIncomingRequestBodySize); + } + + // Update the isolation scope, isolate this request + isolationScope.setSDKProcessingMetadata({ normalizedRequest, ipAddress }); + + // attempt to update the scope's `transactionName` based on the request URL + // Ideally, framework instrumentations coming after the HttpInstrumentation + // update the transactionName once we get a parameterized route. + const httpMethod = (request.method || 'GET').toUpperCase(); + const httpTarget = stripUrlQueryAndFragment(url); + + const bestEffortTransactionName = `${httpMethod} ${httpTarget}`; + + isolationScope.setTransactionName(bestEffortTransactionName); + + if (trackIncomingRequestsAsSessions !== false) { + recordRequestSession({ + requestIsolationScope: isolationScope, + response, + sessionFlushingDelayMS: sessionFlushingDelayMS ?? 60_000, + }); + } + + return withIsolationScope(isolationScope, () => { + // Set a new propagationSpanId for this request + // We rely on the fact that `withIsolationScope()` will implicitly also fork the current scope + // This way we can save an "unnecessary" `withScope()` invocation + getCurrentScope().getPropagationContext().propagationSpanId = generateSpanId(); + + const ctx = propagation.extract(context.active(), normalizedRequest.headers); + return context.with(ctx, () => { + return target.apply(thisArg, args); + }); + }); + }, + }); + + addNonEnumerableProperty(newEmit, '__sentry_patched__', true); + + server.emit = newEmit; +} + +/** + * Starts a session and tracks it in the context of a given isolation scope. + * When the passed response is finished, the session is put into a task and is + * aggregated with other sessions that may happen in a certain time window + * (sessionFlushingDelayMs). + * + * The sessions are always aggregated by the client that is on the current scope + * at the time of ending the response (if there is one). + */ +// Exported for unit tests +export function recordRequestSession({ + requestIsolationScope, + response, + sessionFlushingDelayMS, +}: { + requestIsolationScope: Scope; + response: EventEmitter; + sessionFlushingDelayMS?: number; +}): void { + requestIsolationScope.setSDKProcessingMetadata({ + requestSession: { status: 'ok' }, + }); + response.once('close', () => { + // We need to grab the client off the current scope instead of the isolation scope because the isolation scope doesn't hold any client out of the box. + const client = getClient(); + const requestSession = requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession; + + if (client && requestSession) { + DEBUG_BUILD && debug.log(`Recorded request session with status: ${requestSession.status}`); + + const roundedDate = new Date(); + roundedDate.setSeconds(0, 0); + const dateBucketKey = roundedDate.toISOString(); + + const existingClientAggregate = clientToRequestSessionAggregatesMap.get(client); + const bucket = existingClientAggregate?.[dateBucketKey] || { exited: 0, crashed: 0, errored: 0 }; + bucket[({ ok: 'exited', crashed: 'crashed', errored: 'errored' } as const)[requestSession.status]]++; + + if (existingClientAggregate) { + existingClientAggregate[dateBucketKey] = bucket; + } else { + DEBUG_BUILD && debug.log('Opened new request session aggregate.'); + const newClientAggregate = { [dateBucketKey]: bucket }; + clientToRequestSessionAggregatesMap.set(client, newClientAggregate); + + const flushPendingClientAggregates = (): void => { + clearTimeout(timeout); + unregisterClientFlushHook(); + clientToRequestSessionAggregatesMap.delete(client); + + const aggregatePayload: AggregationCounts[] = Object.entries(newClientAggregate).map( + ([timestamp, value]) => ({ + started: timestamp, + exited: value.exited, + errored: value.errored, + crashed: value.crashed, + }), + ); + client.sendSession({ aggregates: aggregatePayload }); + }; + + const unregisterClientFlushHook = client.on('flush', () => { + DEBUG_BUILD && debug.log('Sending request session aggregate due to client flush'); + flushPendingClientAggregates(); + }); + const timeout = setTimeout(() => { + DEBUG_BUILD && debug.log('Sending request session aggregate due to flushing schedule'); + flushPendingClientAggregates(); + }, sessionFlushingDelayMS).unref(); + } + } + }); +} + +/** + * This method patches the request object to capture the body. + * Instead of actually consuming the streamed body ourselves, which has potential side effects, + * we monkey patch `req.on('data')` to intercept the body chunks. + * This way, we only read the body if the user also consumes the body, ensuring we do not change any behavior in unexpected ways. + */ +function patchRequestToCaptureBody( + req: IncomingMessage, + isolationScope: Scope, + maxIncomingRequestBodySize: 'small' | 'medium' | 'always', +): void { + let bodyByteLength = 0; + const chunks: Buffer[] = []; + + DEBUG_BUILD && debug.log(INSTRUMENTATION_NAME, 'Patching request.on'); + + /** + * We need to keep track of the original callbacks, in order to be able to remove listeners again. + * Since `off` depends on having the exact same function reference passed in, we need to be able to map + * original listeners to our wrapped ones. + */ + const callbackMap = new WeakMap(); + + const maxBodySize = + maxIncomingRequestBodySize === 'small' + ? 1_000 + : maxIncomingRequestBodySize === 'medium' + ? 10_000 + : MAX_BODY_BYTE_LENGTH; + + try { + // eslint-disable-next-line @typescript-eslint/unbound-method + req.on = new Proxy(req.on, { + apply: (target, thisArg, args: Parameters) => { + const [event, listener, ...restArgs] = args; + + if (event === 'data') { + DEBUG_BUILD && + debug.log(INSTRUMENTATION_NAME, `Handling request.on("data") with maximum body size of ${maxBodySize}b`); + + const callback = new Proxy(listener, { + apply: (target, thisArg, args: Parameters) => { + try { + const chunk = args[0] as Buffer | string; + const bufferifiedChunk = Buffer.from(chunk); + + if (bodyByteLength < maxBodySize) { + chunks.push(bufferifiedChunk); + bodyByteLength += bufferifiedChunk.byteLength; + } else if (DEBUG_BUILD) { + debug.log( + INSTRUMENTATION_NAME, + `Dropping request body chunk because maximum body length of ${maxBodySize}b is exceeded.`, + ); + } + } catch (err) { + DEBUG_BUILD && debug.error(INSTRUMENTATION_NAME, 'Encountered error while storing body chunk.'); + } + + return Reflect.apply(target, thisArg, args); + }, + }); + + callbackMap.set(listener, callback); + + return Reflect.apply(target, thisArg, [event, callback, ...restArgs]); + } + + return Reflect.apply(target, thisArg, args); + }, + }); + + // Ensure we also remove callbacks correctly + // eslint-disable-next-line @typescript-eslint/unbound-method + req.off = new Proxy(req.off, { + apply: (target, thisArg, args: Parameters) => { + const [, listener] = args; + + const callback = callbackMap.get(listener); + if (callback) { + callbackMap.delete(listener); + + const modifiedArgs = args.slice(); + modifiedArgs[1] = callback; + return Reflect.apply(target, thisArg, modifiedArgs); + } + + return Reflect.apply(target, thisArg, args); + }, + }); + + req.on('end', () => { + try { + const body = Buffer.concat(chunks).toString('utf-8'); + if (body) { + // Using Buffer.byteLength here, because the body may contain characters that are not 1 byte long + const bodyByteLength = Buffer.byteLength(body, 'utf-8'); + const truncatedBody = + bodyByteLength > maxBodySize + ? `${Buffer.from(body) + .subarray(0, maxBodySize - 3) + .toString('utf-8')}...` + : body; + + isolationScope.setSDKProcessingMetadata({ normalizedRequest: { data: truncatedBody } }); + } + } catch (error) { + if (DEBUG_BUILD) { + debug.error(INSTRUMENTATION_NAME, 'Error building captured request body', error); + } + } + }); + } catch (error) { + if (DEBUG_BUILD) { + debug.error(INSTRUMENTATION_NAME, 'Error patching request to capture body', error); + } + } +} diff --git a/packages/node-core/src/integrations/http/outgoing-requests.ts b/packages/node-core/src/integrations/http/outgoing-requests.ts new file mode 100644 index 000000000000..17f806ab322f --- /dev/null +++ b/packages/node-core/src/integrations/http/outgoing-requests.ts @@ -0,0 +1,144 @@ +import type { LRUMap, SanitizedRequestData } from '@sentry/core'; +import { + addBreadcrumb, + debug, + getBreadcrumbLogLevelFromHttpStatusCode, + getClient, + getSanitizedUrlString, + getTraceData, + isError, + parseUrl, +} from '@sentry/core'; +import { shouldPropagateTraceForUrl } from '@sentry/opentelemetry'; +import type { ClientRequest, IncomingMessage, RequestOptions } from 'http'; +import { DEBUG_BUILD } from '../../debug-build'; +import { mergeBaggageHeaders } from '../../utils/baggage'; +import { INSTRUMENTATION_NAME } from './constants'; + +/** Add a breadcrumb for outgoing requests. */ +export function addRequestBreadcrumb(request: ClientRequest, response: IncomingMessage | undefined): void { + const data = getBreadcrumbData(request); + + const statusCode = response?.statusCode; + const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode); + + addBreadcrumb( + { + category: 'http', + data: { + status_code: statusCode, + ...data, + }, + type: 'http', + level, + }, + { + event: 'response', + request, + response, + }, + ); +} + +/** + * Add trace propagation headers to an outgoing request. + * This must be called _before_ the request is sent! + */ +export function addTracePropagationHeadersToOutgoingRequest( + request: ClientRequest, + propagationDecisionMap: LRUMap, +): void { + const url = getRequestUrl(request); + + // Manually add the trace headers, if it applies + // Note: We do not use `propagation.inject()` here, because our propagator relies on an active span + // Which we do not have in this case + const tracePropagationTargets = getClient()?.getOptions().tracePropagationTargets; + const headersToAdd = shouldPropagateTraceForUrl(url, tracePropagationTargets, propagationDecisionMap) + ? getTraceData() + : undefined; + + if (!headersToAdd) { + return; + } + + const { 'sentry-trace': sentryTrace, baggage } = headersToAdd; + + // We do not want to overwrite existing header here, if it was already set + if (sentryTrace && !request.getHeader('sentry-trace')) { + try { + request.setHeader('sentry-trace', sentryTrace); + DEBUG_BUILD && debug.log(INSTRUMENTATION_NAME, 'Added sentry-trace header to outgoing request'); + } catch (error) { + DEBUG_BUILD && + debug.error( + INSTRUMENTATION_NAME, + 'Failed to add sentry-trace header to outgoing request:', + isError(error) ? error.message : 'Unknown error', + ); + } + } + + if (baggage) { + // For baggage, we make sure to merge this into a possibly existing header + const newBaggage = mergeBaggageHeaders(request.getHeader('baggage'), baggage); + if (newBaggage) { + try { + request.setHeader('baggage', newBaggage); + DEBUG_BUILD && debug.log(INSTRUMENTATION_NAME, 'Added baggage header to outgoing request'); + } catch (error) { + DEBUG_BUILD && + debug.error( + INSTRUMENTATION_NAME, + 'Failed to add baggage header to outgoing request:', + isError(error) ? error.message : 'Unknown error', + ); + } + } + } +} + +function getBreadcrumbData(request: ClientRequest): Partial { + try { + // `request.host` does not contain the port, but the host header does + const host = request.getHeader('host') || request.host; + const url = new URL(request.path, `${request.protocol}//${host}`); + const parsedUrl = parseUrl(url.toString()); + + const data: Partial = { + url: getSanitizedUrlString(parsedUrl), + 'http.method': request.method || 'GET', + }; + + if (parsedUrl.search) { + data['http.query'] = parsedUrl.search; + } + if (parsedUrl.hash) { + data['http.fragment'] = parsedUrl.hash; + } + + return data; + } catch { + return {}; + } +} + +/** Convert an outgoing request to request options. */ +export function getRequestOptions(request: ClientRequest): RequestOptions { + return { + method: request.method, + protocol: request.protocol, + host: request.host, + hostname: request.host, + path: request.path, + headers: request.getHeaders(), + }; +} + +function getRequestUrl(request: ClientRequest): string { + const hostname = request.getHeader('host') || request.host; + const protocol = request.protocol; + const path = request.path; + + return `${protocol}//${hostname}${path}`; +} diff --git a/packages/node-core/test/integrations/request-session-tracking.test.ts b/packages/node-core/test/integrations/request-session-tracking.test.ts index 02446eee875d..b7d7ec4f2354 100644 --- a/packages/node-core/test/integrations/request-session-tracking.test.ts +++ b/packages/node-core/test/integrations/request-session-tracking.test.ts @@ -2,7 +2,7 @@ import type { Client } from '@sentry/core'; import { createTransport, Scope, ServerRuntimeClient, withScope } from '@sentry/core'; import { EventEmitter } from 'stream'; import { describe, expect, it, vi } from 'vitest'; -import { recordRequestSession } from '../../src/integrations/http/SentryHttpInstrumentation'; +import { recordRequestSession } from '../../src/integrations/http/incoming-requests'; vi.useFakeTimers(); diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http.ts similarity index 91% rename from packages/node/src/integrations/http/index.ts rename to packages/node/src/integrations/http.ts index e56842be85cb..4f1eeeea9e9c 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http.ts @@ -3,7 +3,7 @@ import { diag } from '@opentelemetry/api'; import type { HttpInstrumentationConfig } from '@opentelemetry/instrumentation-http'; import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; import type { Span } from '@sentry/core'; -import { defineIntegration, getClient, hasSpansEnabled } from '@sentry/core'; +import { defineIntegration, getClient, hasSpansEnabled, stripUrlQueryAndFragment } from '@sentry/core'; import type { HTTPModuleRequestIncomingMessage, NodeClient } from '@sentry/node-core'; import { type SentryHttpInstrumentationOptions, @@ -13,7 +13,7 @@ import { NODE_VERSION, SentryHttpInstrumentation, } from '@sentry/node-core'; -import type { NodeClientOptions } from '../../types'; +import type { NodeClientOptions } from '../types'; const INTEGRATION_NAME = 'Http'; @@ -74,6 +74,14 @@ interface HttpOptions { */ ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean; + /** + * Whether to automatically ignore common static asset requests like favicon.ico, robots.txt, etc. + * This helps reduce noise in your transactions. + * + * @default `true` + */ + ignoreStaticAssets?: boolean; + /** * Do not capture spans for incoming HTTP requests with the given status codes. * By default, spans with 404 status code are ignored. @@ -284,6 +292,11 @@ function getConfigWithDefaults(options: Partial = {}): HttpInstrume return true; } + // Default static asset filtering + if (options.ignoreStaticAssets !== false && method === 'GET' && urlPath && isStaticAssetRequest(urlPath)) { + return true; + } + const _ignoreIncomingRequests = options.ignoreIncomingRequests; if (urlPath && _ignoreIncomingRequests?.(urlPath, request)) { return true; @@ -316,3 +329,23 @@ function getConfigWithDefaults(options: Partial = {}): HttpInstrume return instrumentationConfig; } + +/** + * Check if a request is for a common static asset that should be ignored by default. + * + * Only exported for tests. + */ +export function isStaticAssetRequest(urlPath: string): boolean { + const path = stripUrlQueryAndFragment(urlPath); + // Common static file extensions + if (path.match(/\.(ico|png|jpg|jpeg|gif|svg|css|js|woff|woff2|ttf|eot|webp|avif)$/)) { + return true; + } + + // Common metadata files + if (path.match(/^\/(robots\.txt|sitemap\.xml|manifest\.json|browserconfig\.xml)$/)) { + return true; + } + + return false; +} diff --git a/packages/node/src/integrations/node-fetch/index.ts b/packages/node/src/integrations/node-fetch.ts similarity index 98% rename from packages/node/src/integrations/node-fetch/index.ts rename to packages/node/src/integrations/node-fetch.ts index 7929d00c87e0..437806e16dbc 100644 --- a/packages/node/src/integrations/node-fetch/index.ts +++ b/packages/node/src/integrations/node-fetch.ts @@ -4,7 +4,7 @@ import type { IntegrationFn } from '@sentry/core'; import { defineIntegration, getClient, hasSpansEnabled, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import type { NodeClient } from '@sentry/node-core'; import { generateInstrumentOnce, SentryNodeFetchInstrumentation } from '@sentry/node-core'; -import type { NodeClientOptions } from '../../types'; +import type { NodeClientOptions } from '../types'; const INTEGRATION_NAME = 'NodeFetch'; diff --git a/packages/node/src/integrations/node-fetch/types.ts b/packages/node/src/integrations/node-fetch/types.ts deleted file mode 100644 index 0139dadde413..000000000000 --- a/packages/node/src/integrations/node-fetch/types.ts +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * Vendored from https://github.com/open-telemetry/opentelemetry-js-contrib/blob/28e209a9da36bc4e1f8c2b0db7360170ed46cb80/plugins/node/instrumentation-undici/src/types.ts - */ - -export interface UndiciRequest { - origin: string; - method: string; - path: string; - /** - * Serialized string of headers in the form `name: value\r\n` for v5 - * Array of strings v6 - */ - headers: string | string[]; - /** - * Helper method to add headers (from v6) - */ - addHeader: (name: string, value: string) => void; - throwOnError: boolean; - completed: boolean; - aborted: boolean; - idempotent: boolean; - contentLength: number | null; - contentType: string | null; - body: unknown; -} - -export interface UndiciResponse { - headers: Buffer[]; - statusCode: number; - statusText: string; -} diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 89052a348ea4..2a24464c801c 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { _shouldInstrumentSpans } from '../../src/integrations/http'; +import { _shouldInstrumentSpans, isStaticAssetRequest } from '../../src/integrations/http'; import { conditionalTest } from '../helpers/conditional'; describe('httpIntegration', () => { @@ -27,4 +27,34 @@ describe('httpIntegration', () => { expect(actual).toBe(true); }); }); + + describe('isStaticAssetRequest', () => { + it.each([ + ['/favicon.ico', true], + ['/apple-touch-icon.png', true], + ['/static/style.css', true], + ['/assets/app.js', true], + ['/fonts/font.woff2', true], + ['/images/logo.svg', true], + ['/img/photo.jpeg', true], + ['/img/photo.jpg', true], + ['/img/photo.jpg?v=123', true], + ['/img/photo.webp', true], + ['/font/font.ttf', true], + ['/robots.txt', true], + ['/sitemap.xml', true], + ['/manifest.json', true], + ['/browserconfig.xml', true], + // non-static routes + ['/api/users', false], + ['/some-json.json', false], + ['/some-xml.xml', false], + ['/some-txt.txt', false], + ['/users', false], + ['/graphql', false], + ['/', false], + ])('returns %s -> %s', (urlPath, expected) => { + expect(isStaticAssetRequest(urlPath)).toBe(expected); + }); + }); }); diff --git a/packages/nuxt/src/server/sdk.ts b/packages/nuxt/src/server/sdk.ts index ed04267a2536..dcd2f46caec9 100644 --- a/packages/nuxt/src/server/sdk.ts +++ b/packages/nuxt/src/server/sdk.ts @@ -43,6 +43,14 @@ export function lowQualityTransactionsFilter(options: SentryNuxtServerOptions): if (event.type !== 'transaction' || !event.transaction) { return event; } + + // Check if this looks like a parametrized route (contains :param or :param() patterns) + const hasRouteParameters = /\/:[^(/\s]*(\([^)]*\))?[^/\s]*/.test(event.transaction); + + if (hasRouteParameters) { + return event; + } + // We don't want to send transaction for file requests, so everything ending with a *.someExtension should be filtered out // path.extname will return an empty string for normal page requests if (path.extname(event.transaction)) { diff --git a/packages/nuxt/test/client/sdk.test.ts b/packages/nuxt/test/client/sdk.test.ts index 1a79cfe445a7..29448c720ea4 100644 --- a/packages/nuxt/test/client/sdk.test.ts +++ b/packages/nuxt/test/client/sdk.test.ts @@ -27,6 +27,9 @@ describe('Nuxt Client SDK', () => { { name: 'npm:@sentry/nuxt', version: SDK_VERSION }, { name: 'npm:@sentry/vue', version: SDK_VERSION }, ], + settings: { + infer_ip: 'never', + }, }, }, }; diff --git a/packages/nuxt/test/server/sdk.test.ts b/packages/nuxt/test/server/sdk.test.ts index 7139b82e30ec..7efe86b84587 100644 --- a/packages/nuxt/test/server/sdk.test.ts +++ b/packages/nuxt/test/server/sdk.test.ts @@ -42,7 +42,7 @@ describe('Nuxt Server SDK', () => { expect(init({})).not.toBeUndefined(); }); - describe('low quality transactions filter (%s)', () => { + describe('lowQualityTransactionsFilter (%s)', () => { const beforeSendEvent = vi.fn(event => event); const client = init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', @@ -63,7 +63,8 @@ describe('Nuxt Server SDK', () => { expect(beforeSendEvent).not.toHaveBeenCalled(); }); - it.each(['GET /', 'POST /_server'])( + // Nuxt parametrizes routes sometimes in a special way - especially catchAll o.O + it.each(['GET /', 'POST /_server', 'GET /catchAll/:id(.*)*', 'GET /article/:slug()', 'GET /user/:id'])( 'does not filter out high quality or route transactions (%s)', async transaction => { client.captureEvent({ type: 'transaction', transaction }); diff --git a/packages/react-router/test/client/sdk.test.ts b/packages/react-router/test/client/sdk.test.ts index d6767ccfff23..46c629a0bbd5 100644 --- a/packages/react-router/test/client/sdk.test.ts +++ b/packages/react-router/test/client/sdk.test.ts @@ -33,6 +33,9 @@ describe('React Router client SDK', () => { { name: 'npm:@sentry/react-router', version: SDK_VERSION }, { name: 'npm:@sentry/browser', version: SDK_VERSION }, ], + settings: { + infer_ip: 'never', + }, }, }, }; diff --git a/packages/remix/test/index.client.test.ts b/packages/remix/test/index.client.test.ts index 2497ce281694..c5af95a420ef 100644 --- a/packages/remix/test/index.client.test.ts +++ b/packages/remix/test/index.client.test.ts @@ -36,6 +36,9 @@ describe('Client init()', () => { version: expect.any(String), }, ], + settings: { + infer_ip: 'never', + }, }, }, }), diff --git a/packages/replay-internal/src/util/prepareReplayEvent.ts b/packages/replay-internal/src/util/prepareReplayEvent.ts index 7de2cff212c0..e9cdd050e122 100644 --- a/packages/replay-internal/src/util/prepareReplayEvent.ts +++ b/packages/replay-internal/src/util/prepareReplayEvent.ts @@ -49,12 +49,13 @@ export async function prepareReplayEvent({ // extract the SDK name because `client._prepareEvent` doesn't add it to the event const metadata = client.getSdkMetadata(); - const { name, version } = metadata?.sdk || {}; + const { name, version, settings } = metadata?.sdk || {}; preparedEvent.sdk = { ...preparedEvent.sdk, name: name || 'sentry.javascript.unknown', version: version || '0.0.0', + settings, }; return preparedEvent; diff --git a/packages/solid/test/sdk.test.ts b/packages/solid/test/sdk.test.ts index dec8220668a8..cbef43425a32 100644 --- a/packages/solid/test/sdk.test.ts +++ b/packages/solid/test/sdk.test.ts @@ -21,6 +21,9 @@ describe('Initialize Solid SDK', () => { name: 'sentry.javascript.solid', packages: [{ name: 'npm:@sentry/solid', version: SDK_VERSION }], version: SDK_VERSION, + settings: { + infer_ip: 'never', + }, }, }, }; diff --git a/packages/solidstart/test/client/sdk.test.ts b/packages/solidstart/test/client/sdk.test.ts index 73bb412d1909..159cc6da8bcb 100644 --- a/packages/solidstart/test/client/sdk.test.ts +++ b/packages/solidstart/test/client/sdk.test.ts @@ -25,6 +25,9 @@ describe('Initialize Solid Start SDK', () => { { name: 'npm:@sentry/solid', version: SDK_VERSION }, ], version: SDK_VERSION, + settings: { + infer_ip: 'never', + }, }, }, }; diff --git a/packages/svelte/test/sdk.test.ts b/packages/svelte/test/sdk.test.ts index 725d9bc66898..665805b95714 100644 --- a/packages/svelte/test/sdk.test.ts +++ b/packages/svelte/test/sdk.test.ts @@ -25,6 +25,9 @@ describe('Initialize Svelte SDk', () => { name: 'sentry.javascript.svelte', packages: [{ name: 'npm:@sentry/svelte', version: SDK_VERSION }], version: SDK_VERSION, + settings: { + infer_ip: 'never', + }, }, }, }; @@ -44,6 +47,9 @@ describe('Initialize Svelte SDk', () => { { name: 'npm:@sentry/sveltekit', version: SDK_VERSION }, { name: 'npm:@sentry/svelte', version: SDK_VERSION }, ], + settings: { + infer_ip: 'never', + }, }, }, }); @@ -59,6 +65,9 @@ describe('Initialize Svelte SDk', () => { { name: 'npm:@sentry/sveltekit', version: SDK_VERSION }, { name: 'npm:@sentry/svelte', version: SDK_VERSION }, ], + settings: { + infer_ip: 'never', + }, }, }, }), diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts index 1bbd2e2bc81f..91bc44d77b21 100644 --- a/packages/sveltekit/test/client/sdk.test.ts +++ b/packages/sveltekit/test/client/sdk.test.ts @@ -33,6 +33,9 @@ describe('Sentry client SDK', () => { { name: 'npm:@sentry/sveltekit', version: SDK_VERSION }, { name: 'npm:@sentry/svelte', version: SDK_VERSION }, ], + settings: { + infer_ip: 'never', + }, }, }, }),