Skip to content

Commit 9f72550

Browse files
chargomeclaude
andcommitted
fix(v10/opentelemetry,sveltekit): Filter query params on exported OTel spans and SvelteKit root spans
Addresses review feedback: the OTel exporter rebuilt `http.query` from the raw span URL, and the SvelteKit handle copied the kit root span URL into `url.full` unfiltered. Co-Authored-By: Claude Fable 5.1 <[email protected]>
1 parent 8ee6dfc commit 9f72550

4 files changed

Lines changed: 43 additions & 5 deletions

File tree

‎packages/opentelemetry/src/utils/getRequestSpanData.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { Span } from '@opentelemetry/api';
22
import type { ReadableSpan } from '@opentelemetry/sdk-trace-base';
33
import { HTTP_METHOD, HTTP_REQUEST_METHOD, HTTP_URL, URL_FULL } from '@sentry/conventions/attributes';
44
import type { SanitizedRequestData } from '@sentry/core';
5-
import { getSanitizedUrlString, parseUrl } from '@sentry/core';
5+
import { filterCollectedUrlQuery, getSanitizedUrlString, parseUrl } from '@sentry/core';
66
import { spanHasAttributes } from './spanTypes';
77

88
/**
@@ -34,8 +34,10 @@ export function getRequestSpanData(span: Span | ReadableSpan): Partial<Sanitized
3434

3535
data.url = getSanitizedUrlString(url);
3636

37-
if (url.search) {
38-
data['http.query'] = url.search;
37+
// The URL may come from a third-party instrumentation that never ran it through the SDK's filtering.
38+
const query = filterCollectedUrlQuery(url.search);
39+
if (query) {
40+
data['http.query'] = query;
3941
}
4042
if (url.hash) {
4143
data['http.fragment'] = url.hash;

‎packages/opentelemetry/test/utils/getRequestSpanData.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,22 @@ describe('getRequestSpanData', () => {
4949
});
5050
});
5151

52+
it('filters sensitive query params according to dataCollection.urlQueryParams', () => {
53+
const span = createSpan('test-span');
54+
span.setAttributes({
55+
[HTTP_URL]: 'http://example.com/reset?token=secret&page=1',
56+
[HTTP_METHOD]: 'GET',
57+
});
58+
59+
const data = getRequestSpanData(span);
60+
61+
expect(data).toEqual({
62+
url: 'http://example.com/reset',
63+
'http.method': 'GET',
64+
'http.query': '?token=[Filtered]&page=1',
65+
});
66+
});
67+
5268
it('works without method', () => {
5369
const span = createSpan('test-span');
5470
span.setAttributes({

‎packages/sveltekit/src/server-common/handle.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,10 @@ async function instrumentHandle(
181181
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit',
182182
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: routeName ? 'route' : 'url',
183183
'sveltekit.tracing.original_name': originalName,
184-
[URL_FULL]:
184+
[URL_FULL]: filterCollectedUrl(
185185
// oxlint-disable-next-line typescript-eslint(no-deprecated)
186-
kitRootSpanAttributes[URL_FULL] ?? kitRootSpanAttributes[HTTP_URL] ?? filterCollectedUrl(event.url.href),
186+
(kitRootSpanAttributes[URL_FULL] ?? kitRootSpanAttributes[HTTP_URL] ?? event.url.href) as string,
187+
),
187188
[URL_PATH]: kitRootSpanAttributes[URL_PATH] ?? event.url.pathname,
188189
...(routeName && {
189190
[HTTP_ROUTE]: routeName,

‎packages/sveltekit/test/server-common/handle.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,25 @@ describe('sentryHandle', () => {
177177
kitRootSpan.end();
178178
});
179179

180+
it('filters sensitive query params in the URL taken over from the sveltekit root span', async () => {
181+
const kitRootSpan = SentryCore.startInactiveSpan({
182+
name: 'sveltekit.handle.root',
183+
attributes: { 'url.full': 'https://example.com/reset?token=secret&page=1' },
184+
});
185+
186+
try {
187+
await sentryHandle()({
188+
event: mockEvent({ tracing: { enabled: true, root: kitRootSpan } }),
189+
resolve: resolve(type, isError),
190+
});
191+
} catch {
192+
//
193+
}
194+
195+
expect(spanToJSON(kitRootSpan).data?.['url.full']).toEqual('https://example.com/reset?token=[Filtered]&page=1');
196+
kitRootSpan.end();
197+
});
198+
180199
it('starts a child span for nested server calls (i.e. if there is an active span)', async () => {
181200
let _span: Span | undefined = undefined;
182201
let txnCount = 0;

0 commit comments

Comments
 (0)