Skip to content

Commit

Permalink
fix(node): Set transactionName for unsampled spans in httpIntegration (
Browse files Browse the repository at this point in the history
…#12071)

We noticed that in http integration, we were only setting
`transactionName` when we had a sampled span. We can actually set this
based on the request object we get either way, making this more robust
for error-only mode.
  • Loading branch information
mydea authored May 16, 2024
1 parent 4003f7e commit c594d6a
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
Original file line number Diff line number Diff line change
Expand Up @@ -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'));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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: {
Expand Down
9 changes: 2 additions & 7 deletions packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit c594d6a

Please sign in to comment.