From 966b8dfe5fb9f669ef3a140a0188c71dad5b6345 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 16 May 2024 14:27:03 +0200 Subject: [PATCH] fix(node): Set transactionName for unsampled spans in httpIntegration --- .../node-express-esm-without-loader/tests/server.test.ts | 5 ++--- .../suites/express/without-tracing/server.ts | 1 - .../suites/express/without-tracing/test.ts | 6 ++---- packages/node/src/integrations/http.ts | 9 ++------- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-esm-without-loader/tests/server.test.ts b/dev-packages/e2e-tests/test-applications/node-express-esm-without-loader/tests/server.test.ts index c2c076f13e4e..eeeb033a42df 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-esm-without-loader/tests/server.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-esm-without-loader/tests/server.test.ts @@ -29,7 +29,6 @@ test('Isolates requests', async ({ request }) => { expect(errorEvent1.tags).toEqual({ 'param-1': 'yes' }); expect(errorEvent2.tags).toEqual({ 'param-2': 'yes' }); - // Transaction is not set, since we have no expressIntegration by default - expect(errorEvent1.transaction).toBeUndefined(); - expect(errorEvent2.transaction).toBeUndefined(); + expect(errorEvent1.transaction).toBe('GET /test-params/1'); + expect(errorEvent2.transaction).toBe('GET /test-params/2'); }); diff --git a/dev-packages/node-integration-tests/suites/express/without-tracing/server.ts b/dev-packages/node-integration-tests/suites/express/without-tracing/server.ts index 6b8af4270430..2a85d39b83b8 100644 --- a/dev-packages/node-integration-tests/suites/express/without-tracing/server.ts +++ b/dev-packages/node-integration-tests/suites/express/without-tracing/server.ts @@ -18,7 +18,6 @@ app.get('/test/isolationScope/:id', (req, res) => { const id = req.params.id; Sentry.setTag('isolation-scope', 'tag'); Sentry.setTag(`isolation-scope-${id}`, id); - Sentry.setTag('isolation-scope-transactionName', `${Sentry.getIsolationScope().getScopeData().transactionName}`); Sentry.captureException(new Error('This is an exception')); diff --git a/dev-packages/node-integration-tests/suites/express/without-tracing/test.ts b/dev-packages/node-integration-tests/suites/express/without-tracing/test.ts index 85c037dc0df9..73d6a322ed28 100644 --- a/dev-packages/node-integration-tests/suites/express/without-tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/without-tracing/test.ts @@ -9,12 +9,11 @@ test('correctly applies isolation scope even without tracing', done => { .ignore('session', 'sessions') .expect({ event: { + transaction: 'GET /test/isolationScope/1', tags: { global: 'tag', 'isolation-scope': 'tag', 'isolation-scope-1': '1', - // We can't properly test non-existance of fields here, so we cast this to a string to test it here - 'isolation-scope-transactionName': 'undefined', }, // Request is correctly set request: { @@ -27,12 +26,11 @@ test('correctly applies isolation scope even without tracing', done => { }) .expect({ event: { + transaction: 'GET /test/isolationScope/2', tags: { global: 'tag', 'isolation-scope': 'tag', 'isolation-scope-2': '2', - // We can't properly test non-existance of fields here, so we cast this to a string to test it here - 'isolation-scope-transactionName': 'undefined', }, // Request is correctly set request: { diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index ce6916da4dcd..8022227c6c4e 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -12,7 +12,6 @@ import { getIsolationScope, isSentryRequestUrl, setCapturedScopesOnSpan, - spanToJSON, } from '@sentry/core'; import { getClient, getRequestSpanData, getSpanKind } from '@sentry/opentelemetry'; import type { IntegrationFn } from '@sentry/types'; @@ -121,13 +120,9 @@ const _httpIntegration = ((options: HttpOptions = {}) => { // attempt to update the scope's `transactionName` based on the request URL // Ideally, framework instrumentations coming after the HttpInstrumentation // update the transactionName once we get a parameterized route. - const attributes = spanToJSON(span).data; - if (!attributes) { - return; - } + const httpMethod = (req.method || 'GET').toUpperCase(); + const httpTarget = stripUrlQueryAndFragment(req.url || '/'); - const httpMethod = String(attributes['http.method']).toUpperCase() || 'GET'; - const httpTarget = stripUrlQueryAndFragment(String(attributes['http.target'])) || '/'; const bestEffortTransactionName = `${httpMethod} ${httpTarget}`; isolationScope.setTransactionName(bestEffortTransactionName);