-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
test(e2e): Port the React Router instrumentation API E2E app to span streaming #23845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
andreiborza
wants to merge
2
commits into
ab/js-3482/framework-apps
from
ab/js-3482/instrumentation-api
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 0 additions & 1 deletion
1
...kages/e2e-tests/test-applications/react-router-7-framework-instrumentation/instrument.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
121 changes: 50 additions & 71 deletions
121
...applications/react-router-7-framework-instrumentation/tests/performance/db.server.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,111 +1,90 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
| import { waitForTransaction } from '@sentry-internal/test-utils'; | ||
| import { collectStreamedTrace, getSpanOp } from '@sentry-internal/test-utils'; | ||
| import { APP_NAME } from '../constants'; | ||
|
|
||
| // Same spans in both runs, from two injectors: the build-time transform in the server bundle, and | ||
| // the runtime hook in `react-router dev`, where the drivers stay on Node's own loader. | ||
| test.describe('server - orchestrion db instrumentation', () => { | ||
| test('instruments ioredis automatically via orchestrion', async ({ page }) => { | ||
| const transactionEventPromise = waitForTransaction(APP_NAME, transactionEvent => { | ||
| return ( | ||
| transactionEvent.contexts?.trace?.op === 'http.server' && | ||
| transactionEvent.transaction === 'GET /performance/db-ioredis' | ||
| ); | ||
| }); | ||
| const spansPromise = collectStreamedTrace(APP_NAME, span => span.name === 'GET /performance/db-ioredis'); | ||
|
|
||
| await page.goto('/performance/db-ioredis'); | ||
|
|
||
| const transactionEvent = await transactionEventPromise; | ||
| const spans = transactionEvent.spans || []; | ||
| const spans = await spansPromise; | ||
| const segmentSpan = spans.find(span => span.name === 'GET /performance/db-ioredis' && span.is_segment)!; | ||
|
|
||
| // The server transaction must come from the native instrumentation API (not the legacy handler), | ||
| // The server segment must come from the native instrumentation API (not the legacy handler), | ||
| // proving the orchestrion-injected db spans share context with the React Router server span. | ||
| expect(transactionEvent.contexts?.trace?.origin).toBe('auto.http.react_router.instrumentation_api'); | ||
| expect(getSpanOp(segmentSpan)).toBe('http.server'); | ||
| expect(segmentSpan.attributes['sentry.origin']?.value).toBe('auto.http.react_router.instrumentation_api'); | ||
|
|
||
| const childSpans = spans.filter(span => !span.is_segment); | ||
|
|
||
| expect(spans).toContainEqual( | ||
| expect(childSpans).toContainEqual( | ||
| expect.objectContaining({ | ||
| op: 'db.query', | ||
| origin: 'auto.db.redis', | ||
| description: 'set test-key [1 other arguments]', | ||
| name: 'set test-key [1 other arguments]', | ||
| status: 'ok', | ||
| data: expect.objectContaining({ | ||
| 'db.system.name': 'redis', | ||
| 'db.operation.name': 'set', | ||
| 'db.query.text': 'set test-key [1 other arguments]', | ||
| attributes: expect.objectContaining({ | ||
| 'sentry.op': { value: 'db.query', type: 'string' }, | ||
| 'sentry.origin': { value: 'auto.db.redis', type: 'string' }, | ||
| 'db.system.name': { value: 'redis', type: 'string' }, | ||
| 'db.operation.name': { value: 'set', type: 'string' }, | ||
| 'db.query.text': { value: 'set test-key [1 other arguments]', type: 'string' }, | ||
| }), | ||
| }), | ||
| ); | ||
| expect(spans).toContainEqual( | ||
| expect(childSpans).toContainEqual( | ||
| expect.objectContaining({ | ||
| op: 'db.query', | ||
| origin: 'auto.db.redis', | ||
| description: 'get test-key', | ||
| name: 'get test-key', | ||
| status: 'ok', | ||
| data: expect.objectContaining({ | ||
| 'db.system.name': 'redis', | ||
| 'db.operation.name': 'get', | ||
| 'db.query.text': 'get test-key', | ||
| attributes: expect.objectContaining({ | ||
| 'sentry.op': { value: 'db.query', type: 'string' }, | ||
| 'sentry.origin': { value: 'auto.db.redis', type: 'string' }, | ||
| 'db.system.name': { value: 'redis', type: 'string' }, | ||
| 'db.operation.name': { value: 'get', type: 'string' }, | ||
| 'db.query.text': { value: 'get test-key', type: 'string' }, | ||
| }), | ||
| }), | ||
| ); | ||
|
|
||
| // Each command maps to exactly one span (no offline-queue duplicate). | ||
| const setSpans = spans.filter(span => span.description === 'set test-key [1 other arguments]'); | ||
| const setSpans = spans.filter(span => span.name === 'set test-key [1 other arguments]'); | ||
| expect(setSpans).toHaveLength(1); | ||
|
|
||
| // Every db span nests under the native instrumentation-API http.server transaction. | ||
| const rootSpanId = transactionEvent.contexts?.trace?.span_id; | ||
| const spanIds = new Set([rootSpanId, ...spans.map(span => span.span_id)]); | ||
| const dbSpans = spans.filter(span => span.origin === 'auto.db.redis'); | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| // Every db span nests under the native instrumentation-API http.server segment. | ||
| const spanIds = new Set(spans.filter(span => span.trace_id === segmentSpan.trace_id).map(span => span.span_id)); | ||
| const dbSpans = spans.filter(span => span.attributes['sentry.origin']?.value === 'auto.db.redis'); | ||
| expect(dbSpans.every(span => typeof span.parent_span_id === 'string' && spanIds.has(span.parent_span_id))).toBe( | ||
| true, | ||
| ); | ||
| }); | ||
|
|
||
| // Under span streaming the mysql span name is the query summary, so both queries below are named | ||
| // `SELECT`. `db.query.text` is what tells them apart. | ||
| test('instruments mysql automatically via orchestrion', async ({ page }) => { | ||
| const transactionEventPromise = waitForTransaction(APP_NAME, transactionEvent => { | ||
| return ( | ||
| transactionEvent.contexts?.trace?.op === 'http.server' && | ||
| transactionEvent.transaction === 'GET /performance/db-mysql' | ||
| ); | ||
| }); | ||
| const spansPromise = collectStreamedTrace(APP_NAME, span => span.name === 'GET /performance/db-mysql'); | ||
|
|
||
| await page.goto('/performance/db-mysql'); | ||
|
|
||
| const transactionEvent = await transactionEventPromise; | ||
| const spans = transactionEvent.spans || []; | ||
| const spans = await spansPromise; | ||
|
|
||
| expect(spans).toContainEqual( | ||
| expect.objectContaining({ | ||
| op: 'db', | ||
| origin: 'auto.db.mysql', | ||
| description: 'SELECT 1 + 1 AS solution', | ||
| status: 'ok', | ||
| data: expect.objectContaining({ | ||
| 'db.system.name': 'mysql', | ||
| 'db.query.text': 'SELECT 1 + 1 AS solution', | ||
| 'db.user': 'root', | ||
| 'db.connection_string': expect.any(String), | ||
| 'server.address': expect.any(String), | ||
| 'server.port': 3306, | ||
| for (const queryText of ['SELECT 1 + 1 AS solution', 'SELECT NOW()']) { | ||
| expect(spans).toContainEqual( | ||
| expect.objectContaining({ | ||
| name: 'SELECT', | ||
| status: 'ok', | ||
| attributes: expect.objectContaining({ | ||
| 'sentry.op': { value: 'db', type: 'string' }, | ||
| 'sentry.origin': { value: 'auto.db.mysql', type: 'string' }, | ||
| 'db.system.name': { value: 'mysql', type: 'string' }, | ||
| 'db.query.text': { value: queryText, type: 'string' }, | ||
| 'db.user': { value: 'root', type: 'string' }, | ||
| 'db.connection_string': { value: expect.any(String), type: 'string' }, | ||
| 'server.address': { value: expect.any(String), type: 'string' }, | ||
| 'server.port': { value: 3306, type: 'integer' }, | ||
| }), | ||
| }), | ||
| }), | ||
| ); | ||
| expect(spans).toContainEqual( | ||
| expect.objectContaining({ | ||
| op: 'db', | ||
| origin: 'auto.db.mysql', | ||
| description: 'SELECT NOW()', | ||
| status: 'ok', | ||
| data: expect.objectContaining({ | ||
| 'db.system.name': 'mysql', | ||
| 'db.query.text': 'SELECT NOW()', | ||
| 'db.user': 'root', | ||
| 'db.connection_string': expect.any(String), | ||
| 'server.address': expect.any(String), | ||
| 'server.port': 3306, | ||
| }), | ||
| }), | ||
| ); | ||
| ); | ||
| } | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Child span collection can flake
Medium Severity
collectStreamedTraceis used without anisDonepredicate in tests that then assert on child spans. That helper resolves as soon as the segment arrives, and streamed children can flush after it, so loader, middleware, redis, and mysql assertions can fail even when the spans are later sent. The same PR already waits for those children incollectUntilSegment, and the siblingreact-router-7-frameworktests do too. This is flagged because the review rules call out races when waiting on telemetry that can arrive in arbitrary order, including inlazy.server.test.ts.Additional Locations (2)
dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/errors/errors.server.test.ts#L43-L54dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts#L6-L11Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 7d44686. Configure here.