diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpContext-streamed/subject.js b/dev-packages/browser-integration-tests/suites/integrations/httpContext-streamed/subject.js new file mode 100644 index 000000000000..f1547634fb45 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/httpContext-streamed/subject.js @@ -0,0 +1,5 @@ +Sentry.startSpan({ name: 'parent-span', op: 'test' }, () => { + Sentry.startSpan({ name: 'child-span', op: 'test-child' }, () => { + // noop + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpContext-streamed/test.ts b/dev-packages/browser-integration-tests/suites/integrations/httpContext-streamed/test.ts index 9b5563dec995..332ceda450e8 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/httpContext-streamed/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/httpContext-streamed/test.ts @@ -1,6 +1,7 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../utils/fixtures'; import { shouldSkipTracingTest } from '../../../utils/helpers'; +import { URL_FULL, USER_AGENT_ORIGINAL } from '@sentry/conventions/attributes'; import { getSpanOp, waitForStreamedSpans } from '../../../utils/spanUtils'; sentryTest('httpContextIntegration captures url, user-agent, and referer', async ({ getLocalTestUrl, page }) => { @@ -15,8 +16,8 @@ sentryTest('httpContextIntegration captures url, user-agent, and referer', async const pageloadSpan = spans.find(s => getSpanOp(s) === 'pageload'); - expect(pageloadSpan!.attributes['url.full']).toEqual({ type: 'string', value: expect.any(String) }); - expect(pageloadSpan!.attributes['http.request.header.user_agent']).toEqual({ + expect(pageloadSpan!.attributes[URL_FULL]).toEqual({ type: 'string', value: expect.any(String) }); + expect(pageloadSpan!.attributes[USER_AGENT_ORIGINAL]).toEqual({ type: 'string', value: expect.any(String), }); @@ -25,3 +26,25 @@ sentryTest('httpContextIntegration captures url, user-agent, and referer', async value: 'https://sentry.io/', }); }); + +sentryTest( + 'httpContextIntegration only attaches the user agent to non-segment spans', + async ({ getLocalTestUrl, page }) => { + sentryTest.skip(shouldSkipTracingTest()); + const url = await getLocalTestUrl({ testDir: __dirname }); + + const spansPromise = waitForStreamedSpans(page, spans => spans.some(s => s.name === 'child-span')); + + await page.goto(url, { referer: 'https://sentry.io/' }); + + const spans = await spansPromise; + + const childSpan = spans.find(s => s.name === 'child-span'); + + expect(childSpan!.is_segment).toBe(false); + expect(childSpan!.attributes[USER_AGENT_ORIGINAL]).toEqual({ type: 'string', value: expect.any(String) }); + // The document URL and referer only belong on the segment span. + expect(childSpan!.attributes[URL_FULL]).toBeUndefined(); + expect(childSpan!.attributes['http.request.header.referer']).toBeUndefined(); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/streamed/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/streamed/test.ts index c6c85d89bd02..e2a09a658f18 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/streamed/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/streamed/test.ts @@ -18,6 +18,7 @@ import { SENTRY_SDK_NAME, SENTRY_SDK_VERSION, SENTRY_TRACE_LIFECYCLE, + USER_AGENT_ORIGINAL, } from '@sentry/conventions/attributes'; sentryTest( @@ -107,6 +108,10 @@ sentryTest( type: 'string', value: 'stream', }, + [USER_AGENT_ORIGINAL]: { + type: 'string', + value: expect.any(String), + }, }, end_timestamp: expect.any(Number), is_segment: false, @@ -147,6 +152,10 @@ sentryTest( type: 'string', value: 'stream', }, + [USER_AGENT_ORIGINAL]: { + type: 'string', + value: expect.any(String), + }, }, end_timestamp: expect.any(Number), is_segment: false, @@ -191,6 +200,10 @@ sentryTest( type: 'string', value: 'stream', }, + [USER_AGENT_ORIGINAL]: { + type: 'string', + value: expect.any(String), + }, }, end_timestamp: expect.any(Number), is_segment: false, @@ -215,7 +228,7 @@ sentryTest( type: 'string', value: expect.any(String), }, - 'http.request.header.user_agent': { + [USER_AGENT_ORIGINAL]: { type: 'string', value: expect.any(String), }, diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-streamed/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-streamed/test.ts index c0ab731ac838..912a72ea6819 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-streamed/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-streamed/test.ts @@ -7,7 +7,13 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SDK_INTEGRATIONS, } from '@sentry/core'; -import { SENTRY_SEGMENT_NAME_SOURCE, SENTRY_TRACE_LIFECYCLE, URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; +import { + SENTRY_SEGMENT_NAME_SOURCE, + SENTRY_TRACE_LIFECYCLE, + URL_FULL, + URL_PATH, + USER_AGENT_ORIGINAL, +} from '@sentry/conventions/attributes'; import { sentryTest } from '../../../../utils/fixtures'; import { shouldSkipTracingTest } from '../../../../utils/helpers'; import { @@ -87,7 +93,7 @@ sentryTest('starts a streamed navigation span on page navigation', async ({ brow type: 'string', value: expect.any(String), }, - 'http.request.header.user_agent': { + [USER_AGENT_ORIGINAL]: { type: 'string', value: expect.any(String), }, diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-streamed/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-streamed/test.ts index 999bd1697d82..a16b6839571f 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-streamed/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-streamed/test.ts @@ -16,6 +16,7 @@ import { SENTRY_TRACE_LIFECYCLE, URL_FULL, URL_PATH, + USER_AGENT_ORIGINAL, } from '@sentry/conventions/attributes'; import { sentryTest } from '../../../../utils/fixtures'; import { shouldSkipTracingTest } from '../../../../utils/helpers'; @@ -81,7 +82,7 @@ sentryTest( type: 'string', value: expect.any(String), }, - 'http.request.header.user_agent': { + [USER_AGENT_ORIGINAL]: { type: 'string', value: expect.any(String), }, diff --git a/dev-packages/browser-integration-tests/suites/tracing/interactions/spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/interactions/spans/test.ts index 183210356666..1e8f653607b9 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/interactions/spans/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/interactions/spans/test.ts @@ -14,6 +14,7 @@ import { SENTRY_SDK_NAME, SENTRY_SDK_VERSION, SENTRY_TRACE_LIFECYCLE, + USER_AGENT_ORIGINAL, } from '@sentry/conventions/attributes'; import { sentryTest } from '../../../../utils/fixtures'; import { shouldSkipTracingTest } from '../../../../utils/helpers'; @@ -61,7 +62,7 @@ sentryTest('captures streamed interaction span tree. @firefox', async ({ browser type: 'string', value: expect.any(String), }, - 'http.request.header.user_agent': { + [USER_AGENT_ORIGINAL]: { type: 'string', value: expect.any(String), }, @@ -134,6 +135,10 @@ sentryTest('captures streamed interaction span tree. @firefox', async ({ browser type: 'string', value: 'ui.interaction.click', }, + [USER_AGENT_ORIGINAL]: { + type: 'string', + value: expect.any(String), + }, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: { type: 'string', value: 'auto.browser.interactions', diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts index a1a0144f706b..a8645550bbc2 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts @@ -40,7 +40,7 @@ test('Sends a pageload span', async ({ page }) => { 'url.path': { value: '/', type: 'string' }, 'url.template': { value: '/', type: 'string' }, 'react.version': { value: expect.any(String), type: 'string' }, - 'http.request.header.user_agent': { value: expect.any(String), type: 'string' }, + 'user_agent.original': { value: expect.any(String), type: 'string' }, }); expect(String(span.attributes['url.full']?.value)).toMatch(/^https?:\/\/localhost:\d+\/$/); }); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-pages-dir/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-pages-dir/tests/transactions.test.ts index 7ed43357d430..2212cba2525f 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-pages-dir/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-pages-dir/tests/transactions.test.ts @@ -34,7 +34,7 @@ test('Sends a pageload span', async ({ page }) => { 'url.path': { value: '/', type: 'string' }, 'url.template': { value: '/', type: 'string' }, 'react.version': { value: expect.any(String), type: 'string' }, - 'http.request.header.user_agent': { value: expect.any(String), type: 'string' }, + 'user_agent.original': { value: expect.any(String), type: 'string' }, }); expect(String(span.attributes['url.full']?.value)).toMatch(/^https?:\/\/localhost:\d+\/$/); }); @@ -65,7 +65,7 @@ test('Sends a navigation span', async ({ page }) => { 'url.path': { value: '/user/5', type: 'string' }, 'url.template': { value: '/user/[id]', type: 'string' }, 'react.version': { value: expect.any(String), type: 'string' }, - 'http.request.header.user_agent': { value: expect.any(String), type: 'string' }, + 'user_agent.original': { value: expect.any(String), type: 'string' }, }); expect(String(span.attributes['url.full']?.value)).toMatch(/^https?:\/\/localhost:\d+\/user\/5$/); diff --git a/dev-packages/e2e-tests/test-applications/nitro-3/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nitro-3/tests/errors.test.ts index 1c89b12e1e54..1441ecd5ecab 100644 --- a/dev-packages/e2e-tests/test-applications/nitro-3/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nitro-3/tests/errors.test.ts @@ -2,8 +2,16 @@ import { expect, test } from '@playwright/test'; import { waitForError } from '@sentry-internal/test-utils'; test('Sends an error event to Sentry', async ({ request }) => { + // The thrown error is reported twice: once via the h3 tracing channel and once via Nitro's `error` + // hook (which sees it wrapped in an `HTTPError`). Match on the mechanism so we deterministically + // await the event under test instead of whichever arrives first. const errorEventPromise = waitForError('nitro-3', event => { - return !event.type && !!event.exception?.values?.some(v => v.value === 'This is a test error'); + return ( + !event.type && + !!event.exception?.values?.some( + v => v.value === 'This is a test error' && v.mechanism?.type === 'auto.http.nitro.onTraceError', + ) + ); }); await request.get('/api/test-error').catch(() => { diff --git a/packages/browser/src/integrations/httpcontext.ts b/packages/browser/src/integrations/httpcontext.ts index c45f9235793e..8f4c2f957bf8 100644 --- a/packages/browser/src/integrations/httpcontext.ts +++ b/packages/browser/src/integrations/httpcontext.ts @@ -1,12 +1,7 @@ -import { - _INTERNAL_filterKeyValueData, - defineIntegration, - safeSetSpanJSONAttributes, - SEMANTIC_ATTRIBUTE_SENTRY_OP, -} from '@sentry/core'; +import { _INTERNAL_filterKeyValueData, defineIntegration, safeSetSpanJSONAttributes } from '@sentry/core'; import { getHttpRequestData, WINDOW } from '../helpers'; import { filterCollectedUrl } from '@sentry/core'; -import { URL_FULL } from '@sentry/conventions/attributes'; +import { HTTP_REQUEST_HEADER_KEY_BASE, SENTRY_OP, URL_FULL, USER_AGENT_ORIGINAL } from '@sentry/conventions/attributes'; /** * Collects information about HTTP request headers and @@ -40,9 +35,8 @@ export const httpContextIntegration = defineIntegration(() => { ...(Object.keys(headers).length > 0 ? { headers } : { headers: undefined }), }; }, - processSegmentSpan(span, client) { - const spanOp = span.attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_OP]; + processSpan(span, client) { // if none of the information we want exists, don't bother if (!WINDOW.navigator && !WINDOW.location && !WINDOW.document) { return; @@ -58,11 +52,17 @@ export const httpContextIntegration = defineIntegration(() => { ); safeSetSpanJSONAttributes(span, { - // Coerce empty string to undefined so the helper's nullish check drops it, - // rather than writing an empty `url.full` attribute onto the span. - [URL_FULL]: spanOp !== 'http.client' ? filterCollectedUrl(reqData.url) : undefined, - 'http.request.header.user_agent': headers['User-Agent'], - 'http.request.header.referer': headers['Referer'], + // This attribute is used by the "Filter out events from legacy browsers and crawlers" features on the Sentry backend. + // Therefore, it's set on every span. + [USER_AGENT_ORIGINAL]: headers['User-Agent'], + + // These attributes, we only need on the segment span (analogous to the `request` context for events) + ...(span.is_segment && { + // Coerce empty string to undefined so the helper's nullish check drops it, + // rather than writing an empty `url.full` attribute onto the span. + [URL_FULL]: span.attributes?.[SENTRY_OP] !== 'http.client' ? filterCollectedUrl(reqData.url) : undefined, + [`${HTTP_REQUEST_HEADER_KEY_BASE}.referer`]: headers['Referer'], + }), }); }, }; diff --git a/packages/browser/test/integrations/httpcontext.test.ts b/packages/browser/test/integrations/httpcontext.test.ts index b5af940f5839..a571becfef0a 100644 --- a/packages/browser/test/integrations/httpcontext.test.ts +++ b/packages/browser/test/integrations/httpcontext.test.ts @@ -27,6 +27,7 @@ describe('httpContextIntegration', () => { const integration = httpContextIntegration(); const span: Partial = { + is_segment: true, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client', }, @@ -34,14 +35,13 @@ describe('httpContextIntegration', () => { const browserClient = new BrowserClient(getDefaultBrowserClientOptions()); - integration.processSegmentSpan!(span as StreamedSpanJSON, browserClient); + integration.processSpan!(span as StreamedSpanJSON, browserClient); expect(span.attributes).not.toHaveProperty('url.full'); expect(span.attributes).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client', 'http.request.header.referer': 'https://example.com', - 'http.request.header.user_agent': - 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36', + 'user_agent.original': USER_AGENT, }); }); @@ -49,6 +49,7 @@ describe('httpContextIntegration', () => { const integration = httpContextIntegration(); const span: Partial = { + is_segment: true, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', }, @@ -56,17 +57,33 @@ describe('httpContextIntegration', () => { const browserClient = new BrowserClient(getDefaultBrowserClientOptions()); - integration.processSegmentSpan!(span as StreamedSpanJSON, browserClient); + integration.processSpan!(span as StreamedSpanJSON, browserClient); expect(span.attributes).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', 'http.request.header.referer': 'https://example.com', - 'http.request.header.user_agent': - 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36', + 'user_agent.original': USER_AGENT, 'url.full': 'https://example.com', }); }); + it('only attaches the user agent to non-segment spans', () => { + const integration = httpContextIntegration(); + + const span: Partial = { + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.click', + }, + }; + + integration.processSpan!(span as StreamedSpanJSON, new BrowserClient(getDefaultBrowserClientOptions())); + + expect(span.attributes).toEqual({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.click', + 'user_agent.original': USER_AGENT, + }); + }); + describe('dataCollection', () => { it('attaches headers to events by default', () => { const client = new BrowserClient(getDefaultBrowserClientOptions()); @@ -140,10 +157,11 @@ describe('httpContextIntegration', () => { getDefaultBrowserClientOptions({ dataCollection: { httpHeaders: { request: false } } }), ); const span: Partial = { + is_segment: true, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload' }, }; - httpContextIntegration().processSegmentSpan!(span as StreamedSpanJSON, client); + httpContextIntegration().processSpan!(span as StreamedSpanJSON, client); expect(span.attributes).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', @@ -156,16 +174,17 @@ describe('httpContextIntegration', () => { getDefaultBrowserClientOptions({ dataCollection: { httpHeaders: { request: { deny: ['referer'] } } } }), ); const span: Partial = { + is_segment: true, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload' }, }; - httpContextIntegration().processSegmentSpan!(span as StreamedSpanJSON, client); + httpContextIntegration().processSpan!(span as StreamedSpanJSON, client); expect(span.attributes).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', 'url.full': 'https://example.com', 'http.request.header.referer': '[Filtered]', - 'http.request.header.user_agent': USER_AGENT, + 'user_agent.original': USER_AGENT, }); }); });