Skip to content

Commit da10dcd

Browse files
smgvcursoragent
andcommitted
fix(sentry): discard timed-out onboarding traces instead of sending fake ~5-min durations
When a span is still open after TRACES_CLEANUP_INTERVAL (5 min), the cleanup timeout now marks it with SpanStatus ERROR / deadline_exceeded and sets `trace.timed_out = true` before calling end(). The `excludeEvents` beforeSendTransaction hook drops any event carrying that flag, so artificially-clamped spans (p50 ≈ 300,014 ms for "Onboarding - Overall Journey") are never sent to Sentry. Legitimate long-but-real spans are unaffected because the drop is keyed strictly on the flag, not on duration. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent da9a364 commit da10dcd

4 files changed

Lines changed: 80 additions & 1 deletion

File tree

app/util/sentry/utils.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,38 @@ describe('excludeEvents', () => {
249249
expect(result.start_timestamp).toBe(1234567890);
250250
});
251251

252+
it('drops event when trace.timed_out flag is true', () => {
253+
const event = {
254+
transaction: 'Onboarding - Overall Journey',
255+
contexts: {
256+
trace: {
257+
data: {
258+
'trace.timed_out': true,
259+
},
260+
},
261+
},
262+
};
263+
264+
const result = excludeEvents(event);
265+
expect(result).toBeNull();
266+
});
267+
268+
it('keeps event when trace.timed_out flag is absent', () => {
269+
const event = {
270+
transaction: 'Onboarding - Overall Journey',
271+
contexts: {
272+
trace: {
273+
data: {
274+
'some.other.attr': 'value',
275+
},
276+
},
277+
},
278+
};
279+
280+
const result = excludeEvents(event);
281+
expect(result).toBe(event);
282+
});
283+
252284
it('returns main-exp for experimental environment and main build type', async () => {
253285
const METAMASK_ENVIRONMENT = 'exp';
254286
const isDev = false;

app/util/sentry/utils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,10 @@ export function excludeEvents(event: SentryEvent | null): SentryEvent | null {
481481
}
482482
}
483483
}
484+
if (event?.contexts?.trace?.data?.['trace.timed_out'] === true) {
485+
return null;
486+
}
487+
484488
//Modify or drop event here
485489
if (event?.transaction === 'Route Change') {
486490
//Route change is dropped because is does not reflect a screen we can action on.

app/util/trace.test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,11 @@ describe('Trace', () => {
382382
updateCachedConsent(true);
383383

384384
const spanEndMock = jest.fn();
385-
const spanMock = { end: spanEndMock } as unknown as Span;
385+
const spanMock = {
386+
end: spanEndMock,
387+
setStatus: jest.fn(),
388+
setAttribute: jest.fn(),
389+
} as unknown as Span;
386390

387391
startSpanManualMock.mockImplementationOnce((_, fn) =>
388392
fn(spanMock, () => {
@@ -404,6 +408,41 @@ describe('Trace', () => {
404408

405409
expect(spanEndMock).toHaveBeenCalledTimes(1);
406410
});
411+
412+
it('marks span as timed out with deadline_exceeded status when cleanup fires for open span', () => {
413+
updateCachedConsent(true);
414+
415+
const spanEndMock = jest.fn();
416+
const setStatusMock = jest.fn();
417+
const setAttributeMock = jest.fn();
418+
const spanMock = {
419+
end: spanEndMock,
420+
setStatus: setStatusMock,
421+
setAttribute: setAttributeMock,
422+
} as unknown as Span;
423+
424+
startSpanManualMock.mockImplementationOnce((_, fn) =>
425+
fn(spanMock, () => {
426+
// Intentionally empty
427+
}),
428+
);
429+
430+
trace({
431+
name: NAME_MOCK,
432+
id: ID_MOCK,
433+
tags: TAGS_MOCK,
434+
data: DATA_MOCK,
435+
});
436+
437+
jest.advanceTimersByTime(TRACES_CLEANUP_INTERVAL);
438+
439+
expect(setStatusMock).toHaveBeenCalledWith({
440+
code: 2,
441+
message: 'deadline_exceeded',
442+
});
443+
expect(setAttributeMock).toHaveBeenCalledWith('trace.timed_out', true);
444+
expect(spanEndMock).toHaveBeenCalledTimes(1);
445+
});
407446
});
408447

409448
describe('flushBufferedTraces', () => {

app/util/trace.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,10 @@ function startTrace(request: TraceRequest): TraceContext {
732732

733733
const timeoutId = setTimeout(() => {
734734
log('Trace cleanup due to timeout', name, id);
735+
if (span) {
736+
span.setStatus({ code: 2, message: 'deadline_exceeded' });
737+
span.setAttribute('trace.timed_out', true);
738+
}
735739
end();
736740
tracesByKey.delete(getTraceKey(request));
737741
}, TRACES_CLEANUP_INTERVAL);

0 commit comments

Comments
 (0)