From d05b3609ee894114be278a9dff47546e26faf225 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 5 Apr 2024 14:52:49 +0200 Subject: [PATCH 1/3] feat(hapi): Update scope `transactionName` when handling request --- .../test-applications/node-hapi-app/src/app.js | 9 +++++++++ .../node-hapi-app/tests/server.test.ts | 14 ++++++++++++++ .../suites/tracing/hapi/scenario.js | 8 ++++++++ .../suites/tracing/hapi/test.ts | 14 ++++++++++++++ .../node/src/integrations/tracing/hapi/index.ts | 6 ++++++ 5 files changed, 51 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/src/app.js b/dev-packages/e2e-tests/test-applications/node-hapi-app/src/app.js index 95564255b60f..6d6f959410dd 100644 --- a/dev-packages/e2e-tests/test-applications/node-hapi-app/src/app.js +++ b/dev-packages/e2e-tests/test-applications/node-hapi-app/src/app.js @@ -44,6 +44,15 @@ const init = async () => { }, }); + server.route({ + method: 'GET', + path: '/test-error/{id}', + handler: function (request) { + console.log('This is an error with id', request.params.id); + throw new Error(`This is an error with id ${request.params.id}`); + }, + }); + server.route({ method: 'GET', path: '/test-failure', diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/tests/server.test.ts b/dev-packages/e2e-tests/test-applications/node-hapi-app/tests/server.test.ts index d50c68ac4e72..d0c8c5d19fb7 100644 --- a/dev-packages/e2e-tests/test-applications/node-hapi-app/tests/server.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-hapi-app/tests/server.test.ts @@ -121,6 +121,20 @@ test('Sends successful transactions to Sentry', async ({ baseURL }) => { .toBe(200); }); +test('sends error with parameterized transaction name', async ({ baseURL }) => { + const errorEventPromise = waitForError('node-hapi-app', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'This is an error with id 123'; + }); + + try { + await axios.get(`${baseURL}/test-error/123`); + } catch {} + + const errorEvent = await errorEventPromise; + + expect(errorEvent?.transaction).toBe('GET /test-error/{id}'); +}); + test('Sends parameterized transactions to Sentry', async ({ baseURL }) => { const pageloadTransactionEventPromise = waitForTransaction('node-hapi-app', transactionEvent => { return ( diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js index 69443559e9a8..184dcb3b8ea1 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js @@ -35,6 +35,14 @@ const init = async () => { }, }); + server.route({ + method: 'GET', + path: '/error/{id}', + handler: (_request, _h) => { + return new Error('Sentry Test Error'); + }, + }); + server.route({ method: 'GET', path: '/boom-error', diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts index e5903c536a95..10967a6d6be2 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts @@ -56,6 +56,20 @@ describe('hapi auto-instrumentation', () => { .makeRequest('get', '/error'); }); + test('CJS - should assign parameterized transactionName to error.', done => { + createRunner(__dirname, 'scenario.js') + .expect({ + event: { + ...EXPECTED_ERROR_EVENT, + transaction: 'GET /error/{id}', + }, + }) + .ignore('transaction') + .expectError() + .start(done) + .makeRequest('get', '/error/123'); + }); + test('CJS - should handle returned Boom errors in routes.', done => { createRunner(__dirname, 'scenario.js') .expect({ diff --git a/packages/node/src/integrations/tracing/hapi/index.ts b/packages/node/src/integrations/tracing/hapi/index.ts index ac3766aaae86..f5d760efd292 100644 --- a/packages/node/src/integrations/tracing/hapi/index.ts +++ b/packages/node/src/integrations/tracing/hapi/index.ts @@ -6,6 +6,7 @@ import { captureException, defineIntegration, getActiveSpan, + getCurrentScope, getRootSpan, } from '@sentry/core'; import type { IntegrationFn } from '@sentry/types'; @@ -58,6 +59,11 @@ export const hapiErrorPlugin = { const server = serverArg as unknown as Server; server.events.on('request', (request, event) => { + const route = request.route; + if (route && route.path) { + getCurrentScope().setTransactionName(`${route.method?.toUpperCase() || 'GET'} ${route.path}`); + } + const activeSpan = getActiveSpan(); const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; From ba02f1b7a6f6238083f7fde737a7ef7933f1a86c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 9 Apr 2024 15:47:33 +0200 Subject: [PATCH 2/3] use isolationScope for consistency, guard for defaultIsolationScope --- .../node/src/integrations/tracing/hapi/index.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/node/src/integrations/tracing/hapi/index.ts b/packages/node/src/integrations/tracing/hapi/index.ts index f5d760efd292..1d3e1a5965af 100644 --- a/packages/node/src/integrations/tracing/hapi/index.ts +++ b/packages/node/src/integrations/tracing/hapi/index.ts @@ -7,9 +7,13 @@ import { defineIntegration, getActiveSpan, getCurrentScope, + getDefaultIsolationScope, + getIsolationScope, getRootSpan, } from '@sentry/core'; import type { IntegrationFn } from '@sentry/types'; +import { logger } from '@sentry/utils'; +import { DEBUG_BUILD } from '../../../debug-build'; import type { Boom, RequestEvent, ResponseObject, Server } from './types'; const _hapiIntegration = (() => { @@ -59,9 +63,14 @@ export const hapiErrorPlugin = { const server = serverArg as unknown as Server; server.events.on('request', (request, event) => { - const route = request.route; - if (route && route.path) { - getCurrentScope().setTransactionName(`${route.method?.toUpperCase() || 'GET'} ${route.path}`); + if (getIsolationScope() !== getDefaultIsolationScope()) { + const route = request.route; + if (route && route.path) { + getIsolationScope().setTransactionName(`${route.method?.toUpperCase() || 'GET'} ${route.path}`); + } + } else { + DEBUG_BUILD && + logger.warn('Isolation scope is still the default isolation scope - skipping setting transactionName'); } const activeSpan = getActiveSpan(); From c3e30c78bc7096856f864e4e4258923b240b1327 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 9 Apr 2024 17:02:27 +0200 Subject: [PATCH 3/3] lint/biome --- packages/node/src/integrations/tracing/hapi/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/hapi/index.ts b/packages/node/src/integrations/tracing/hapi/index.ts index 1d3e1a5965af..c605a6cbd551 100644 --- a/packages/node/src/integrations/tracing/hapi/index.ts +++ b/packages/node/src/integrations/tracing/hapi/index.ts @@ -6,7 +6,6 @@ import { captureException, defineIntegration, getActiveSpan, - getCurrentScope, getDefaultIsolationScope, getIsolationScope, getRootSpan,