From 1389a049499162e94f92d20c2dcf722e6fc44f80 Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Thu, 5 Mar 2026 16:35:35 +0100 Subject: [PATCH 1/5] fix(browser): Fix missing traces for user feedback --- .../captureFeedbackWithProfiling/init.js | 18 ++++++ .../template.html | 7 +++ .../captureFeedbackWithProfiling/test.ts | 60 +++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/init.js create mode 100644 dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/template.html create mode 100644 dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/init.js b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/init.js new file mode 100644 index 000000000000..45b89df6243d --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/init.js @@ -0,0 +1,18 @@ +import * as Sentry from '@sentry/browser'; +// Import these separately so that generatePlugin can handle them for CDN scenarios +import { browserProfilingIntegration, feedbackIntegration } from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + Sentry.browserTracingIntegration(), + browserProfilingIntegration(), + feedbackIntegration({ + tags: { from: 'integration init' }, + }), + ], + tracesSampleRate: 1.0, + profilesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/template.html b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/template.html new file mode 100644 index 000000000000..7c26f485c41b --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/template.html @@ -0,0 +1,7 @@ + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts new file mode 100644 index 000000000000..f7f8eb935986 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts @@ -0,0 +1,60 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../utils/fixtures'; +import { + envelopeRequestParser, + getEnvelopeType, + shouldSkipFeedbackTest, + shouldSkipTracingTest, + waitForTransactionRequest, +} from '../../../utils/helpers'; + +sentryTest( + 'feedback should have trace_id when profiling is enabled and idle span has ended', + async ({ getLocalTestUrl, page, browserName }) => { + if (shouldSkipFeedbackTest() || shouldSkipTracingTest() || browserName !== 'chromium') { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ + testDir: __dirname, + handleLazyLoadedFeedback: true, + responseHeaders: { 'Document-Policy': 'js-profiling' }, + }); + + // Wait for the pageload transaction to be sent (idle span ended) + const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); + + const feedbackRequestPromise = page.waitForResponse(res => { + const req = res.request(); + const postData = req.postData(); + if (!postData) { + return false; + } + try { + return getEnvelopeType(req) === 'feedback'; + } catch { + return false; + } + }); + + await page.goto(url); + + // Wait for the idle span to finish + await pageloadRequestPromise; + + // Submit feedback after idle span ended — no active span + await page.getByText('Report a Bug').click(); + await page.locator('[name="name"]').fill('Jane Doe'); + await page.locator('[name="email"]').fill('janedoe@example.org'); + await page.locator('[name="message"]').fill('feedback after idle span ended'); + await page.locator('[data-sentry-feedback] .btn--primary').click(); + + const feedbackEvent = envelopeRequestParser((await feedbackRequestPromise).request()); + + // BUG: With profiling enabled and no active span, attachProfiledThreadToEvent + // creates a partial trace context { data: { thread.id, thread.name } } that + // overwrites the real trace context (with trace_id/span_id) via spread in _prepareEvent. + expect(feedbackEvent.contexts?.trace?.trace_id).toMatch(/\w{32}/); + expect(feedbackEvent.contexts?.trace?.span_id).toMatch(/\w{16}/); + }, +); From a5c830be590311edeb0c5aef904a4a5d04b74bad Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 6 Mar 2026 13:56:23 +0100 Subject: [PATCH 2/5] guard profiling data attachment; add tests --- .../captureFeedbackWithProfiling/test.ts | 14 ++++++---- .../suites/profiling/legacyMode/test.ts | 6 +++++ .../test.ts | 26 ++++++++++++++++++- .../test.ts | 5 +++- packages/browser/src/profiling/utils.ts | 22 +++++++++------- 5 files changed, 56 insertions(+), 17 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts index f7f8eb935986..860b1639a4c9 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts @@ -39,8 +39,8 @@ sentryTest( await page.goto(url); - // Wait for the idle span to finish - await pageloadRequestPromise; + // Wait for the idle page load span to finish + const pageLoadEvent = envelopeRequestParser(await pageloadRequestPromise); // Submit feedback after idle span ended — no active span await page.getByText('Report a Bug').click(); @@ -51,10 +51,14 @@ sentryTest( const feedbackEvent = envelopeRequestParser((await feedbackRequestPromise).request()); - // BUG: With profiling enabled and no active span, attachProfiledThreadToEvent - // creates a partial trace context { data: { thread.id, thread.name } } that - // overwrites the real trace context (with trace_id/span_id) via spread in _prepareEvent. expect(feedbackEvent.contexts?.trace?.trace_id).toMatch(/\w{32}/); expect(feedbackEvent.contexts?.trace?.span_id).toMatch(/\w{16}/); + + // contexts.trace.data must include thread.id to identify which thread is associated with the transaction + expect(pageLoadEvent.contexts?.trace?.data?.['thread.id']).toBe('0'); + expect(pageLoadEvent.contexts?.trace?.data?.['thread.name']).toBe('main'); + + // fixme: figure out why profiler_id is set on feedback and not on pageload transaction + expect(feedbackEvent.contexts?.profile?.profiler_id).toMatch(/^[a-f\d]{32}$/); }, ); diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts index 4d8caa3a2be3..9726fd283cf4 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts @@ -36,7 +36,9 @@ sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestU const url = await getLocalTestUrl({ testDir: __dirname, responseHeaders: { 'Document-Policy': 'js-profiling' } }); const req = await waitForTransactionRequestOnUrl(page, url); + const transactionEvent = properEnvelopeRequestParser(req, 0); const profileEvent = properEnvelopeRequestParser(req, 1); + expect(profileEvent).toBeDefined(); const profile = profileEvent.profile; @@ -54,4 +56,8 @@ sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestU minSampleDurationMs: 20, isChunkFormat: false, }); + + // contexts.trace.data must include thread.id to identify which thread is associated with the transaction + expect(transactionEvent?.contexts?.trace?.data?.['thread.id']).toBe('0'); + expect(transactionEvent?.contexts?.trace?.data?.['thread.name']).toBe('main'); }); diff --git a/dev-packages/browser-integration-tests/suites/profiling/traceLifecycleMode_multiple-chunks/test.ts b/dev-packages/browser-integration-tests/suites/profiling/traceLifecycleMode_multiple-chunks/test.ts index a16a054da45a..124dd3c2292e 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/traceLifecycleMode_multiple-chunks/test.ts +++ b/dev-packages/browser-integration-tests/suites/profiling/traceLifecycleMode_multiple-chunks/test.ts @@ -1,11 +1,13 @@ import { expect } from '@playwright/test'; -import type { ProfileChunkEnvelope } from '@sentry/core'; +import type { Event, ProfileChunkEnvelope } from '@sentry/core'; import { sentryTest } from '../../../utils/fixtures'; import { countEnvelopes, getMultipleSentryEnvelopeRequests, + properEnvelopeRequestParser, properFullEnvelopeRequestParser, shouldSkipTracingTest, + waitForTransactionRequestOnUrl, } from '../../../utils/helpers'; import { validateProfile, validateProfilePayloadMetadata } from '../test-utils'; @@ -96,3 +98,25 @@ sentryTest( }); }, ); + +sentryTest( + 'attaches profiler_id and thread data to transaction events (trace mode)', + async ({ page, getLocalTestUrl, browserName }) => { + if (shouldSkipTracingTest() || browserName !== 'chromium') { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname, responseHeaders: { 'Document-Policy': 'js-profiling' } }); + + const req = await waitForTransactionRequestOnUrl(page, url); + const transactionEvent = properEnvelopeRequestParser(req, 0) as any; + + expect(transactionEvent?.type).toBe('transaction'); + + expect(transactionEvent?.contexts?.profile?.profiler_id).toMatch(/^[a-f\d]{32}$/); + + // contexts.trace.data must include thread.id to identify which thread is associated with the transaction + expect(transactionEvent?.contexts?.trace?.data?.['thread.id']).toBe('0'); + expect(transactionEvent?.contexts?.trace?.data?.['thread.name']).toBe('main'); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/profiling/traceLifecycleMode_overlapping-spans/test.ts b/dev-packages/browser-integration-tests/suites/profiling/traceLifecycleMode_overlapping-spans/test.ts index 487cd05f1dbd..e05685ca4868 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/traceLifecycleMode_overlapping-spans/test.ts +++ b/dev-packages/browser-integration-tests/suites/profiling/traceLifecycleMode_overlapping-spans/test.ts @@ -89,9 +89,12 @@ sentryTest('attaches thread data to child spans (trace mode)', async ({ page, ge const profilerId = rootSpan?.contexts?.profile?.profiler_id as string | undefined; expect(typeof profilerId).toBe('string'); - expect(profilerId).toMatch(/^[a-f\d]{32}$/); + // contexts.trace.data must include thread.id to identify which thread is associated with the transaction + expect(rootSpan?.contexts?.trace?.data?.['thread.id']).toBe('0'); + expect(rootSpan?.contexts?.trace?.data?.['thread.name']).toBe('main'); + const spans = (rootSpan?.spans ?? []) as Array<{ data?: Record }>; expect(spans.length).toBeGreaterThan(0); for (const span of spans) { diff --git a/packages/browser/src/profiling/utils.ts b/packages/browser/src/profiling/utils.ts index c50c76c84de4..dceb5e45a691 100644 --- a/packages/browser/src/profiling/utils.ts +++ b/packages/browser/src/profiling/utils.ts @@ -798,16 +798,18 @@ export function attachProfiledThreadToEvent(event: Event): Event { return event; } - // @ts-expect-error the trace fallback value is wrong, though it should never happen - // and in case it does, we dont want to override whatever was passed initially. - event.contexts.trace = { - ...(event.contexts?.trace ?? {}), - data: { - ...(event.contexts?.trace?.data ?? {}), - ['thread.id']: PROFILER_THREAD_ID_STRING, - ['thread.name']: PROFILER_THREAD_NAME, - }, - }; + // Only mutate the trace context when it already has a trace_id — that + // guarantees `applySpanToEvent` has already run, and we are not creating a partial trace context from scratch. + if (event.contexts.trace?.trace_id) { + event.contexts.trace = { + ...event.contexts.trace, + data: { + ...(event.contexts.trace.data ?? {}), + ['thread.id']: PROFILER_THREAD_ID_STRING, + ['thread.name']: PROFILER_THREAD_NAME, + }, + }; + } // Attach thread info to individual spans so that spans can be associated with the profiled thread on the UI even if contexts are missing. event.spans?.forEach(span => { From ea6aefc3169285e3b7a97a50881cd6ba5e2c76b6 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 6 Mar 2026 15:11:08 +0100 Subject: [PATCH 3/5] wait for text --- .../suites/feedback/captureFeedbackWithProfiling/test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts index 860b1639a4c9..c2e98f75cc97 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts @@ -43,6 +43,7 @@ sentryTest( const pageLoadEvent = envelopeRequestParser(await pageloadRequestPromise); // Submit feedback after idle span ended — no active span + await page.getByText('Report a Bug').waitFor({ state: 'visible' }); await page.getByText('Report a Bug').click(); await page.locator('[name="name"]').fill('Jane Doe'); await page.locator('[name="email"]').fill('janedoe@example.org'); From 68b967a7e9a60299978568a12a0ba12cb9e478d2 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 6 Mar 2026 16:04:06 +0100 Subject: [PATCH 4/5] add context --- .../suites/feedback/captureFeedbackWithProfiling/test.ts | 3 ++- packages/core/src/client.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts index c2e98f75cc97..9242af676a96 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts @@ -60,6 +60,7 @@ sentryTest( expect(pageLoadEvent.contexts?.trace?.data?.['thread.name']).toBe('main'); // fixme: figure out why profiler_id is set on feedback and not on pageload transaction - expect(feedbackEvent.contexts?.profile?.profiler_id).toMatch(/^[a-f\d]{32}$/); + // todo: rename to `profiler_id` + expect(feedbackEvent.contexts?.profile?.profile_id).toMatch(/^[a-f\d]{32}$/); }, ); diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 766e66a72a99..3afe8fa2442c 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -1222,7 +1222,7 @@ export abstract class Client { this.emit('postprocessEvent', evt, hint); evt.contexts = { - trace: getTraceContextFromScope(currentScope), + trace: { ...evt.contexts?.trace, ...getTraceContextFromScope(currentScope) }, ...evt.contexts, }; From ada3e88db5f8580e3233127fe5751d17dae89bf5 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 9 Mar 2026 09:57:28 +0100 Subject: [PATCH 5/5] fix bundle tests --- .../suites/feedback/captureFeedbackWithProfiling/init.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/init.js b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/init.js index 45b89df6243d..eafdd7b8605e 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/init.js +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/init.js @@ -1,6 +1,9 @@ import * as Sentry from '@sentry/browser'; // Import these separately so that generatePlugin can handle them for CDN scenarios -import { browserProfilingIntegration, feedbackIntegration } from '@sentry/browser'; +// eslint-disable-next-line import/no-duplicates +import { browserProfilingIntegration } from '@sentry/browser'; +// eslint-disable-next-line import/no-duplicates +import { feedbackIntegration } from '@sentry/browser'; window.Sentry = Sentry;