Skip to content

Commit 5f77fe2

Browse files
fix(v10/browser): Apply urlQueryParams to event URLs, static span names and LCP URLs (#25297)
Backport of: #25262 ## Differences to the original PR - **Source files moved.** v10 still uses the older `browser-utils` layout. The `performance/entries.ts` and `web-vitals/tracking.ts` changes went into `src/metrics/browserMetrics.ts`. The `web-vitals/spans.ts` change went into `src/metrics/webVitalSpans.ts`. The filtering is the same. - **Navigation spans:** v10's `_addPerformanceNavigationTiming` / `_addRequest` set no `url.full` attribute and have no span-streaming name branch. So only the span `name` is now filtered (`filterCollectedUrl(entry.name)`). - **`_sendLcpSpan` (`webVitalSpans.ts`):** on v10 this function takes no `client` parameter. `filterCollectedUrl(entry.url)` falls back to `getClient()`. - **Added beyond the original:** `src/metrics/lcp.ts` (`_sendStandaloneLcpSpan`) now filters its `lcp.url` attribute too. This standalone LCP span code exists only on v10 (it was removed on develop). Without this change it would still send the unfiltered LCP URL. This matches the original PR's goal of filtering `lcp.url`. - **`httpcontext.ts`:** v10 spreads `...reqData` into `event.request` instead of setting `url` explicitly. The backport adds `url: filterCollectedUrl(reqData.url, client)` after the spread and before `...event.request`, so a URL the user set still wins. `preprocessEvent` now takes the `client` argument. - **Unit tests adapted to v10 paths and shapes:** - The new `browserMetrics.test.ts` cases are in `test/browser/browserMetrics.test.ts`. They check `description`/`data`, not `name`/`attributes`. - The navigation case checks only span descriptions, because v10 navigation spans have no `url.full`. To share it with the new case, the navigation `entry` fixture moved to `describe` scope. - The `_sendLcpSpan` expectation update is in `test/metrics/webVitalSpans.test.ts`. No client mock is needed. - `test/metrics/lcp.test.ts` gets the same query-param expectation for the standalone LCP span. - The `httpcontext.test.ts` cases were added at the top level, because v10 has no `dataCollection` describe block. - **Integration tests:** - The develop-only `web-vitals-lcp-static` suite maps to v10's `web-vitals-lcp` (static is the default there). - The new `pageload-sensitive-query-params-static` test uses string ops (`'pageload'`, `'browser.request'`, `'resource.img'`). v10's `@sentry/conventions@0.16.0` doesn't export `BROWSER_REQUEST` / `PAGELOAD` / `RESOURCE_IMG` from `@sentry/conventions/op`. - The `httpContext` integration test imports `waitForErrorRequest`/`envelopeRequestParser`, which v10's version didn't import yet. ## Verification - `yarn format`, `yarn lint:fix` (0 warnings, 0 errors) and `yarn build:dev` all pass. - Vitest passes for: `browser-utils` (`browserMetrics`, `lcp`, `webVitalSpans`), `browser` (`browserTracingIntegration`) and `nextjs` (`pagesRouterInstrumentation`). - `browser` `httpcontext.test.ts` passed (5/5) only with a temporary local workaround. Under the sandbox's Node 22, the existing `globalThis.navigator = …` setup throws, and it does the same on unmodified v10. The repo pins Node 20. - Playwright browser-integration tests were not run locally. They are left to CI. <!-- junior-request-attribution:start --> via Junior system actor `junior`. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent-dispatch%3Adispatch_968746a4c2386e3695487fe7c0061e2b --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/agent-dispatch%3Adispatch_968746a4c2386e3695487fe7c0061e2b) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent-dispatch%3Adispatch_968746a4c2386e3695487fe7c0061e2b/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
1 parent c96d509 commit 5f77fe2

17 files changed

Lines changed: 191 additions & 58 deletions

File tree

‎dev-packages/browser-integration-tests/suites/integrations/httpContext/test.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { expect } from '@playwright/test';
22
import type { Event } from '@sentry/core';
33
import { sentryTest } from '../../../utils/fixtures';
4-
import { getFirstSentryEnvelopeRequest } from '../../../utils/helpers';
4+
import { envelopeRequestParser, getFirstSentryEnvelopeRequest, waitForErrorRequest } from '../../../utils/helpers';
55

66
sentryTest('httpContextIntegration captures user-agent and referrer', async ({ getLocalTestUrl, page }) => {
77
const url = await getLocalTestUrl({ testDir: __dirname });
@@ -23,3 +23,15 @@ sentryTest('httpContextIntegration captures user-agent and referrer', async ({ g
2323
url: expect.any(String),
2424
});
2525
});
26+
27+
sentryTest('httpContextIntegration filters sensitive query params in the URL', async ({ getLocalTestUrl, page }) => {
28+
const url = await getLocalTestUrl({ testDir: __dirname });
29+
30+
const errorEventPromise = waitForErrorRequest(page);
31+
32+
await page.goto(`${url}?jwt=sensitive-jwt-value&page=1`);
33+
34+
const errorEvent = envelopeRequestParser(await errorEventPromise);
35+
36+
expect(errorEvent.request?.url).toBe(`${url}?jwt=[Filtered]&page=1`);
37+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
traceLifecycle: 'static',
7+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
8+
integrations: [Sentry.browserTracingIntegration()],
9+
tracesSampleRate: 1,
10+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<!doctype html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<img src="https://sentry-test-site.example/path/to/image.svg?token=sensitive-token-value&size=2" />
8+
</body>
9+
</html>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { expect } from '@playwright/test';
2+
import type { Event } from '@sentry/core';
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
5+
6+
sentryTest(
7+
'filters sensitive query params in the pageload transaction URL and span descriptions',
8+
async ({ getLocalTestUrl, page }) => {
9+
sentryTest.skip(shouldSkipTracingTest());
10+
11+
await page.route('https://sentry-test-site.example/path/to/image.svg?*', route =>
12+
route.fulfill({ body: '<svg xmlns="http://www.w3.org/2000/svg"/>', contentType: 'image/svg+xml' }),
13+
);
14+
15+
const url = await getLocalTestUrl({ testDir: __dirname });
16+
17+
const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload');
18+
await page.goto(`${url}?jwt=sensitive-jwt-value&page=1`);
19+
const eventData = envelopeRequestParser<Event>(await pageloadRequestPromise);
20+
21+
expect(eventData.request?.url).toBe(`${url}?jwt=[Filtered]&page=1`);
22+
expect(eventData.spans?.find(({ op }) => op === 'browser.request')?.description).toBe(
23+
`${url}?jwt=[Filtered]&page=1`,
24+
);
25+
expect(eventData.spans?.find(({ op }) => op === 'resource.img')?.description).toBe(
26+
'https://sentry-test-site.example/path/to/image.svg?token=[Filtered]&size=2',
27+
);
28+
29+
const serializedEvent = JSON.stringify(eventData);
30+
expect(serializedEvent).not.toContain('sensitive-jwt-value');
31+
expect(serializedEvent).not.toContain('sensitive-token-value');
32+
},
33+
);

‎dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/template.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
</head>
66
<body>
77
<div id="content"></div>
8-
<img src="https://sentry-test-site.example/my/image.png" />
8+
<img src="https://sentry-test-site.example/my/image.png?token=sensitive-token-value&v=1" />
99
</body>
1010
</html>

‎dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ sentryTest('captures LCP vitals with element details', async ({ browserName, get
1010
}
1111

1212
page.route('**', route => route.continue());
13-
page.route('**/my/image.png', async (route: Route) => {
13+
page.route('**/my/image.png?*', async (route: Route) => {
1414
return route.fulfill({
1515
path: `${__dirname}/assets/sentry-logo-600x179.png`,
1616
});
@@ -26,6 +26,9 @@ sentryTest('captures LCP vitals with element details', async ({ browserName, get
2626
expect(eventData.contexts?.trace?.data?.['lcp.size']).toBeGreaterThan(0);
2727
expect(eventData.contexts?.trace?.data?.['lcp.loadTime']).toBeGreaterThan(0);
2828
expect(eventData.contexts?.trace?.data?.['lcp.renderTime']).toBeGreaterThan(0);
29+
expect(eventData.contexts?.trace?.data?.['lcp.url']).toBe(
30+
'https://sentry-test-site.example/my/image.png?token=[Filtered]&v=1',
31+
);
2932

3033
// The LCP value should be the renderTime because the renderTime is set
3134
expect(eventData.measurements?.lcp?.value).toBeCloseTo(eventData.contexts?.trace?.data?.['lcp.renderTime']);

‎dev-packages/browser-integration-tests/utils/fixtures.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ const sentryTest = base.extend<TestFixtures>({
9090
}
9191

9292
await page.route(`${TEST_HOST}/*.*`, route => {
93-
const file = route.request().url().split('/').pop();
93+
const file = new URL(route.request().url()).pathname.split('/').pop();
9494
const filePath = path.resolve(tmpDir, `./${file}`);
9595

9696
return fs.existsSync(filePath)

‎packages/browser-utils/src/metrics/browserMetrics.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ function _addPerformanceNavigationTiming(
690690
}
691691
startAndEndSpan(span, timeOrigin + msToSec(start), timeOrigin + msToSec(end), {
692692
op: `browser.${name}`,
693-
name: entry.name,
693+
name: filterCollectedUrl(entry.name),
694694
attributes: {
695695
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
696696
...(event === 'redirect' && entry.redirectCount != null ? { 'http.redirect_count': entry.redirectCount } : {}),
@@ -718,17 +718,19 @@ function _addRequest(span: Span, entry: PerformanceNavigationTiming, timeOrigin:
718718
// In this case, ie. when the document request hasn't finished yet, `entry.responseEnd` will be 0.
719719
// In order not to produce faulty spans, where the end timestamp is before the start timestamp, we will only collect
720720
// these spans when the responseEnd value is available. The backend (Relay) would drop the entire span if it contained faulty spans.
721+
const filteredUrl = filterCollectedUrl(entry.name);
722+
721723
startAndEndSpan(span, requestStartTimestamp, responseEndTimestamp, {
722724
op: 'browser.request',
723-
name: entry.name,
725+
name: filteredUrl,
724726
attributes: {
725727
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
726728
},
727729
});
728730

729731
startAndEndSpan(span, responseStartTimestamp, responseEndTimestamp, {
730732
op: 'browser.response',
731-
name: entry.name,
733+
name: filteredUrl,
732734
attributes: {
733735
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
734736
},
@@ -776,7 +778,8 @@ export function _addResourceSpans(
776778

777779
attributes['url.same_origin'] = resourceUrl.includes(WINDOW.location.origin);
778780

779-
attributes[URL_FULL] = filterCollectedUrl(resourceUrl);
781+
const filteredUrl = filterCollectedUrl(resourceUrl);
782+
attributes[URL_FULL] = filteredUrl;
780783

781784
_setResourceRequestAttributes(entry, attributes, [
782785
// https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStatus
@@ -799,7 +802,7 @@ export function _addResourceSpans(
799802
const endTimestamp = startTimestamp + duration;
800803

801804
startAndEndSpan(span, startTimestamp, endTimestamp, {
802-
name: resourceUrl.replace(WINDOW.location.origin, ''),
805+
name: filteredUrl.replace(WINDOW.location.origin, ''),
803806
op,
804807
attributes: attributesWithResourceTiming,
805808
});
@@ -873,7 +876,7 @@ function _setWebVitalAttributes(span: Span, options: AddWebVitalsToSpanOptions):
873876

874877
if (_lcpEntry.url) {
875878
// Trim URL to the first 200 characters.
876-
span.setAttribute('lcp.url', _lcpEntry.url.trim().slice(0, 200));
879+
span.setAttribute('lcp.url', filterCollectedUrl(_lcpEntry.url.trim()).slice(0, 200));
877880
}
878881

879882
if (_lcpEntry.loadTime != null) {

‎packages/browser-utils/src/metrics/lcp.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { Client, SpanAttributes } from '@sentry/core';
22
import {
33
browserPerformanceTimeOrigin,
44
debug,
5+
filterCollectedUrl,
56
getCurrentScope,
67
SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME,
78
SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT,
@@ -90,7 +91,7 @@ export function _sendStandaloneLcpSpan(
9091
entry.element && (attributes['lcp.element'] = htmlTreeAsString(entry.element));
9192
entry.id && (attributes['lcp.id'] = entry.id);
9293

93-
entry.url && (attributes['lcp.url'] = entry.url);
94+
entry.url && (attributes['lcp.url'] = filterCollectedUrl(entry.url));
9495

9596
// loadTime is the time of LCP that's related to receiving the LCP element response..
9697
entry.loadTime != null && (attributes['lcp.loadTime'] = entry.loadTime);

‎packages/browser-utils/src/metrics/webVitalSpans.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { Client, Span, SpanAttributes } from '@sentry/core';
22
import {
33
browserPerformanceTimeOrigin,
44
debug,
5+
filterCollectedUrl,
56
getActiveSpan,
67
getCurrentScope,
78
getRootSpan,
@@ -162,7 +163,7 @@ export function _sendLcpSpan(
162163

163164
entry?.element && (attributes['browser.web_vital.lcp.element'] = htmlTreeAsString(entry.element));
164165
entry?.id && (attributes['browser.web_vital.lcp.id'] = entry.id);
165-
entry?.url && (attributes['browser.web_vital.lcp.url'] = entry.url);
166+
entry?.url && (attributes['browser.web_vital.lcp.url'] = filterCollectedUrl(entry.url));
166167
entry?.loadTime != null && (attributes['browser.web_vital.lcp.load_time'] = entry.loadTime);
167168
entry?.renderTime != null && (attributes['browser.web_vital.lcp.render_time'] = entry.renderTime);
168169
entry?.size != null && (attributes['browser.web_vital.lcp.size'] = entry.size);

0 commit comments

Comments
 (0)