From f116ef146dc97dcf6373884c0443563cc33559e3 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 19 Apr 2024 10:17:02 +0000 Subject: [PATCH 1/2] ref(core): Don't start transaction for trpc middleware --- .../node-express-app/tests/server.test.ts | 97 +------------------ .../node-express-app/tests/trpc.test.ts | 97 +++++++++++++++++++ packages/core/src/trpc.ts | 1 - 3 files changed, 98 insertions(+), 97 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/node-express-app/tests/trpc.test.ts diff --git a/dev-packages/e2e-tests/test-applications/node-express-app/tests/server.test.ts b/dev-packages/e2e-tests/test-applications/node-express-app/tests/server.test.ts index b08dca4dd299..2fd47da7f35f 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-app/tests/server.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-app/tests/server.test.ts @@ -1,8 +1,6 @@ import { expect, test } from '@playwright/test'; -import { waitForError, waitForTransaction } from '@sentry-internal/event-proxy-server'; -import { createTRPCProxyClient, httpBatchLink } from '@trpc/client'; +import { waitForError } from '@sentry-internal/event-proxy-server'; import axios, { AxiosError, AxiosResponse } from 'axios'; -import type { AppRouter } from '../src/app'; const authToken = process.env.E2E_TEST_AUTH_TOKEN; const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; @@ -132,96 +130,3 @@ test('Should record uncaught exceptions with local variable', async ({ baseURL } expect(frames[frames.length - 1].vars?.randomVariableToRecord).toBeDefined(); }); - -test('Should record transaction for trpc query', async ({ baseURL }) => { - const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { - return transactionEvent.transaction === 'trpc/getSomething'; - }); - - const trpcClient = createTRPCProxyClient({ - links: [ - httpBatchLink({ - url: `${baseURL}/trpc`, - }), - ], - }); - - await trpcClient.getSomething.query('foobar'); - - await expect(transactionEventPromise).resolves.toBeDefined(); - const transaction = await transactionEventPromise; - - expect(transaction.contexts?.trpc).toMatchObject({ - procedure_type: 'query', - input: 'foobar', - }); -}); - -test('Should record transaction for trpc mutation', async ({ baseURL }) => { - const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { - return transactionEvent.transaction === 'trpc/createSomething'; - }); - - const trpcClient = createTRPCProxyClient({ - links: [ - httpBatchLink({ - url: `${baseURL}/trpc`, - }), - ], - }); - - await trpcClient.createSomething.mutate(); - - await expect(transactionEventPromise).resolves.toBeDefined(); - const transaction = await transactionEventPromise; - - expect(transaction.contexts?.trpc).toMatchObject({ - procedure_type: 'mutation', - }); -}); - -test('Should record transaction and error for a crashing trpc handler', async ({ baseURL }) => { - const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { - return transactionEvent.transaction === 'trpc/crashSomething'; - }); - - const errorEventPromise = waitForError('node-express-app', errorEvent => { - return !!errorEvent?.exception?.values?.some(exception => exception.value?.includes('I crashed in a trpc handler')); - }); - - const trpcClient = createTRPCProxyClient({ - links: [ - httpBatchLink({ - url: `${baseURL}/trpc`, - }), - ], - }); - - await expect(trpcClient.crashSomething.mutate()).rejects.toBeDefined(); - - await expect(transactionEventPromise).resolves.toBeDefined(); - await expect(errorEventPromise).resolves.toBeDefined(); -}); - -test('Should record transaction and error for a trpc handler that returns a status code', async ({ baseURL }) => { - const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { - return transactionEvent.transaction === 'trpc/dontFindSomething'; - }); - - const errorEventPromise = waitForError('node-express-app', errorEvent => { - return !!errorEvent?.exception?.values?.some(exception => exception.value?.includes('Page not found')); - }); - - const trpcClient = createTRPCProxyClient({ - links: [ - httpBatchLink({ - url: `${baseURL}/trpc`, - }), - ], - }); - - await expect(trpcClient.dontFindSomething.mutate()).rejects.toBeDefined(); - - await expect(transactionEventPromise).resolves.toBeDefined(); - await expect(errorEventPromise).resolves.toBeDefined(); -}); diff --git a/dev-packages/e2e-tests/test-applications/node-express-app/tests/trpc.test.ts b/dev-packages/e2e-tests/test-applications/node-express-app/tests/trpc.test.ts new file mode 100644 index 000000000000..96cde41864e1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-express-app/tests/trpc.test.ts @@ -0,0 +1,97 @@ +import { expect, test } from '@playwright/test'; +import { waitForError, waitForTransaction } from '@sentry-internal/event-proxy-server'; +import { createTRPCProxyClient, httpBatchLink } from '@trpc/client'; +import type { AppRouter } from '../src/app'; + +test('Should record transaction for trpc query', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { + return transactionEvent.transaction === 'trpc/getSomething'; + }); + + const trpcClient = createTRPCProxyClient({ + links: [ + httpBatchLink({ + url: `${baseURL}/trpc`, + }), + ], + }); + + await trpcClient.getSomething.query('foobar'); + + await expect(transactionEventPromise).resolves.toBeDefined(); + const transaction = await transactionEventPromise; + + expect(transaction.contexts?.trpc).toMatchObject({ + procedure_type: 'query', + input: 'foobar', + }); +}); + +test('Should record transaction for trpc mutation', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { + return transactionEvent.transaction === 'trpc/createSomething'; + }); + + const trpcClient = createTRPCProxyClient({ + links: [ + httpBatchLink({ + url: `${baseURL}/trpc`, + }), + ], + }); + + await trpcClient.createSomething.mutate(); + + await expect(transactionEventPromise).resolves.toBeDefined(); + const transaction = await transactionEventPromise; + + expect(transaction.contexts?.trpc).toMatchObject({ + procedure_type: 'mutation', + }); +}); + +test('Should record transaction and error for a crashing trpc handler', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { + return transactionEvent.transaction === 'trpc/crashSomething'; + }); + + const errorEventPromise = waitForError('node-express-app', errorEvent => { + return !!errorEvent?.exception?.values?.some(exception => exception.value?.includes('I crashed in a trpc handler')); + }); + + const trpcClient = createTRPCProxyClient({ + links: [ + httpBatchLink({ + url: `${baseURL}/trpc`, + }), + ], + }); + + await expect(trpcClient.crashSomething.mutate()).rejects.toBeDefined(); + + await expect(transactionEventPromise).resolves.toBeDefined(); + await expect(errorEventPromise).resolves.toBeDefined(); +}); + +test('Should record transaction and error for a trpc handler that returns a status code', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { + return transactionEvent.transaction === 'trpc/dontFindSomething'; + }); + + const errorEventPromise = waitForError('node-express-app', errorEvent => { + return !!errorEvent?.exception?.values?.some(exception => exception.value?.includes('Page not found')); + }); + + const trpcClient = createTRPCProxyClient({ + links: [ + httpBatchLink({ + url: `${baseURL}/trpc`, + }), + ], + }); + + await expect(trpcClient.dontFindSomething.mutate()).rejects.toBeDefined(); + + await expect(transactionEventPromise).resolves.toBeDefined(); + await expect(errorEventPromise).resolves.toBeDefined(); +}); diff --git a/packages/core/src/trpc.ts b/packages/core/src/trpc.ts index f2cc6656d62d..1320f0ff15bc 100644 --- a/packages/core/src/trpc.ts +++ b/packages/core/src/trpc.ts @@ -55,7 +55,6 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) { { name: `trpc/${path}`, op: 'rpc.server', - forceTransaction: true, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.rpc.trpc', From d02b3b0fdf215f49ce8688aef61ab29961726030 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 19 Apr 2024 11:12:01 +0000 Subject: [PATCH 2/2] update tests --- .../node-express-app/tests/trpc.test.ts | 42 ++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-app/tests/trpc.test.ts b/dev-packages/e2e-tests/test-applications/node-express-app/tests/trpc.test.ts index 96cde41864e1..4b287946d86d 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-app/tests/trpc.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-app/tests/trpc.test.ts @@ -3,9 +3,12 @@ import { waitForError, waitForTransaction } from '@sentry-internal/event-proxy-s import { createTRPCProxyClient, httpBatchLink } from '@trpc/client'; import type { AppRouter } from '../src/app'; -test('Should record transaction for trpc query', async ({ baseURL }) => { +test('Should record span for trpc query', async ({ baseURL }) => { const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { - return transactionEvent.transaction === 'trpc/getSomething'; + return ( + transactionEvent.transaction === 'GET /trpc' && + !!transactionEvent.spans?.find(span => span.description === 'trpc/getSomething') + ); }); const trpcClient = createTRPCProxyClient({ @@ -21,6 +24,16 @@ test('Should record transaction for trpc query', async ({ baseURL }) => { await expect(transactionEventPromise).resolves.toBeDefined(); const transaction = await transactionEventPromise; + expect(transaction.spans).toContainEqual( + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.op': 'rpc.server', + 'sentry.origin': 'auto.rpc.trpc', + }), + description: `trpc/getSomething`, + }), + ); + expect(transaction.contexts?.trpc).toMatchObject({ procedure_type: 'query', input: 'foobar', @@ -29,7 +42,10 @@ test('Should record transaction for trpc query', async ({ baseURL }) => { test('Should record transaction for trpc mutation', async ({ baseURL }) => { const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { - return transactionEvent.transaction === 'trpc/createSomething'; + return ( + transactionEvent.transaction === 'POST /trpc' && + !!transactionEvent.spans?.find(span => span.description === 'trpc/createSomething') + ); }); const trpcClient = createTRPCProxyClient({ @@ -45,6 +61,16 @@ test('Should record transaction for trpc mutation', async ({ baseURL }) => { await expect(transactionEventPromise).resolves.toBeDefined(); const transaction = await transactionEventPromise; + expect(transaction.spans).toContainEqual( + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.op': 'rpc.server', + 'sentry.origin': 'auto.rpc.trpc', + }), + description: `trpc/createSomething`, + }), + ); + expect(transaction.contexts?.trpc).toMatchObject({ procedure_type: 'mutation', }); @@ -52,7 +78,10 @@ test('Should record transaction for trpc mutation', async ({ baseURL }) => { test('Should record transaction and error for a crashing trpc handler', async ({ baseURL }) => { const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { - return transactionEvent.transaction === 'trpc/crashSomething'; + return ( + transactionEvent.transaction === 'POST /trpc' && + !!transactionEvent.spans?.find(span => span.description === 'trpc/crashSomething') + ); }); const errorEventPromise = waitForError('node-express-app', errorEvent => { @@ -75,7 +104,10 @@ test('Should record transaction and error for a crashing trpc handler', async ({ test('Should record transaction and error for a trpc handler that returns a status code', async ({ baseURL }) => { const transactionEventPromise = waitForTransaction('node-express-app', transactionEvent => { - return transactionEvent.transaction === 'trpc/dontFindSomething'; + return ( + transactionEvent.transaction === 'POST /trpc' && + !!transactionEvent.spans?.find(span => span.description === 'trpc/dontFindSomething') + ); }); const errorEventPromise = waitForError('node-express-app', errorEvent => {