fix(core): Ensure http.client span descriptions don't contain query…

… params or fragments (#15404) This patch fixes an oversight with our `fetch` instrumentation in the core package and the browser XHR instrumentation. We didn't strip query params and URL hash fragments from the span name (description) of `http.client` spans. With this fix, the span description now only contains the URL protocol, host and path as defined in our [develop specification](https://develop.sentry.dev/sdk/expected-features/data-handling/#spans).
getsentry · Feb 17, 2025 · 6e6f85b · 6e6f85b
1 parent 5e6b852
commit 6e6f85b
Show file tree

Hide file tree

Showing 21 changed files with 821 additions and 23 deletions.
diff --git a/...s/browser-integration-tests/suites/tracing/request/fetch-strip-query-and-fragment/init.js b/...s/browser-integration-tests/suites/tracing/request/fetch-strip-query-and-fragment/init.js
@@ -0,0 +1,11 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  integrations: [Sentry.browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false })],
+  tracePropagationTargets: ['http://sentry-test-site.example'],
+  tracesSampleRate: 1,
+  autoSessionTracking: false,
+});
diff --git a/...rowser-integration-tests/suites/tracing/request/fetch-strip-query-and-fragment/subject.js b/...rowser-integration-tests/suites/tracing/request/fetch-strip-query-and-fragment/subject.js
@@ -0,0 +1,19 @@
+function withRootSpan(cb) {
+  return Sentry.startSpan({ name: 'rootSpan' }, cb);
+}
+
+document.getElementById('btnQuery').addEventListener('click', async () => {
+  await withRootSpan(() => fetch('http://sentry-test-site.example/0?id=123;page=5'));
+});
+
+document.getElementById('btnFragment').addEventListener('click', async () => {
+  await withRootSpan(() => fetch('http://sentry-test-site.example/1#fragment'));
+});
+
+document.getElementById('btnQueryFragment').addEventListener('click', async () => {
+  await withRootSpan(() => fetch('http://sentry-test-site.example/2?id=1#fragment'));
+});
+
+document.getElementById('btnQueryFragmentSameOrigin').addEventListener('click', async () => {
+  await withRootSpan(() => fetch('/api/users?id=1#fragment'));
+});
diff --git a/...ser-integration-tests/suites/tracing/request/fetch-strip-query-and-fragment/template.html b/...ser-integration-tests/suites/tracing/request/fetch-strip-query-and-fragment/template.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <button id="btnQuery">Request with query</button>
+    <button id="btnFragment">Request with fragment</button>
+    <button id="btnQueryFragment">Request with query and fragment</button>
+    <button id="btnQueryFragmentSameOrigin">Request with query and fragment</button>
+  </body>
+</html>
diff --git a/...s/browser-integration-tests/suites/tracing/request/fetch-strip-query-and-fragment/test.ts b/...s/browser-integration-tests/suites/tracing/request/fetch-strip-query-and-fragment/test.ts
@@ -0,0 +1,176 @@
+import { expect } from '@playwright/test';
+import { sentryTest } from '../../../../utils/fixtures';
+import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
+
+sentryTest('strips query params in fetch request spans', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipTracingTest()) {
+    sentryTest.skip();
+  }
+
+  await page.route('http://sentry-test-site.example/*', route => route.fulfill({ body: 'ok' }));
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.goto(url);
+
+  const txnPromise = waitForTransactionRequest(page);
+  await page.locator('#btnQuery').click();
+  const transactionEvent = envelopeRequestParser(await txnPromise);
+
+  expect(transactionEvent.transaction).toEqual('rootSpan');
+
+  const requestSpan = transactionEvent.spans?.find(({ op }) => op === 'http.client');
+
+  expect(requestSpan).toMatchObject({
+    description: 'GET http://sentry-test-site.example/0',
+    parent_span_id: transactionEvent.contexts?.trace?.span_id,
+    span_id: expect.stringMatching(/[a-f0-9]{16}/),
+    start_timestamp: expect.any(Number),
+    timestamp: expect.any(Number),
+    trace_id: transactionEvent.contexts?.trace?.trace_id,
+    data: expect.objectContaining({
+      'http.method': 'GET',
+      'http.url': 'http://sentry-test-site.example/0?id=123;page=5',
+      'http.query': '?id=123;page=5',
+      'http.response.status_code': 200,
+      'http.response_content_length': 2,
+      'sentry.op': 'http.client',
+      'sentry.origin': 'auto.http.browser',
+      type: 'fetch',
+      'server.address': 'sentry-test-site.example',
+      url: 'http://sentry-test-site.example/0?id=123;page=5',
+    }),
+  });
+
+  expect(requestSpan?.data).not.toHaveProperty('http.fragment');
+});
+
+sentryTest('strips hash fragment in fetch request spans', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipTracingTest()) {
+    sentryTest.skip();
+  }
+
+  await page.route('http://sentry-test-site.example/*', route => route.fulfill({ body: 'ok' }));
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.goto(url);
+
+  const txnPromise = waitForTransactionRequest(page);
+  await page.locator('#btnFragment').click();
+  const transactionEvent = envelopeRequestParser(await txnPromise);
+
+  expect(transactionEvent.transaction).toEqual('rootSpan');
+
+  const requestSpan = transactionEvent.spans?.find(({ op }) => op === 'http.client');
+
+  expect(requestSpan).toMatchObject({
+    description: 'GET http://sentry-test-site.example/1',
+    parent_span_id: transactionEvent.contexts?.trace?.span_id,
+    span_id: expect.stringMatching(/[a-f0-9]{16}/),
+    start_timestamp: expect.any(Number),
+    timestamp: expect.any(Number),
+    trace_id: transactionEvent.contexts?.trace?.trace_id,
+    data: expect.objectContaining({
+      'http.method': 'GET',
+      'http.url': 'http://sentry-test-site.example/1#fragment',
+      'http.fragment': '#fragment',
+      'http.response.status_code': 200,
+      'http.response_content_length': 2,
+      'sentry.op': 'http.client',
+      'sentry.origin': 'auto.http.browser',
+      type: 'fetch',
+      'server.address': 'sentry-test-site.example',
+      url: 'http://sentry-test-site.example/1#fragment',
+    }),
+  });
+
+  expect(requestSpan?.data).not.toHaveProperty('http.query');
+});
+
+sentryTest('strips hash fragment and query params in fetch request spans', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipTracingTest()) {
+    sentryTest.skip();
+  }
+
+  await page.route('http://sentry-test-site.example/*', route => route.fulfill({ body: 'ok' }));
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.goto(url);
+
+  const txnPromise = waitForTransactionRequest(page);
+  await page.locator('#btnQueryFragment').click();
+  const transactionEvent = envelopeRequestParser(await txnPromise);
+
+  expect(transactionEvent.transaction).toEqual('rootSpan');
+
+  const requestSpan = transactionEvent.spans?.find(({ op }) => op === 'http.client');
+
+  expect(requestSpan).toMatchObject({
+    description: 'GET http://sentry-test-site.example/2',
+    parent_span_id: transactionEvent.contexts?.trace?.span_id,
+    span_id: expect.stringMatching(/[a-f0-9]{16}/),
+    start_timestamp: expect.any(Number),
+    timestamp: expect.any(Number),
+    trace_id: transactionEvent.contexts?.trace?.trace_id,
+    data: expect.objectContaining({
+      'http.method': 'GET',
+      'http.url': 'http://sentry-test-site.example/2?id=1#fragment',
+      'http.query': '?id=1',
+      'http.fragment': '#fragment',
+      'http.response.status_code': 200,
+      'http.response_content_length': 2,
+      'sentry.op': 'http.client',
+      'sentry.origin': 'auto.http.browser',
+      type: 'fetch',
+      'server.address': 'sentry-test-site.example',
+      url: 'http://sentry-test-site.example/2?id=1#fragment',
+    }),
+  });
+});
+
+sentryTest(
+  'strips hash fragment and query params in same-origin fetch request spans',
+  async ({ getLocalTestUrl, page }) => {
+    if (shouldSkipTracingTest()) {
+      sentryTest.skip();
+    }
+
+    await page.route('**/*', route => route.fulfill({ body: 'ok' }));
+
+    const url = await getLocalTestUrl({ testDir: __dirname });
+
+    await page.goto(url);
+
+    const txnPromise = waitForTransactionRequest(page);
+    await page.locator('#btnQueryFragmentSameOrigin').click();
+    const transactionEvent = envelopeRequestParser(await txnPromise);
+
+    expect(transactionEvent.transaction).toEqual('rootSpan');
+
+    const requestSpan = transactionEvent.spans?.find(({ op }) => op === 'http.client');
+
+    expect(requestSpan).toMatchObject({
+      description: 'GET /api/users',
+      parent_span_id: transactionEvent.contexts?.trace?.span_id,
+      span_id: expect.stringMatching(/[a-f0-9]{16}/),
+      start_timestamp: expect.any(Number),
+      timestamp: expect.any(Number),
+      trace_id: transactionEvent.contexts?.trace?.trace_id,
+      data: expect.objectContaining({
+        'http.method': 'GET',
+        'http.url': 'http://sentry-test.io/api/users?id=1#fragment',
+        'http.query': '?id=1',
+        'http.fragment': '#fragment',
+        'http.response.status_code': 200,
+        'http.response_content_length': 2,
+        'sentry.op': 'http.client',
+        'sentry.origin': 'auto.http.browser',
+        type: 'fetch',
+        'server.address': 'sentry-test.io',
+        url: '/api/users?id=1#fragment',
+      }),
+    });
+  },
+);
diff --git a/...ges/browser-integration-tests/suites/tracing/request/xhr-strip-query-and-fragment/init.js b/...ges/browser-integration-tests/suites/tracing/request/xhr-strip-query-and-fragment/init.js
@@ -0,0 +1,11 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  integrations: [Sentry.browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false })],
+  tracePropagationTargets: ['http://sentry-test-site.example'],
+  tracesSampleRate: 1,
+  autoSessionTracking: false,
+});
diff --git a/.../browser-integration-tests/suites/tracing/request/xhr-strip-query-and-fragment/subject.js b/.../browser-integration-tests/suites/tracing/request/xhr-strip-query-and-fragment/subject.js
@@ -0,0 +1,29 @@
+function withRootSpan(cb) {
+  return Sentry.startSpan({ name: 'rootSpan' }, cb);
+}
+
+function makeXHRRequest(url) {
+  return new Promise((resolve, reject) => {
+    const xhr = new XMLHttpRequest();
+    xhr.open('GET', url);
+    xhr.onload = () => resolve(xhr.responseText);
+    xhr.onerror = () => reject(xhr.statusText);
+    xhr.send();
+  });
+}
+
+document.getElementById('btnQuery').addEventListener('click', async () => {
+  await withRootSpan(() => makeXHRRequest('http://sentry-test-site.example/0?id=123;page=5'));
+});
+
+document.getElementById('btnFragment').addEventListener('click', async () => {
+  await withRootSpan(() => makeXHRRequest('http://sentry-test-site.example/1#fragment'));
+});
+
+document.getElementById('btnQueryFragment').addEventListener('click', async () => {
+  await withRootSpan(() => makeXHRRequest('http://sentry-test-site.example/2?id=1#fragment'));
+});
+
+document.getElementById('btnQueryFragmentSameOrigin').addEventListener('click', async () => {
+  await withRootSpan(() => makeXHRRequest('/api/users?id=1#fragment'));
+});
diff --git a/...owser-integration-tests/suites/tracing/request/xhr-strip-query-and-fragment/template.html b/...owser-integration-tests/suites/tracing/request/xhr-strip-query-and-fragment/template.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <button id="btnQuery">Request with query</button>
+    <button id="btnFragment">Request with fragment</button>
+    <button id="btnQueryFragment">Request with query and fragment</button>
+    <button id="btnQueryFragmentSameOrigin">Same origin request with query and fragment</button>
+  </body>
+</html>