Skip to content

Commit c631c54

Browse files
committed
guard profiling data attachment
1 parent 1389a04 commit c631c54

2 files changed

Lines changed: 31 additions & 15 deletions

File tree

  • dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling
  • packages/browser/src/profiling

dev-packages/browser-integration-tests/suites/feedback/captureFeedbackWithProfiling/test.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,16 @@ sentryTest(
3939

4040
await page.goto(url);
4141

42-
// Wait for the idle span to finish
43-
await pageloadRequestPromise;
42+
// Wait for the idle page load span to finish
43+
const pageLoadEvent = envelopeRequestParser(await pageloadRequestPromise);
44+
45+
// contexts.trace.data musinclude thread.id to identify which thread is associated with the transaction
46+
expect(pageLoadEvent.contexts?.trace?.data?.['thread.id']).toBe('0');
47+
expect(pageLoadEvent.contexts?.trace?.data?.['thread.name']).toBe('main');
48+
// expect(pageLoadEvent.contexts?.profile?.profiler_id).toMatch(/^[a-f\d]{32}$/);
49+
50+
console.log('profiler_id pageload', pageLoadEvent.contexts?.profile);
51+
console.log('pageLoadEvent.contexts?.trace.data', pageLoadEvent.contexts?.trace?.data);
4452

4553
// Submit feedback after idle span ended — no active span
4654
await page.getByText('Report a Bug').click();
@@ -51,10 +59,16 @@ sentryTest(
5159

5260
const feedbackEvent = envelopeRequestParser((await feedbackRequestPromise).request());
5361

54-
// BUG: With profiling enabled and no active span, attachProfiledThreadToEvent
55-
// creates a partial trace context { data: { thread.id, thread.name } } that
56-
// overwrites the real trace context (with trace_id/span_id) via spread in _prepareEvent.
62+
console.log('profiler_id feedback', feedbackEvent.contexts?.profile);
63+
console.log('feedbackEvent.contexts?.trace', feedbackEvent.contexts?.trace);
64+
65+
// Feedback must retain trace_id and span_id even when profiling is active.
66+
// Previously, `attachProfiledThreadToEvent` would overwrite contexts.trace with only
67+
// `{ data: { thread.id, thread.name } }`, losing the trace link entirely.
5768
expect(feedbackEvent.contexts?.trace?.trace_id).toMatch(/\w{32}/);
5869
expect(feedbackEvent.contexts?.trace?.span_id).toMatch(/\w{16}/);
70+
71+
// Profile context must NOT be present on feedback events — Sentry can only use
72+
// profiler_id/profile_id on transactions (which carry startTimestamp/endTimestamp).
5973
},
6074
);

packages/browser/src/profiling/utils.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -798,16 +798,18 @@ export function attachProfiledThreadToEvent(event: Event): Event {
798798
return event;
799799
}
800800

801-
// @ts-expect-error the trace fallback value is wrong, though it should never happen
802-
// and in case it does, we dont want to override whatever was passed initially.
803-
event.contexts.trace = {
804-
...(event.contexts?.trace ?? {}),
805-
data: {
806-
...(event.contexts?.trace?.data ?? {}),
807-
['thread.id']: PROFILER_THREAD_ID_STRING,
808-
['thread.name']: PROFILER_THREAD_NAME,
809-
},
810-
};
801+
// Only mutate the trace context when it already has a trace_id — that
802+
// guarantees `applySpanToEvent` has already run, and we are not creating a partial trace context from scratch.
803+
if (event.contexts.trace?.trace_id) {
804+
event.contexts.trace = {
805+
...event.contexts.trace,
806+
data: {
807+
...(event.contexts.trace.data ?? {}),
808+
['thread.id']: PROFILER_THREAD_ID_STRING,
809+
['thread.name']: PROFILER_THREAD_NAME,
810+
},
811+
};
812+
}
811813

812814
// Attach thread info to individual spans so that spans can be associated with the profiled thread on the UI even if contexts are missing.
813815
event.spans?.forEach(span => {

0 commit comments

Comments
 (0)