From fbe91bba78bbfce15d1e062ab0d832661b3d95dd Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 14 Jan 2025 16:25:06 +0100 Subject: [PATCH 1/6] feat(browser): Set `user.ip_address` explicitly to `{{auto}}` --- packages/browser/src/client.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 8582f92056be..288f612703ad 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -112,6 +112,15 @@ export class BrowserClient extends Client { isolationScope: Scope, ): PromiseLike { event.platform = event.platform || 'javascript'; + + // By default, we want to infer the IP address, unless this is explicitly set to `null` + if (typeof event.user?.ip_address === 'undefined') { + event.user = { + ...event.user, + ip_address: '{{auto}}', + }; + } + return super._prepareEvent(event, hint, currentScope, isolationScope); } } From d704c9e31a019eb98b3f864f5bd5a3c67da0ebd8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 15 Jan 2025 09:28:11 +0100 Subject: [PATCH 2/6] fix & tests --- .../suites/feedback/attachTo/test.ts | 3 +++ .../suites/feedback/captureFeedback/test.ts | 3 +++ .../feedback/captureFeedbackCsp/test.ts | 3 +++ .../manual-client/browser-context/test.ts | 3 +++ .../setUser/ip_address_null/subject.js | 6 +++++ .../setUser/ip_address_null/test.ts | 17 +++++++++++++ .../public-api/setUser/unset_user/test.ts | 8 ++++--- .../public-api/setUser/update_user/test.ts | 5 ++-- .../withScope/nested_scopes/test.ts | 8 +++---- packages/browser/src/client.ts | 24 ++++++++++++------- 10 files changed, 63 insertions(+), 17 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/public-api/setUser/ip_address_null/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/setUser/ip_address_null/test.ts 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 a9888ad6edf6..177d8cc2994f 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/attachTo/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/attachTo/test.ts @@ -57,6 +57,9 @@ sentryTest('should capture feedback with custom button', async ({ getLocalTestUr event_id: expect.stringMatching(/\w{32}/), environment: 'production', tags: {}, + user: { + ip_address: '{{auto}}', + }, sdk: { integrations: expect.arrayContaining(['Feedback']), version: expect.any(String), 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 138e7225a7c8..68c45e2c2f2f 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts @@ -69,6 +69,9 @@ sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => { 'User-Agent': expect.stringContaining(''), }, }, + user: { + ip_address: '{{auto}}', + }, platform: 'javascript', }); }); 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 77eadb71173a..56ba6606c724 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts @@ -69,6 +69,9 @@ sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => { 'User-Agent': expect.stringContaining(''), }, }, + user: { + ip_address: '{{auto}}', + }, platform: 'javascript', }); const cspViolation = await page.evaluate('window.__CSPVIOLATION__'); 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 642006f4c2fc..a220ba2923d6 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 @@ -34,6 +34,9 @@ sentryTest('allows to setup a client manually & capture exceptions', async ({ ge 'User-Agent': expect.any(String), }), }, + user: { + ip_address: '{{auto}}', + }, timestamp: expect.any(Number), environment: 'local', release: '0.0.1', diff --git a/dev-packages/browser-integration-tests/suites/public-api/setUser/ip_address_null/subject.js b/dev-packages/browser-integration-tests/suites/public-api/setUser/ip_address_null/subject.js new file mode 100644 index 000000000000..a59797f44212 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/setUser/ip_address_null/subject.js @@ -0,0 +1,6 @@ +Sentry.setUser({ + id: 'foo', + ip_address: null, +}); + +Sentry.captureMessage('first_user'); diff --git a/dev-packages/browser-integration-tests/suites/public-api/setUser/ip_address_null/test.ts b/dev-packages/browser-integration-tests/suites/public-api/setUser/ip_address_null/test.ts new file mode 100644 index 000000000000..9ea891858749 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/setUser/ip_address_null/test.ts @@ -0,0 +1,17 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/core'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; + +sentryTest('should allow to set ip_address to null', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.message).toBe('first_user'); + expect(eventData.user).toEqual({ + id: 'foo', + ip_address: null, + }); +}); 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 ac3c06311bf9..43bc03bfb4d1 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 @@ -10,15 +10,17 @@ sentryTest('should unset user', async ({ getLocalTestUrl, page }) => { const eventData = await getMultipleSentryEnvelopeRequests(page, 3, { url }); expect(eventData[0].message).toBe('no_user'); - expect(eventData[0].user).toBeUndefined(); + expect(eventData[0].user).toEqual({ ip_address: '{{auto}}' }); expect(eventData[1].message).toBe('user'); - expect(eventData[1].user).toMatchObject({ + expect(eventData[1].user).toEqual({ id: 'foo', ip_address: 'bar', other_key: 'baz', }); expect(eventData[2].message).toBe('unset_user'); - expect(eventData[2].user).toBeUndefined(); + expect(eventData[2].user).toEqual({ + ip_address: '{{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 38a18daf2c2f..b799022fa4e7 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 @@ -10,13 +10,14 @@ sentryTest('should update user', async ({ getLocalTestUrl, page }) => { const eventData = await getMultipleSentryEnvelopeRequests(page, 2, { url }); expect(eventData[0].message).toBe('first_user'); - expect(eventData[0].user).toMatchObject({ + expect(eventData[0].user).toEqual({ id: 'foo', ip_address: 'bar', }); expect(eventData[1].message).toBe('second_user'); - expect(eventData[1].user).toMatchObject({ + expect(eventData[1].user).toEqual({ id: 'baz', + ip_address: '{{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 be81e06a6e40..efda6588bf73 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 @@ -10,11 +10,11 @@ sentryTest('should allow nested scoping', async ({ getLocalTestUrl, page }) => { const eventData = await getMultipleSentryEnvelopeRequests(page, 5, { url }); expect(eventData[0].message).toBe('root_before'); - expect(eventData[0].user).toMatchObject({ id: 'qux' }); + expect(eventData[0].user).toEqual({ id: 'qux' }); expect(eventData[0].tags).toBeUndefined(); expect(eventData[1].message).toBe('outer_before'); - expect(eventData[1].user).toMatchObject({ id: 'qux' }); + expect(eventData[1].user).toEqual({ id: 'qux' }); expect(eventData[1].tags).toMatchObject({ foo: false }); expect(eventData[2].message).toBe('inner'); @@ -22,10 +22,10 @@ sentryTest('should allow nested scoping', async ({ getLocalTestUrl, page }) => { expect(eventData[2].tags).toMatchObject({ foo: false, bar: 10 }); expect(eventData[3].message).toBe('outer_after'); - expect(eventData[3].user).toMatchObject({ id: 'baz' }); + expect(eventData[3].user).toEqual({ id: 'baz' }); expect(eventData[3].tags).toMatchObject({ foo: false }); expect(eventData[4].message).toBe('root_after'); - expect(eventData[4].user).toMatchObject({ id: 'qux' }); + expect(eventData[4].user).toEqual({ id: 'qux' }); expect(eventData[4].tags).toBeUndefined(); }); diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 288f612703ad..a56e1acbee53 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -113,14 +113,22 @@ export class BrowserClient extends Client { ): PromiseLike { event.platform = event.platform || 'javascript'; - // By default, we want to infer the IP address, unless this is explicitly set to `null` - if (typeof event.user?.ip_address === 'undefined') { - event.user = { - ...event.user, - ip_address: '{{auto}}', - }; - } + return super._prepareEvent(event, hint, currentScope, isolationScope).then(prepared => { + if (!prepared) { + return prepared; + } + + // By default, we want to infer the IP address, unless this is explicitly set to `null` + // We do this after all other processing is done + // If `ip_address` is explicitly set to `null` or a value, we leave it as is + if (prepared.user?.ip_address === undefined) { + prepared.user = { + ...prepared.user, + ip_address: '{{auto}}', + }; + } - return super._prepareEvent(event, hint, currentScope, isolationScope); + return prepared; + }); } } From 225a71d025121cff5841bf1563b0921f18fc66a2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 15 Jan 2025 09:48:17 +0100 Subject: [PATCH 3/6] also do for replay --- .../captureFeedbackAndReplay/hasSampling/test.ts | 3 +++ .../startSpan/standalone-mixed-transaction/test.ts | 3 +++ .../suites/public-api/withScope/nested_scopes/test.ts | 10 +++++----- .../suites/replay/captureReplay/test.ts | 3 +++ .../replay/captureReplayFromReplayPackage/test.ts | 3 +++ .../utils/replayEventTemplates.ts | 3 +++ .../replay-internal/src/util/prepareReplayEvent.ts | 10 ++++++++++ 7 files changed, 30 insertions(+), 5 deletions(-) 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 51374ac81947..c91be3511d0a 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 @@ -103,6 +103,9 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestUrl 'User-Agent': expect.stringContaining(''), }, }, + user: { + ip_address: '{{auto}}', + }, platform: 'javascript', }); }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts index 0663d16b6995..9c4c83bf6764 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts @@ -103,6 +103,9 @@ sentryTest( headers: expect.any(Object), url: expect.any(String), }, + user: { + ip_address: '{{auto}}', + }, sdk: expect.any(Object), spans: [ { 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 efda6588bf73..d75b0248bd4f 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 @@ -10,22 +10,22 @@ sentryTest('should allow nested scoping', async ({ getLocalTestUrl, page }) => { const eventData = await getMultipleSentryEnvelopeRequests(page, 5, { url }); expect(eventData[0].message).toBe('root_before'); - expect(eventData[0].user).toEqual({ id: 'qux' }); + expect(eventData[0].user).toEqual({ id: 'qux', ip_address: '{{auto}}' }); expect(eventData[0].tags).toBeUndefined(); expect(eventData[1].message).toBe('outer_before'); - expect(eventData[1].user).toEqual({ id: 'qux' }); + expect(eventData[1].user).toEqual({ id: 'qux', ip_address: '{{auto}}' }); expect(eventData[1].tags).toMatchObject({ foo: false }); expect(eventData[2].message).toBe('inner'); - expect(eventData[2].user).toBeUndefined(); + expect(eventData[2].user).toEqual({ ip_address: '{{auto}}' }); expect(eventData[2].tags).toMatchObject({ foo: false, bar: 10 }); expect(eventData[3].message).toBe('outer_after'); - expect(eventData[3].user).toEqual({ id: 'baz' }); + expect(eventData[3].user).toEqual({ id: 'baz', ip_address: '{{auto}}' }); expect(eventData[3].tags).toMatchObject({ foo: false }); expect(eventData[4].message).toBe('root_after'); - expect(eventData[4].user).toEqual({ id: 'qux' }); + expect(eventData[4].user).toEqual({ id: 'qux', ip_address: '{{auto}}' }); 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 e581a8eacd57..ebc7e3236329 100644 --- a/dev-packages/browser-integration-tests/suites/replay/captureReplay/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/captureReplay/test.ts @@ -55,6 +55,9 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT 'User-Agent': expect.stringContaining(''), }, }, + user: { + ip_address: '{{auto}}', + }, platform: 'javascript', }); 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 3a10ea72e18c..4caa304f4c93 100644 --- a/dev-packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts @@ -55,6 +55,9 @@ sentryTest('should capture replays (@sentry-internal/replay export)', async ({ g 'User-Agent': expect.stringContaining(''), }, }, + user: { + ip_address: '{{auto}}', + }, platform: 'javascript', }); diff --git a/dev-packages/browser-integration-tests/utils/replayEventTemplates.ts b/dev-packages/browser-integration-tests/utils/replayEventTemplates.ts index 52dbbca1c086..8455bcc34600 100644 --- a/dev-packages/browser-integration-tests/utils/replayEventTemplates.ts +++ b/dev-packages/browser-integration-tests/utils/replayEventTemplates.ts @@ -37,6 +37,9 @@ const DEFAULT_REPLAY_EVENT = { 'User-Agent': expect.any(String), }, }, + user: { + ip_address: '{{auto}}', + }, platform: 'javascript', }; diff --git a/packages/replay-internal/src/util/prepareReplayEvent.ts b/packages/replay-internal/src/util/prepareReplayEvent.ts index b881ce6d7e84..ca1546eb9bfa 100644 --- a/packages/replay-internal/src/util/prepareReplayEvent.ts +++ b/packages/replay-internal/src/util/prepareReplayEvent.ts @@ -55,5 +55,15 @@ export async function prepareReplayEvent({ version: version || '0.0.0', }; + // By default, we want to infer the IP address, unless this is explicitly set to `null` + // We do this after all other processing is done + // If `ip_address` is explicitly set to `null` or a value, we leave it as is + if (preparedEvent.user?.ip_address === undefined) { + preparedEvent.user = { + ...preparedEvent.user, + ip_address: '{{auto}}', + }; + } + return preparedEvent; } From 1d35352682517b53f99f8d84e7688681a6783879 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 15 Jan 2025 10:36:18 +0100 Subject: [PATCH 4/6] fix tests --- .../suites/replay/captureReplay/test.ts | 3 + .../captureReplayFromReplayPackage/test.ts | 3 + packages/core/src/types-hoist/user.ts | 2 +- .../test/unit/util/prepareReplayEvent.test.ts | 128 +++++++++++++++++- 4 files changed, 133 insertions(+), 3 deletions(-) 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 ebc7e3236329..9737a3908247 100644 --- a/dev-packages/browser-integration-tests/suites/replay/captureReplay/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/captureReplay/test.ts @@ -96,6 +96,9 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT 'User-Agent': expect.stringContaining(''), }, }, + user: { + ip_address: '{{auto}}', + }, platform: 'javascript', }); }); 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 4caa304f4c93..1c7fe09e3c33 100644 --- a/dev-packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts @@ -96,6 +96,9 @@ sentryTest('should capture replays (@sentry-internal/replay export)', async ({ g 'User-Agent': expect.stringContaining(''), }, }, + user: { + ip_address: '{{auto}}', + }, platform: 'javascript', }); }); diff --git a/packages/core/src/types-hoist/user.ts b/packages/core/src/types-hoist/user.ts index f559c5029825..801fb66202b1 100644 --- a/packages/core/src/types-hoist/user.ts +++ b/packages/core/src/types-hoist/user.ts @@ -4,7 +4,7 @@ export interface User { [key: string]: any; id?: string | number; - ip_address?: string; + ip_address?: string | null; email?: string; username?: string; geo?: GeoLocation; diff --git a/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts b/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts index 14d2e36e9a07..67b78747a29b 100644 --- a/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts +++ b/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts @@ -29,10 +29,9 @@ describe('Unit | util | prepareReplayEvent', () => { it('works', async () => { const client = getClient()!; - const scope = getCurrentScope()!; + const scope = getCurrentScope(); expect(client).toBeDefined(); - expect(scope).toBeDefined(); const replayId = 'replay-ID'; const event: ReplayEvent = { @@ -78,6 +77,131 @@ describe('Unit | util | prepareReplayEvent', () => { name: 'sentry.javascript.testSdk', version: '1.0.0', }, + user: { + ip_address: '{{auto}}', + }, + sdkProcessingMetadata: expect.any(Object), + breadcrumbs: undefined, + }); + }); + + it('sets user', async () => { + const client = getClient()!; + const scope = getCurrentScope().clone(); + + scope.setUser({ id: 'user-id' }); + + expect(client).toBeDefined(); + + const replayId = 'replay-ID'; + const event: ReplayEvent = { + type: REPLAY_EVENT_NAME, + timestamp: 1670837008.634, + error_ids: ['error-ID'], + trace_ids: ['trace-ID'], + urls: ['https://sentry.io/'], + replay_id: replayId, + replay_type: 'session', + segment_id: 3, + contexts: { + replay: { + error_sample_rate: 1.0, + session_sample_rate: 0.1, + }, + }, + }; + + const replayEvent = await prepareReplayEvent({ scope, client, replayId, event }); + + expect(client.getSdkMetadata).toHaveBeenCalledTimes(1); + + expect(replayEvent).toEqual({ + type: 'replay_event', + timestamp: 1670837008.634, + error_ids: ['error-ID'], + trace_ids: ['trace-ID'], + urls: ['https://sentry.io/'], + replay_id: 'replay-ID', + replay_type: 'session', + segment_id: 3, + platform: 'javascript', + event_id: 'replay-ID', + environment: 'production', + contexts: { + replay: { + error_sample_rate: 1.0, + session_sample_rate: 0.1, + }, + }, + sdk: { + name: 'sentry.javascript.testSdk', + version: '1.0.0', + }, + user: { + id: 'user-id', + ip_address: '{{auto}}', + }, + sdkProcessingMetadata: expect.any(Object), + breadcrumbs: undefined, + }); + }); + + it('allows to set user.ip_address=null', async () => { + const client = getClient()!; + const scope = getCurrentScope().clone(); + + scope.setUser({ id: 'user-id', ip_address: null }); + + expect(client).toBeDefined(); + + const replayId = 'replay-ID'; + const event: ReplayEvent = { + type: REPLAY_EVENT_NAME, + timestamp: 1670837008.634, + error_ids: ['error-ID'], + trace_ids: ['trace-ID'], + urls: ['https://sentry.io/'], + replay_id: replayId, + replay_type: 'session', + segment_id: 3, + contexts: { + replay: { + error_sample_rate: 1.0, + session_sample_rate: 0.1, + }, + }, + }; + + const replayEvent = await prepareReplayEvent({ scope, client, replayId, event }); + + expect(client.getSdkMetadata).toHaveBeenCalledTimes(1); + + expect(replayEvent).toEqual({ + type: 'replay_event', + timestamp: 1670837008.634, + error_ids: ['error-ID'], + trace_ids: ['trace-ID'], + urls: ['https://sentry.io/'], + replay_id: 'replay-ID', + replay_type: 'session', + segment_id: 3, + platform: 'javascript', + event_id: 'replay-ID', + environment: 'production', + contexts: { + replay: { + error_sample_rate: 1.0, + session_sample_rate: 0.1, + }, + }, + sdk: { + name: 'sentry.javascript.testSdk', + version: '1.0.0', + }, + user: { + id: 'user-id', + ip_address: null, + }, sdkProcessingMetadata: expect.any(Object), breadcrumbs: undefined, }); From a0bbfff79c3050bc1d159061092e05e876891b5f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 15 Jan 2025 14:39:14 +0100 Subject: [PATCH 5/6] fix it better?? --- packages/browser-utils/src/metrics/utils.ts | 3 + packages/browser/src/client.ts | 46 ++++++---- packages/core/src/client.ts | 32 +++++++ packages/core/src/types-hoist/session.ts | 1 + packages/profiling-node/src/integration.ts | 5 ++ .../src/util/prepareReplayEvent.ts | 12 +-- .../test/unit/util/prepareReplayEvent.test.ts | 89 ++++--------------- 7 files changed, 91 insertions(+), 97 deletions(-) diff --git a/packages/browser-utils/src/metrics/utils.ts b/packages/browser-utils/src/metrics/utils.ts index 928df83a9689..2bc97d588395 100644 --- a/packages/browser-utils/src/metrics/utils.ts +++ b/packages/browser-utils/src/metrics/utils.ts @@ -108,6 +108,9 @@ export function startStandaloneWebVitalSpan(options: StandaloneWebVitalSpanOptio // For example: Chrome vs. Chrome Mobile 'user_agent.original': WINDOW.navigator?.userAgent, + // This tells Sentry to infer the IP address from the request + 'client.address': '{{auto}}', + ...passedAttributes, }; diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index a56e1acbee53..801a1bffbd2e 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -8,6 +8,7 @@ import type { ParameterizedString, Scope, SeverityLevel, + User, } from '@sentry/core'; import { Client, applySdkMetadata, getSDKSource } from '@sentry/core'; import { eventFromException, eventFromMessage } from './eventbuilder'; @@ -82,6 +83,23 @@ export class BrowserClient extends Client { } }); } + + this.on('postprocessEvent', event => { + addAutoIpAddressToUser(event); + }); + + this.on('beforeSendSession', session => { + if ('aggregates' in session) { + if (session.attrs?.['ip_address'] === undefined) { + session.attrs = { + ...session.attrs, + ip_address: '{{auto}}', + }; + } + } else { + addAutoIpAddressToUser(session); + } + }); } /** @@ -113,22 +131,18 @@ export class BrowserClient extends Client { ): PromiseLike { event.platform = event.platform || 'javascript'; - return super._prepareEvent(event, hint, currentScope, isolationScope).then(prepared => { - if (!prepared) { - return prepared; - } - - // By default, we want to infer the IP address, unless this is explicitly set to `null` - // We do this after all other processing is done - // If `ip_address` is explicitly set to `null` or a value, we leave it as is - if (prepared.user?.ip_address === undefined) { - prepared.user = { - ...prepared.user, - ip_address: '{{auto}}', - }; - } + return super._prepareEvent(event, hint, currentScope, isolationScope); + } +} - return prepared; - }); +// By default, we want to infer the IP address, unless this is explicitly set to `null` +// We do this after all other processing is done +// If `ip_address` is explicitly set to `null` or a value, we leave it as is +function addAutoIpAddressToUser(objWithMaybeUser: { user?: User | null }): void { + if (objWithMaybeUser.user?.ip_address === undefined) { + objWithMaybeUser.user = { + ...objWithMaybeUser.user, + ip_address: '{{auto}}', + }; } } diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 803520c166b6..0158afe724f1 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -411,6 +411,7 @@ export abstract class Client { } sessionAttrs.release = sessionAttrs.release || clientReleaseOption; sessionAttrs.environment = sessionAttrs.environment || clientEnvironmentOption; + session.attrs = sessionAttrs; } else { if (!session.release && !clientReleaseOption) { DEBUG_BUILD && logger.warn(MISSING_RELEASE_FOR_SESSION_ERROR); @@ -420,6 +421,8 @@ export abstract class Client { session.environment = session.environment || clientEnvironmentOption; } + this.emit('beforeSendSession', session); + const env = createSessionEnvelope(session, this._dsn, this._options._metadata, this._options.tunnel); // sendEnvelope should not throw @@ -503,6 +506,13 @@ export abstract class Client { */ public on(hook: 'beforeSendEvent', callback: (event: Event, hint?: EventHint | undefined) => void): () => void; + /** + * Register a callback for before sending a session or session aggregrates.. + * Receives the session/aggregate as second argument. + * @returns {() => void} A function that, when executed, removes the registered callback. + */ + public on(hook: 'beforeSendSession', callback: (session: Session | SessionAggregates) => void): () => void; + /** * Register a callback for preprocessing an event, * before it is passed to (global) event processors. @@ -511,6 +521,14 @@ export abstract class Client { */ public on(hook: 'preprocessEvent', callback: (event: Event, hint?: EventHint | undefined) => void): () => void; + /** + * Register a callback for postprocessing an event, + * after it was passed to (global) event processors, before it is being sent. + * Receives an Event & EventHint as arguments. + * @returns {() => void} A function that, when executed, removes the registered callback. + */ + public on(hook: 'postprocessEvent', callback: (event: Event, hint?: EventHint | undefined) => void): () => void; + /** * Register a callback for when an event has been sent. * @returns {() => void} A function that, when executed, removes the registered callback. @@ -636,12 +654,24 @@ export abstract class Client { */ public emit(hook: 'beforeSendEvent', event: Event, hint?: EventHint): void; + /** + * Fire a hook event before sending a session/aggregates. + * Expects to be given the prepared session/aggregates as second argument. + */ + public emit(hook: 'beforeSendSession', session: Session | SessionAggregates): void; + /** * Fire a hook event to process events before they are passed to (global) event processors. * Expects to be given an Event & EventHint as the second/third argument. */ public emit(hook: 'preprocessEvent', event: Event, hint?: EventHint): void; + /** + * Fire a hook event to process a user on an event before it is sent to Sentry, after all other processors have run. + * Expects to be given an Event & EventHint as the second/third argument. + */ + public emit(hook: 'postprocessEvent', event: Event, hint?: EventHint): void; + /* * Fire a hook event after sending an event. Expects to be given an Event as the * second argument. @@ -831,6 +861,8 @@ export abstract class Client { return evt; } + this.emit('postprocessEvent', evt, hint); + evt.contexts = { trace: getTraceContextFromScope(currentScope), ...evt.contexts, diff --git a/packages/core/src/types-hoist/session.ts b/packages/core/src/types-hoist/session.ts index 589c71f9094c..1cdfef158af8 100644 --- a/packages/core/src/types-hoist/session.ts +++ b/packages/core/src/types-hoist/session.ts @@ -37,6 +37,7 @@ export interface SessionAggregates { attrs?: { environment?: string; release?: string; + ip_address?: string | null; }; aggregates: Array; } diff --git a/packages/profiling-node/src/integration.ts b/packages/profiling-node/src/integration.ts index f4a860d035f0..ca9db531d1e9 100644 --- a/packages/profiling-node/src/integration.ts +++ b/packages/profiling-node/src/integration.ts @@ -147,6 +147,11 @@ function setupAutomatedSpanProfiling(client: NodeClient): void { client.emit('preprocessEvent', profile, { event_id: profiledTransaction.event_id, }); + + // @ts-expect-error profile does not inherit from Event + client.emit('postprocessEvent', profile, { + event_id: profiledTransaction.event_id, + }); } addProfilesToEnvelope(envelope, profilesToAddToEnvelope); diff --git a/packages/replay-internal/src/util/prepareReplayEvent.ts b/packages/replay-internal/src/util/prepareReplayEvent.ts index ca1546eb9bfa..9f74c2c8a685 100644 --- a/packages/replay-internal/src/util/prepareReplayEvent.ts +++ b/packages/replay-internal/src/util/prepareReplayEvent.ts @@ -40,6 +40,8 @@ export async function prepareReplayEvent({ return null; } + client.emit('postprocessEvent', preparedEvent, eventHint); + // This normally happens in browser client "_prepareEvent" // but since we do not use this private method from the client, but rather the plain import // we need to do this manually. @@ -55,15 +57,5 @@ export async function prepareReplayEvent({ version: version || '0.0.0', }; - // By default, we want to infer the IP address, unless this is explicitly set to `null` - // We do this after all other processing is done - // If `ip_address` is explicitly set to `null` or a value, we leave it as is - if (preparedEvent.user?.ip_address === undefined) { - preparedEvent.user = { - ...preparedEvent.user, - ip_address: '{{auto}}', - }; - } - return preparedEvent; } diff --git a/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts b/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts index 67b78747a29b..0808481b21ed 100644 --- a/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts +++ b/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts @@ -77,21 +77,20 @@ describe('Unit | util | prepareReplayEvent', () => { name: 'sentry.javascript.testSdk', version: '1.0.0', }, - user: { - ip_address: '{{auto}}', - }, sdkProcessingMetadata: expect.any(Object), breadcrumbs: undefined, }); }); - it('sets user', async () => { + it('emits hooks', async () => { const client = getClient()!; const scope = getCurrentScope().clone(); - scope.setUser({ id: 'user-id' }); + const preprocessEvent = vi.fn(); + const postprocessEvent = vi.fn(); - expect(client).toBeDefined(); + const removeHook1 = client.on('preprocessEvent', preprocessEvent); + const removeHook2 = client.on('postprocessEvent', postprocessEvent); const replayId = 'replay-ID'; const event: ReplayEvent = { @@ -111,11 +110,7 @@ describe('Unit | util | prepareReplayEvent', () => { }, }; - const replayEvent = await prepareReplayEvent({ scope, client, replayId, event }); - - expect(client.getSdkMetadata).toHaveBeenCalledTimes(1); - - expect(replayEvent).toEqual({ + const processedEvent = { type: 'replay_event', timestamp: 1670837008.634, error_ids: ['error-ID'], @@ -137,73 +132,25 @@ describe('Unit | util | prepareReplayEvent', () => { name: 'sentry.javascript.testSdk', version: '1.0.0', }, - user: { - id: 'user-id', - ip_address: '{{auto}}', - }, sdkProcessingMetadata: expect.any(Object), breadcrumbs: undefined, - }); - }); - - it('allows to set user.ip_address=null', async () => { - const client = getClient()!; - const scope = getCurrentScope().clone(); - - scope.setUser({ id: 'user-id', ip_address: null }); - - expect(client).toBeDefined(); - - const replayId = 'replay-ID'; - const event: ReplayEvent = { - type: REPLAY_EVENT_NAME, - timestamp: 1670837008.634, - error_ids: ['error-ID'], - trace_ids: ['trace-ID'], - urls: ['https://sentry.io/'], - replay_id: replayId, - replay_type: 'session', - segment_id: 3, - contexts: { - replay: { - error_sample_rate: 1.0, - session_sample_rate: 0.1, - }, - }, }; - const replayEvent = await prepareReplayEvent({ scope, client, replayId, event }); + await prepareReplayEvent({ scope, client, replayId, event }); - expect(client.getSdkMetadata).toHaveBeenCalledTimes(1); + expect(preprocessEvent).toHaveBeenCalledTimes(1); + expect(preprocessEvent).toHaveBeenCalledWith(event, { + event_id: 'replay-ID', + integrations: [], + }); - expect(replayEvent).toEqual({ - type: 'replay_event', - timestamp: 1670837008.634, - error_ids: ['error-ID'], - trace_ids: ['trace-ID'], - urls: ['https://sentry.io/'], - replay_id: 'replay-ID', - replay_type: 'session', - segment_id: 3, - platform: 'javascript', + expect(postprocessEvent).toHaveBeenCalledTimes(1); + expect(postprocessEvent).toHaveBeenCalledWith(processedEvent, { event_id: 'replay-ID', - environment: 'production', - contexts: { - replay: { - error_sample_rate: 1.0, - session_sample_rate: 0.1, - }, - }, - sdk: { - name: 'sentry.javascript.testSdk', - version: '1.0.0', - }, - user: { - id: 'user-id', - ip_address: null, - }, - sdkProcessingMetadata: expect.any(Object), - breadcrumbs: undefined, + integrations: [], }); + + removeHook1(); + removeHook2(); }); }); From a8293ad40e1bbcaae269755cf1080a2864f6c9c2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 17 Jan 2025 12:08:40 +0100 Subject: [PATCH 6/6] fix tests --- .../tracing/metrics/web-vitals-cls-standalone-spans/test.ts | 4 ++++ .../suites/tracing/metrics/web-vitals-inp-late/test.ts | 1 + .../tracing/metrics/web-vitals-inp-parametrized-late/test.ts | 1 + .../tracing/metrics/web-vitals-inp-parametrized/test.ts | 1 + .../suites/tracing/metrics/web-vitals-inp/test.ts | 1 + .../test-applications/react-17/tests/transactions.test.ts | 1 + .../react-router-6/tests/transactions.test.ts | 1 + .../react-router-7-spa/tests/transactions.test.ts | 1 + 8 files changed, 11 insertions(+) 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 index 02431dae3b79..9018252f37cb 100644 --- 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 @@ -69,6 +69,7 @@ sentryTest('captures a "GOOD" CLS vital with its source as a standalone span', a transaction: expect.stringContaining('index.html'), 'user_agent.original': expect.stringContaining('Chrome'), 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), + 'client.address': '{{auto}}', }, description: expect.stringContaining('body > div#content > p'), exclusive_time: 0, @@ -137,6 +138,7 @@ sentryTest('captures a "MEH" CLS vital with its source as a standalone span', as transaction: expect.stringContaining('index.html'), 'user_agent.original': expect.stringContaining('Chrome'), 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), + 'client.address': '{{auto}}', }, description: expect.stringContaining('body > div#content > p'), exclusive_time: 0, @@ -203,6 +205,7 @@ sentryTest('captures a "POOR" CLS vital with its source as a standalone span.', transaction: expect.stringContaining('index.html'), 'user_agent.original': expect.stringContaining('Chrome'), 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), + 'client.address': '{{auto}}', }, description: expect.stringContaining('body > div#content > p'), exclusive_time: 0, @@ -270,6 +273,7 @@ sentryTest( transaction: expect.stringContaining('index.html'), 'user_agent.original': expect.stringContaining('Chrome'), 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), + 'client.address': '{{auto}}', }, description: 'Layout shift', exclusive_time: 0, diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts index fffa85b89ae2..d151772439a1 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts @@ -71,6 +71,7 @@ sentryTest('should capture an INP click event span after pageload', async ({ bro 'sentry.source': 'custom', transaction: 'test-url', 'user_agent.original': expect.stringContaining('Chrome'), + 'client.address': '{{auto}}', }, measurements: { inp: { diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts index 65852c734c98..961f98518049 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts @@ -74,6 +74,7 @@ sentryTest( 'sentry.source': 'custom', transaction: 'test-route', 'user_agent.original': expect.stringContaining('Chrome'), + 'client.address': '{{auto}}', }, measurements: { inp: { diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts index 5705fe6863b5..911929471ea4 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts @@ -70,6 +70,7 @@ sentryTest( 'sentry.origin': 'auto.http.browser.inp', transaction: 'test-route', 'user_agent.original': expect.stringContaining('Chrome'), + 'client.address': '{{auto}}', }, measurements: { inp: { diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts index b3435a49b002..1be97e2446dc 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts @@ -69,6 +69,7 @@ sentryTest('should capture an INP click event span during pageload', async ({ br 'sentry.origin': 'auto.http.browser.inp', transaction: 'test-url', 'user_agent.original': expect.stringContaining('Chrome'), + 'client.address': '{{auto}}', }, measurements: { inp: { diff --git a/dev-packages/e2e-tests/test-applications/react-17/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-17/tests/transactions.test.ts index 58e3df1ee8d6..14d8b8b21d65 100644 --- a/dev-packages/e2e-tests/test-applications/react-17/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-17/tests/transactions.test.ts @@ -83,6 +83,7 @@ test('sends an INP span', async ({ page }) => { 'sentry.exclusive_time': expect.any(Number), replay_id: expect.any(String), 'user_agent.original': expect.stringContaining('Chrome'), + 'client.address': '{{auto}}', }, description: 'body > div#root > input#exception-button[type="button"]', op: 'ui.interaction.click', diff --git a/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts index 555e0655c52e..ae9ff366abd4 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts @@ -83,6 +83,7 @@ test('sends an INP span', async ({ page }) => { 'sentry.exclusive_time': expect.any(Number), replay_id: expect.any(String), 'user_agent.original': expect.stringContaining('Chrome'), + 'client.address': '{{auto}}', }, description: 'body > div#root > input#exception-button[type="button"]', op: 'ui.interaction.click', diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-spa/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-spa/tests/transactions.test.ts index c915d3694742..f0c7c680d07e 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-spa/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-spa/tests/transactions.test.ts @@ -83,6 +83,7 @@ test('sends an INP span', async ({ page }) => { 'sentry.exclusive_time': expect.any(Number), replay_id: expect.any(String), 'user_agent.original': expect.stringContaining('Chrome'), + 'client.address': '{{auto}}', }, description: 'body > div#root > input#exception-button[type="button"]', op: 'ui.interaction.click',