Skip to content

Commit 60c0eb8

Browse files
committed
fix(browser): Add ok status to succesful idleSpans
1 parent 10211f4 commit 60c0eb8

File tree

2 files changed

+96
-1
lines changed

2 files changed

+96
-1
lines changed

dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ test('Creates a pageload transaction with parameterized route', async ({ page })
2121
expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
2222
expect(event.type).toBe('transaction');
2323
expect(event.contexts?.trace?.op).toBe('pageload');
24+
expect(event.contexts?.trace?.status).toBe('ok');
2425
});
2526

2627
test('Does not create a navigation transaction on initial load to deep lazy route', async ({ page }) => {
@@ -82,6 +83,7 @@ test('Creates a navigation transaction inside a lazy route', async ({ page }) =>
8283
expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
8384
expect(event.type).toBe('transaction');
8485
expect(event.contexts?.trace?.op).toBe('navigation');
86+
expect(event.contexts?.trace?.status).toBe('ok');
8587
});
8688

8789
test('Creates navigation transactions between two different lazy routes', async ({ page }) => {
@@ -498,3 +500,90 @@ test('Updates navigation transaction name correctly when span is cancelled early
498500
expect(['externalFinish', 'cancelled']).toContain(idleSpanFinishReason);
499501
}
500502
});
503+
504+
test('Creates separate transactions for rapid consecutive navigations', async ({ page }) => {
505+
await page.goto('/');
506+
507+
// First navigation: / -> /lazy/inner/:id/:anotherId/:someAnotherId
508+
const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
509+
return (
510+
!!transactionEvent?.transaction &&
511+
transactionEvent.contexts?.trace?.op === 'navigation' &&
512+
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
513+
);
514+
});
515+
516+
const navigationToInner = page.locator('id=navigation');
517+
await expect(navigationToInner).toBeVisible();
518+
await navigationToInner.click();
519+
520+
const firstEvent = await firstTransactionPromise;
521+
522+
// Verify first transaction
523+
expect(firstEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
524+
expect(firstEvent.contexts?.trace?.op).toBe('navigation');
525+
expect(firstEvent.contexts?.trace?.status).toBe('ok');
526+
const firstTraceId = firstEvent.contexts?.trace?.trace_id;
527+
const firstSpanId = firstEvent.contexts?.trace?.span_id;
528+
529+
// Second navigation: /lazy/inner -> /another-lazy/sub/:id/:subId
530+
const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
531+
return (
532+
!!transactionEvent?.transaction &&
533+
transactionEvent.contexts?.trace?.op === 'navigation' &&
534+
transactionEvent.transaction === '/another-lazy/sub/:id/:subId'
535+
);
536+
});
537+
538+
const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
539+
await expect(navigationToAnother).toBeVisible();
540+
await navigationToAnother.click();
541+
542+
const secondEvent = await secondTransactionPromise;
543+
544+
// Verify second transaction
545+
expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
546+
expect(secondEvent.contexts?.trace?.op).toBe('navigation');
547+
expect(secondEvent.contexts?.trace?.status).toBe('ok');
548+
const secondTraceId = secondEvent.contexts?.trace?.trace_id;
549+
const secondSpanId = secondEvent.contexts?.trace?.span_id;
550+
551+
// Third navigation: /another-lazy -> /lazy/inner/:id/:anotherId/:someAnotherId (back to same route as first)
552+
const thirdTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
553+
return (
554+
!!transactionEvent?.transaction &&
555+
transactionEvent.contexts?.trace?.op === 'navigation' &&
556+
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' &&
557+
// Ensure we're not matching the first transaction again
558+
transactionEvent.contexts?.trace?.trace_id !== firstTraceId
559+
);
560+
});
561+
562+
const navigationBackToInner = page.locator('id=navigate-to-inner-from-deep');
563+
await expect(navigationBackToInner).toBeVisible();
564+
await navigationBackToInner.click();
565+
566+
const thirdEvent = await thirdTransactionPromise;
567+
568+
// Verify third transaction
569+
expect(thirdEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
570+
expect(thirdEvent.contexts?.trace?.op).toBe('navigation');
571+
expect(thirdEvent.contexts?.trace?.status).toBe('ok');
572+
const thirdTraceId = thirdEvent.contexts?.trace?.trace_id;
573+
const thirdSpanId = thirdEvent.contexts?.trace?.span_id;
574+
575+
// Verify each navigation created a separate transaction with unique trace and span IDs
576+
expect(firstTraceId).toBeDefined();
577+
expect(secondTraceId).toBeDefined();
578+
expect(thirdTraceId).toBeDefined();
579+
580+
// All trace IDs should be unique
581+
expect(firstTraceId).not.toBe(secondTraceId);
582+
expect(secondTraceId).not.toBe(thirdTraceId);
583+
expect(firstTraceId).not.toBe(thirdTraceId);
584+
585+
// All span IDs should be unique
586+
expect(firstSpanId).not.toBe(secondSpanId);
587+
expect(secondSpanId).not.toBe(thirdSpanId);
588+
expect(firstSpanId).not.toBe(thirdSpanId);
589+
});

packages/core/src/tracing/idleSpan.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { timestampInSeconds } from '../utils/time';
1919
import { freezeDscOnSpan, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
2020
import { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
2121
import { SentrySpan } from './sentrySpan';
22-
import { SPAN_STATUS_ERROR } from './spanstatus';
22+
import { SPAN_STATUS_ERROR, SPAN_STATUS_OK } from './spanstatus';
2323
import { startInactiveSpan } from './trace';
2424

2525
export const TRACING_DEFAULTS = {
@@ -302,6 +302,12 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
302302
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON, _finishReason);
303303
}
304304

305+
// Set span status to 'ok' if it hasn't been explicitly set to an error status
306+
const currentStatus = spanJSON.status;
307+
if (!currentStatus || currentStatus === 'unknown') {
308+
span.setStatus({ code: SPAN_STATUS_OK });
309+
}
310+
305311
debug.log(`[Tracing] Idle span "${spanJSON.op}" finished`);
306312

307313
const childSpans = getSpanDescendants(span).filter(child => child !== span);

0 commit comments

Comments
 (0)