fix(otel): Use HTTP_URL attribute for client requests

packages/opentelemetry-node/src/spanprocessor.ts

@@ -7,9 +7,9 @@ import type { DynamicSamplingContext, Span as SentrySpan, TraceparentData, Trans
 import { isString, logger } from '@sentry/utils';
 
 import { SENTRY_DYNAMIC_SAMPLING_CONTEXT_KEY, SENTRY_TRACE_PARENT_CONTEXT_KEY } from './constants';
-import { isSentryRequestSpan } from './utils/is-sentry-request';
-import { mapOtelStatus } from './utils/map-otel-status';
-import { parseSpanDescription } from './utils/parse-otel-span-description';
+import { isSentryRequestSpan } from './utils/isSentryRequest';
+import { mapOtelStatus } from './utils/mapOtelStatus';
+import { parseSpanDescription } from './utils/parseOtelSpanDescription';
 
 export const SENTRY_SPAN_PROCESSOR_MAP: Map<SentrySpan['spanId'], SentrySpan> = new Map<
   SentrySpan['spanId'],

packages/opentelemetry-node/src/utils/parse-otel-span-description.ts → packages/opentelemetry-node/src/utils/parseOtelSpanDescription.ts

@@ -1,8 +1,9 @@
-import type { AttributeValue } from '@opentelemetry/api';
+import type { Attributes, AttributeValue } from '@opentelemetry/api';
 import { SpanKind } from '@opentelemetry/api';
 import type { Span as OtelSpan } from '@opentelemetry/sdk-trace-base';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import type { TransactionSource } from '@sentry/types';
+import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils';
 
 interface SpanDescription {
   op: string | undefined;
@@ -87,21 +88,48 @@ function descriptionForHttpMethod(otelSpan: OtelSpan, httpMethod: AttributeValue
       break;
   }
 
-  const httpTarget = attributes[SemanticAttributes.HTTP_TARGET];
   const httpRoute = attributes[SemanticAttributes.HTTP_ROUTE];
+  const url = getSanitizedUrl(attributes, kind);
 
-  // Ex. /api/users
-  const httpPath = httpRoute || httpTarget;
-
-  if (!httpPath) {
+  if (!url) {
     return { op: opParts.join('.'), description: name, source: 'custom' };
   }
 
   // Ex. description="GET /api/users".
-  const description = `${httpMethod} ${httpPath}`;
+  const description = `${httpMethod} ${url}`;
 
   // If `httpPath` is a root path, then we can categorize the transaction source as route.
-  const source: TransactionSource = httpRoute || httpPath === '/' ? 'route' : 'url';
+  const source: TransactionSource = httpRoute || url === '/' ? 'route' : 'url';
 
   return { op: opParts.join('.'), description, source };
 }
+
+/** Exported for tests only */
+export function getSanitizedUrl(attributes: Attributes, kind: SpanKind): string | undefined {
+  // This is the relative path of the URL, e.g. /sub
+  const httpTarget = attributes[SemanticAttributes.HTTP_TARGET];
+  // This is the full URL, including host & query params etc., e.g. https://example.com/sub?foo=bar
+  const httpUrl = attributes[SemanticAttributes.HTTP_URL];
+  // This is the normalized route name - may not always be available!
+  const httpRoute = attributes[SemanticAttributes.HTTP_ROUTE];
+
+  if (typeof httpRoute === 'string') {
+    return httpRoute;
+  }
+
+  if (kind === SpanKind.SERVER && typeof httpTarget === 'string') {
+    return stripUrlQueryAndFragment(httpTarget);
+  }
+
+  if (typeof httpUrl === 'string') {
+    const url = parseUrl(httpUrl);
+    return getSanitizedUrlString(url);
+  }
+
+  // fall back to target even for client spans, if no URL is present
+  if (typeof httpTarget === 'string') {
+    return stripUrlQueryAndFragment(httpTarget);
+  }
+
+  return undefined;
+}

packages/opentelemetry-node/test/utils/parseOtelSpanDescription.test.ts

@@ -0,0 +1,99 @@
+import { SpanKind } from '@opentelemetry/api';
+import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
+
+import { getSanitizedUrl } from '../../src/utils/parseOtelSpanDescription';
+
+describe('getSanitizedUrl', () => {
+  it.each([
+    ['works without attributes', {}, SpanKind.CLIENT, undefined],
+    [
+      'uses url without query for client request',
+      {
+        [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true',
+        [SemanticAttributes.HTTP_METHOD]: 'GET',
+        [SemanticAttributes.HTTP_TARGET]: '/?what=true',
+        [SemanticAttributes.HTTP_HOST]: 'example.com:80',
+        [SemanticAttributes.HTTP_STATUS_CODE]: 200,
+      },
+      SpanKind.CLIENT,
+      'http://example.com/',
+    ],
+    [
+      'uses url without hash for client request',
+      {
+        [SemanticAttributes.HTTP_URL]: 'http://example.com/sub#hash',
+        [SemanticAttributes.HTTP_METHOD]: 'GET',
+        [SemanticAttributes.HTTP_TARGET]: '/sub#hash',
+        [SemanticAttributes.HTTP_HOST]: 'example.com:80',
+        [SemanticAttributes.HTTP_STATUS_CODE]: 200,
+      },
+      SpanKind.CLIENT,
+      'http://example.com/sub',
+    ],
+    [
+      'uses route if available for client request',
+      {
+        [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true',
+        [SemanticAttributes.HTTP_METHOD]: 'GET',
+        [SemanticAttributes.HTTP_TARGET]: '/?what=true',
+        [SemanticAttributes.HTTP_ROUTE]: '/my-route',
+        [SemanticAttributes.HTTP_HOST]: 'example.com:80',
+        [SemanticAttributes.HTTP_STATUS_CODE]: 200,
+      },
+      SpanKind.CLIENT,
+      '/my-route',
+    ],
+    [
+      'falls back to target for client request if url not available',
+      {
+        [SemanticAttributes.HTTP_METHOD]: 'GET',
+        [SemanticAttributes.HTTP_TARGET]: '/?what=true',
+        [SemanticAttributes.HTTP_HOST]: 'example.com:80',
+        [SemanticAttributes.HTTP_STATUS_CODE]: 200,
+      },
+      SpanKind.CLIENT,
+      '/',
+    ],
+    [
+      'uses target without query for server request',
+      {
+        [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true',
+        [SemanticAttributes.HTTP_METHOD]: 'GET',
+        [SemanticAttributes.HTTP_TARGET]: '/?what=true',
+        [SemanticAttributes.HTTP_HOST]: 'example.com:80',
+        [SemanticAttributes.HTTP_STATUS_CODE]: 200,
+      },
+      SpanKind.SERVER,
+      '/',
+    ],
+    [
+      'uses target without hash for server request',
+      {
+        [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true',
+        [SemanticAttributes.HTTP_METHOD]: 'GET',
+        [SemanticAttributes.HTTP_TARGET]: '/sub#hash',
+        [SemanticAttributes.HTTP_HOST]: 'example.com:80',
+        [SemanticAttributes.HTTP_STATUS_CODE]: 200,
+      },
+      SpanKind.SERVER,
+      '/sub',
+    ],
+    [
+      'uses route for server request if available',
+      {
+        [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true',
+        [SemanticAttributes.HTTP_METHOD]: 'GET',
+        [SemanticAttributes.HTTP_TARGET]: '/?what=true',
+        [SemanticAttributes.HTTP_ROUTE]: '/my-route',
+        [SemanticAttributes.HTTP_HOST]: 'example.com:80',
+        [SemanticAttributes.HTTP_STATUS_CODE]: 200,
+      },
+      SpanKind.SERVER,
+      '/my-route',
+    ],
+  ])('%s', (_, attributes, kind, expected) => {
+    const actual = getSanitizedUrl(attributes, kind);
+
+    expect(actual).toEqual(expected);
+  });
+});