-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(nextjs): Migrate edge event processors to span-first APIs #20551
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { stripUrlQueryAndFragment } from '@sentry/core'; | ||
| import { ATTR_NEXT_SPAN_NAME, ATTR_NEXT_SPAN_TYPE } from '../common/nextSpanAttributes'; | ||
|
|
||
| export interface MutableMiddlewareRootSpan { | ||
| attributes: Record<string, unknown>; | ||
| getName(): string | undefined; | ||
| setName(name: string): void; | ||
| } | ||
|
|
||
| /** | ||
| * Normalizes the transaction name for the root span of a Next.js `Middleware.execute` request on the Edge runtime. | ||
| * | ||
| * Older Next.js versions append the full URL to the middleware span name (e.g. `middleware GET /foo?bar=1`), | ||
| * producing high-cardinality transaction names. We collapse the name to `middleware {METHOD}` when possible, | ||
| * and strip query/fragment otherwise. | ||
| * | ||
| * Called from two places that operate on different shapes of the same underlying root span: | ||
| * - Legacy mode: from `preprocessEvent`, adapted around a transaction `Event` whose `contexts.trace.data` | ||
| * holds the root span's attributes and whose `event.transaction` is the root span's name. | ||
| * - Streamed mode: from `processSegmentSpan`, adapted around a `StreamedSpanJSON` (the streamed | ||
| * counterpart of the legacy transaction root) directly. | ||
| */ | ||
| export function enhanceMiddlewareRootSpan(span: MutableMiddlewareRootSpan): void { | ||
| const { attributes } = span; | ||
|
|
||
| if (attributes[ATTR_NEXT_SPAN_TYPE] !== 'Middleware.execute') { | ||
| return; | ||
| } | ||
|
|
||
| const spanName = attributes[ATTR_NEXT_SPAN_NAME]; | ||
| if (typeof spanName !== 'string' || !spanName || !span.getName()) { | ||
| return; | ||
| } | ||
|
|
||
| const match = spanName.match(/^middleware (GET|POST|PUT|DELETE|PATCH|HEAD|OPTIONS)/); | ||
| if (match) { | ||
| span.setName(`middleware ${match[1]}`); | ||
| } else { | ||
| span.setName(stripUrlQueryAndFragment(spanName)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { ATTR_NEXT_SPAN_NAME, ATTR_NEXT_SPAN_TYPE } from '../../src/common/nextSpanAttributes'; | ||
| import { enhanceMiddlewareRootSpan } from '../../src/edge/enhanceMiddlewareRootSpan'; | ||
|
|
||
| function makeSpan(attributes: Record<string, unknown>, name?: string) { | ||
| let currentName = name; | ||
| return { | ||
| span: { | ||
| attributes, | ||
| getName: () => currentName, | ||
| setName: (n: string) => { | ||
| currentName = n; | ||
| }, | ||
| }, | ||
| getName: () => currentName, | ||
| }; | ||
| } | ||
|
|
||
| describe('enhanceMiddlewareRootSpan', () => { | ||
| it('does nothing for spans that are not Middleware.execute', () => { | ||
| const { span, getName } = makeSpan( | ||
| { [ATTR_NEXT_SPAN_TYPE]: 'BaseServer.handleRequest', [ATTR_NEXT_SPAN_NAME]: 'middleware GET /foo' }, | ||
| 'GET /foo', | ||
| ); | ||
|
|
||
| enhanceMiddlewareRootSpan(span); | ||
|
|
||
| expect(getName()).toBe('GET /foo'); | ||
| }); | ||
|
|
||
| it('does nothing when next.span_name is missing', () => { | ||
| const { span, getName } = makeSpan({ [ATTR_NEXT_SPAN_TYPE]: 'Middleware.execute' }, 'middleware'); | ||
|
|
||
| enhanceMiddlewareRootSpan(span); | ||
|
|
||
| expect(getName()).toBe('middleware'); | ||
| }); | ||
|
|
||
| it('does nothing when next.span_name is an empty string', () => { | ||
| const { span, getName } = makeSpan( | ||
| { [ATTR_NEXT_SPAN_TYPE]: 'Middleware.execute', [ATTR_NEXT_SPAN_NAME]: '' }, | ||
| 'middleware', | ||
| ); | ||
|
|
||
| enhanceMiddlewareRootSpan(span); | ||
|
|
||
| expect(getName()).toBe('middleware'); | ||
| }); | ||
|
|
||
| it('does nothing when next.span_name is not a string', () => { | ||
| const { span, getName } = makeSpan( | ||
| { [ATTR_NEXT_SPAN_TYPE]: 'Middleware.execute', [ATTR_NEXT_SPAN_NAME]: 123 }, | ||
| 'middleware', | ||
| ); | ||
|
|
||
| enhanceMiddlewareRootSpan(span); | ||
|
|
||
| expect(getName()).toBe('middleware'); | ||
| }); | ||
|
|
||
| it('does nothing when the current name is empty', () => { | ||
| const { span, getName } = makeSpan( | ||
| { [ATTR_NEXT_SPAN_TYPE]: 'Middleware.execute', [ATTR_NEXT_SPAN_NAME]: 'middleware GET /foo' }, | ||
| undefined, | ||
| ); | ||
|
|
||
| enhanceMiddlewareRootSpan(span); | ||
|
|
||
| expect(getName()).toBeUndefined(); | ||
| }); | ||
|
|
||
| it.each([ | ||
| ['middleware GET /foo', 'middleware GET'], | ||
| ['middleware POST /api/protected?token=abc', 'middleware POST'], | ||
| ['middleware DELETE /resources/[id]', 'middleware DELETE'], | ||
| ['middleware HEAD /', 'middleware HEAD'], | ||
| ])('collapses "%s" to "%s"', (spanName, expected) => { | ||
| const { span, getName } = makeSpan( | ||
| { [ATTR_NEXT_SPAN_TYPE]: 'Middleware.execute', [ATTR_NEXT_SPAN_NAME]: spanName }, | ||
| spanName, | ||
| ); | ||
|
|
||
| enhanceMiddlewareRootSpan(span); | ||
|
|
||
| expect(getName()).toBe(expected); | ||
| }); | ||
|
|
||
| it('strips query and fragment from non-method-prefixed middleware names', () => { | ||
| const { span, getName } = makeSpan( | ||
| { [ATTR_NEXT_SPAN_TYPE]: 'Middleware.execute', [ATTR_NEXT_SPAN_NAME]: '/api/foo?token=abc#section' }, | ||
| '/api/foo?token=abc#section', | ||
| ); | ||
|
|
||
| enhanceMiddlewareRootSpan(span); | ||
|
|
||
| expect(getName()).toBe('/api/foo'); | ||
| }); | ||
|
|
||
| it('does not collapse names that do not match the middleware-method prefix', () => { | ||
| // CONNECT and TRACE are not in the regex - they fall through to query/fragment stripping | ||
| const { span, getName } = makeSpan( | ||
| { [ATTR_NEXT_SPAN_TYPE]: 'Middleware.execute', [ATTR_NEXT_SPAN_NAME]: 'middleware CONNECT /foo?bar=1' }, | ||
| 'middleware CONNECT /foo?bar=1', | ||
| ); | ||
|
|
||
| enhanceMiddlewareRootSpan(span); | ||
|
|
||
| expect(getName()).toBe('middleware CONNECT /foo'); | ||
| }); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing integration or E2E test for feat PRLow Severity This Additional Locations (1)Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit 7334e14. Configure here. |
||


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.
Feat PR missing integration or E2E test
Low Severity
This is a
featPR that migrates edge event processors to span-first APIs, including a newignoreSpansconfiguration and a newprocessSegmentSpanhook. The diff only includes unit tests (enhanceMiddlewareRootSpan.test.tsandedgeSdk.test.ts). Per the project review rules, feat PRs need at least one integration or E2E test to verify the end-to-end behavior of the new span-first flow, especially for the streamed-span path which is entirely new functionality.Additional Locations (1)
packages/nextjs/src/edge/enhanceMiddlewareRootSpan.ts#L1-L41Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 04e5262. Configure here.