-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(tanstackstart-react): Show readable server function names in traces #21190
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 |
|---|---|---|
|
|
@@ -25,14 +25,16 @@ test('Sends a server function transaction with span from wrapFetchWithSentry', a | |
| expect(transactionEvent?.spans).toHaveLength(1); | ||
| expect(transactionEvent?.spans).toEqual([ | ||
| expect.objectContaining({ | ||
| description: expect.stringContaining('GET /_serverFn/'), | ||
| description: 'GET /_serverFn/testLog', | ||
| op: 'function.tanstackstart', | ||
| origin: 'auto.function.tanstackstart.server', | ||
| data: expect.objectContaining({ | ||
| data: { | ||
| 'sentry.op': 'function.tanstackstart', | ||
| 'sentry.origin': 'auto.function.tanstackstart.server', | ||
| 'tanstackstart.function.hash.sha256': expect.any(String), | ||
| }), | ||
| 'sentry.source': 'route', | ||
| 'tanstackstart.function.id': expect.any(String), | ||
| 'tanstackstart.function.filename': 'src/routes/test-serverFn.tsx', | ||
| }, | ||
| }), | ||
| ]); | ||
| }); | ||
|
|
@@ -62,14 +64,16 @@ test('Sends a server function transaction for a nested server function with manu | |
| expect(transactionEvent?.spans).toEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| description: expect.stringContaining('GET /_serverFn/'), | ||
| description: 'GET /_serverFn/testNestedLog', | ||
| op: 'function.tanstackstart', | ||
| origin: 'auto.function.tanstackstart.server', | ||
| data: expect.objectContaining({ | ||
| data: { | ||
| 'sentry.op': 'function.tanstackstart', | ||
| 'sentry.origin': 'auto.function.tanstackstart.server', | ||
| 'tanstackstart.function.hash.sha256': expect.any(String), | ||
| }), | ||
| 'sentry.source': 'route', | ||
|
Member
Author
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. Using this because from what I can see in my experiments the name here never has parameters so we don't need relay-side route parametrization |
||
| 'tanstackstart.function.id': expect.any(String), | ||
| 'tanstackstart.function.filename': 'src/routes/test-serverFn.tsx', | ||
| }, | ||
| }), | ||
| expect.objectContaining({ | ||
| description: 'testNestedLog', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,20 @@ | ||
| import { addNonEnumerableProperty, captureException } from '@sentry/core'; | ||
| import { | ||
| SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, | ||
| addNonEnumerableProperty, | ||
| captureException, | ||
| getActiveSpan, | ||
| spanToJSON, | ||
| updateSpanName, | ||
| } from '@sentry/core'; | ||
| import type { TanStackMiddlewareBase } from '../common/types'; | ||
| import { SENTRY_INTERNAL } from './middleware'; | ||
|
|
||
| type ServerFnMeta = { | ||
| id?: string; | ||
| name?: string; | ||
| filename?: string; | ||
| }; | ||
|
|
||
| function createSentryMiddlewareHandler(mechanismType: string) { | ||
| return async function sentryMiddlewareHandler({ next }: { next: () => Promise<unknown> }): Promise<unknown> { | ||
| try { | ||
|
|
@@ -15,6 +28,41 @@ function createSentryMiddlewareHandler(mechanismType: string) { | |
| }; | ||
| } | ||
|
|
||
| function createSentryFunctionMiddlewareHandler(mechanismType: string) { | ||
| return async function sentryFunctionMiddlewareHandler({ | ||
| next, | ||
| serverFnMeta, | ||
| }: { | ||
| next: () => Promise<unknown>; | ||
| serverFnMeta?: ServerFnMeta; | ||
| }): Promise<unknown> { | ||
| const activeSpan = getActiveSpan(); | ||
| const spanData = activeSpan ? spanToJSON(activeSpan) : undefined; | ||
| if (activeSpan && spanData?.op === 'function.tanstackstart') { | ||
| if (serverFnMeta?.name) { | ||
| const method = spanData.description?.split(' ')[0] || 'GET'; | ||
| updateSpanName(activeSpan, `${method} /_serverFn/${serverFnMeta.name}`); | ||
| activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); | ||
| } | ||
| if (serverFnMeta?.id) { | ||
| activeSpan.setAttribute('tanstackstart.function.id', serverFnMeta.id); | ||
| } | ||
| if (serverFnMeta?.filename) { | ||
| activeSpan.setAttribute('tanstackstart.function.filename', serverFnMeta.filename); | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| return await next(); | ||
| } catch (e) { | ||
| captureException(e, { | ||
| mechanism: { type: mechanismType, handled: false }, | ||
| }); | ||
| throw e; | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Global request middleware that captures errors from API route requests. | ||
| * Should be added as the first entry in the `requestMiddleware` array of `createStart()`. | ||
|
|
@@ -36,8 +84,9 @@ export const sentryGlobalFunctionMiddleware: TanStackMiddlewareBase = { | |
| '~types': undefined, | ||
|
|
||
| options: { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| server: createSentryMiddlewareHandler('auto.middleware.tanstackstart.server_function') as (...args: any[]) => any, | ||
| server: createSentryFunctionMiddlewareHandler('auto.middleware.tanstackstart.server_function') as ( | ||
| ...args: any[] // eslint-disable-line @typescript-eslint/no-explicit-any | ||
| ) => any, // eslint-disable-line @typescript-eslint/no-explicit-any | ||
|
Comment on lines
+87
to
+89
Member
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. Q: Why is the
Member
Author
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.
|
||
| }, | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,41 +1,16 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { extractServerFunctionSha256 } from '../../src/server/utils'; | ||
| import { getMiddlewareSpanOptions } from '../../src/server/utils'; | ||
|
|
||
| describe('extractServerFunctionSha256', () => { | ||
| it('extracts SHA256 hash from valid server function pathname', () => { | ||
| const pathname = '/_serverFn/1ac31c23f613ec7e58631cf789642e2feb86c58e3128324cf00d746474a044bf'; | ||
| const result = extractServerFunctionSha256(pathname); | ||
| expect(result).toBe('1ac31c23f613ec7e58631cf789642e2feb86c58e3128324cf00d746474a044bf'); | ||
| }); | ||
|
|
||
| it('extracts SHA256 hash from valid server function pathname that is a subpath', () => { | ||
| const pathname = '/api/_serverFn/1ac31c23f613ec7e58631cf789642e2feb86c58e3128324cf00d746474a044bf'; | ||
| const result = extractServerFunctionSha256(pathname); | ||
| expect(result).toBe('1ac31c23f613ec7e58631cf789642e2feb86c58e3128324cf00d746474a044bf'); | ||
| }); | ||
|
|
||
| it('extracts SHA256 hash from valid server function pathname with query parameters', () => { | ||
| const pathname = '/_serverFn/1ac31c23f613ec7e58631cf789642e2feb86c58e3128324cf00d746474a044bf?param=value'; | ||
| const result = extractServerFunctionSha256(pathname); | ||
| expect(result).toBe('1ac31c23f613ec7e58631cf789642e2feb86c58e3128324cf00d746474a044bf'); | ||
| }); | ||
|
|
||
| it('extracts SHA256 hash with uppercase hex characters', () => { | ||
| const pathname = '/_serverFn/1AC31C23F613EC7E58631CF789642E2FEB86C58E3128324CF00D746474A044BF'; | ||
| const result = extractServerFunctionSha256(pathname); | ||
| expect(result).toBe('1AC31C23F613EC7E58631CF789642E2FEB86C58E3128324CF00D746474A044BF'); | ||
| }); | ||
|
|
||
| it('returns unknown for pathname without server function pattern', () => { | ||
| const pathname = '/api/users/123'; | ||
| const result = extractServerFunctionSha256(pathname); | ||
| expect(result).toBe('unknown'); | ||
| }); | ||
|
|
||
| it('returns unknown for pathname with incomplete hash', () => { | ||
| // Hash is too short (only 32 chars instead of 64) | ||
| const pathname = '/_serverFn/1ac31c23f613ec7e58631cf789642e2f'; | ||
| const result = extractServerFunctionSha256(pathname); | ||
| expect(result).toBe('unknown'); | ||
| describe('getMiddlewareSpanOptions', () => { | ||
| it('returns correct span options', () => { | ||
| const options = getMiddlewareSpanOptions('testMiddleware'); | ||
| expect(options).toEqual({ | ||
| op: 'middleware.tanstackstart', | ||
| name: 'testMiddleware', | ||
| attributes: { | ||
| 'sentry.op': 'middleware.tanstackstart', | ||
| 'sentry.origin': 'auto.middleware.tanstackstart', | ||
| }, | ||
| }); | ||
| }); | ||
| }); |
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.
Keep in mind that
.objectContainingmight ignore fields that matter as it does not to an equality check (is successful when one item is missing). You can either dotoEqualortoBeon each individual item you want to check.Same for all other checks in this PR.
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.
yes that's why I removed the
.objectContaining, so this should now do an exact match