diff --git a/packages/opentelemetry/src/utils/getRequestSpanData.ts b/packages/opentelemetry/src/utils/getRequestSpanData.ts index cc27aadf7a78..427a44191fbb 100644 --- a/packages/opentelemetry/src/utils/getRequestSpanData.ts +++ b/packages/opentelemetry/src/utils/getRequestSpanData.ts @@ -6,7 +6,7 @@ import { SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_URL, } from '@opentelemetry/semantic-conventions'; -import { getSanitizedUrlString, parseUrl } from '@sentry/core'; +import { parseStringToURLObject, getSanitizedUrlStringFromUrlObject } from '@sentry/core'; import type { SanitizedRequestData } from '@sentry/core'; import { spanHasAttributes } from './spanTypes'; @@ -40,15 +40,15 @@ export function getRequestSpanData(span: Span | ReadableSpan): Partial = {}; - if (url) { - data.url = url; + data.url = isURLObjectRelative(parsedUrl) ? parsedUrl.pathname : parsedUrl.toString(); + if (parsedUrl.search) { + data['http.query'] = parsedUrl.search; } - if (query) { - data['http.query'] = query; - } - if (fragment) { - data['http.fragment'] = fragment; + if (parsedUrl.hash) { + data['http.fragment'] = parsedUrl.hash; } // If the span kind is neither client nor server, we use the original name @@ -234,48 +232,31 @@ function getGraphqlOperationNamesFromAttribute(attr: AttributeValue): string { } /** Exported for tests only */ -export function getSanitizedUrl( +export function getParsedUrl( attributes: Attributes, - kind: SpanKind, -): { - url: string | undefined; - urlPath: string | undefined; - query: string | undefined; - fragment: string | undefined; - hasRoute: boolean; -} { +): [parsedUrl: ReturnType, httpRoute: string | undefined] { // This is the relative path of the URL, e.g. /sub // eslint-disable-next-line deprecation/deprecation - const httpTarget = attributes[SEMATTRS_HTTP_TARGET]; + const possibleRelativeUrl = attributes[SEMATTRS_HTTP_TARGET]; // This is the full URL, including host & query params etc., e.g. https://example.com/sub?foo=bar // eslint-disable-next-line deprecation/deprecation - const httpUrl = attributes[SEMATTRS_HTTP_URL] || attributes[ATTR_URL_FULL]; + const possibleFullUrl = attributes[SEMATTRS_HTTP_URL] || attributes[ATTR_URL_FULL]; // This is the normalized route name - may not always be available! - const httpRoute = attributes[ATTR_HTTP_ROUTE]; - - const parsedUrl = typeof httpUrl === 'string' ? parseUrl(httpUrl) : undefined; - const url = parsedUrl ? getSanitizedUrlString(parsedUrl) : undefined; - const query = parsedUrl?.search || undefined; - const fragment = parsedUrl?.hash || undefined; + const httpRoute = attributes[ATTR_HTTP_ROUTE] as string | undefined; - if (typeof httpRoute === 'string') { - return { urlPath: httpRoute, url, query, fragment, hasRoute: true }; - } - - if (kind === SpanKind.SERVER && typeof httpTarget === 'string') { - return { urlPath: stripUrlQueryAndFragment(httpTarget), url, query, fragment, hasRoute: false }; - } + const parsedHttpUrl = typeof possibleFullUrl === 'string' ? parseStringToURLObject(possibleFullUrl) : undefined; + const parsedHttpTarget = + typeof possibleRelativeUrl === 'string' ? parseStringToURLObject(possibleRelativeUrl) : undefined; - if (parsedUrl) { - return { urlPath: url, url, query, fragment, hasRoute: false }; + if (parsedHttpUrl) { + return [parsedHttpUrl, httpRoute]; } - // fall back to target even for client spans, if no URL is present - if (typeof httpTarget === 'string') { - return { urlPath: stripUrlQueryAndFragment(httpTarget), url, query, fragment, hasRoute: false }; + if (parsedHttpTarget) { + return [parsedHttpTarget, httpRoute]; } - return { urlPath: undefined, url, query, fragment, hasRoute: false }; + return [undefined, httpRoute]; } /** diff --git a/packages/opentelemetry/test/utils/getRequestSpanData.test.ts b/packages/opentelemetry/test/utils/getRequestSpanData.test.ts index b2fba5b2f2f7..56dfbd04533b 100644 --- a/packages/opentelemetry/test/utils/getRequestSpanData.test.ts +++ b/packages/opentelemetry/test/utils/getRequestSpanData.test.ts @@ -42,7 +42,7 @@ describe('getRequestSpanData', () => { const data = getRequestSpanData(span); expect(data).toEqual({ - url: 'http://example.com', + url: 'http://example.com/', 'http.method': 'GET', 'http.query': '?foo=bar', 'http.fragment': '#baz', @@ -58,7 +58,7 @@ describe('getRequestSpanData', () => { const data = getRequestSpanData(span); expect(data).toEqual({ - url: 'http://example.com', + url: 'http://example.com/', 'http.method': 'GET', }); }); diff --git a/packages/opentelemetry/test/utils/parseSpanDescription.test.ts b/packages/opentelemetry/test/utils/parseSpanDescription.test.ts index 61e7ba7aada9..fdcc92a56bd1 100644 --- a/packages/opentelemetry/test/utils/parseSpanDescription.test.ts +++ b/packages/opentelemetry/test/utils/parseSpanDescription.test.ts @@ -6,9 +6,7 @@ import { SEMATTRS_DB_STATEMENT, SEMATTRS_DB_SYSTEM, SEMATTRS_FAAS_TRIGGER, - SEMATTRS_HTTP_HOST, SEMATTRS_HTTP_METHOD, - SEMATTRS_HTTP_STATUS_CODE, SEMATTRS_HTTP_TARGET, SEMATTRS_HTTP_URL, SEMATTRS_MESSAGING_SYSTEM, @@ -19,7 +17,6 @@ import { describe, expect, it } from 'vitest'; import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; import { descriptionForHttpMethod, - getSanitizedUrl, getUserUpdatedNameAndSource, parseSpanDescription, } from '../../src/utils/parseSpanDescription'; @@ -390,7 +387,7 @@ describe('descriptionForHttpMethod', () => { SpanKind.SERVER, { op: 'http.server', - description: 'POST /my-path', + description: 'POST https://www.example.com/my-path', data: { url: 'https://www.example.com/my-path', }, @@ -507,154 +504,6 @@ describe('descriptionForHttpMethod', () => { }); }); -describe('getSanitizedUrl', () => { - it.each([ - [ - 'works without attributes', - {}, - SpanKind.CLIENT, - { - urlPath: undefined, - url: undefined, - fragment: undefined, - query: undefined, - hasRoute: false, - }, - ], - [ - 'uses url without query for client request', - { - [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_TARGET]: '/?what=true', - [SEMATTRS_HTTP_HOST]: 'example.com:80', - [SEMATTRS_HTTP_STATUS_CODE]: 200, - }, - SpanKind.CLIENT, - { - urlPath: 'http://example.com/', - url: 'http://example.com/', - fragment: undefined, - query: '?what=true', - hasRoute: false, - }, - ], - [ - 'uses url without hash for client request', - { - [SEMATTRS_HTTP_URL]: 'http://example.com/sub#hash', - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_TARGET]: '/sub#hash', - [SEMATTRS_HTTP_HOST]: 'example.com:80', - [SEMATTRS_HTTP_STATUS_CODE]: 200, - }, - SpanKind.CLIENT, - { - urlPath: 'http://example.com/sub', - url: 'http://example.com/sub', - fragment: '#hash', - query: undefined, - hasRoute: false, - }, - ], - [ - 'uses route if available for client request', - { - [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_TARGET]: '/?what=true', - [ATTR_HTTP_ROUTE]: '/my-route', - [SEMATTRS_HTTP_HOST]: 'example.com:80', - [SEMATTRS_HTTP_STATUS_CODE]: 200, - }, - SpanKind.CLIENT, - { - urlPath: '/my-route', - url: 'http://example.com/', - fragment: undefined, - query: '?what=true', - hasRoute: true, - }, - ], - [ - 'falls back to target for client request if url not available', - { - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_TARGET]: '/?what=true', - [SEMATTRS_HTTP_HOST]: 'example.com:80', - [SEMATTRS_HTTP_STATUS_CODE]: 200, - }, - SpanKind.CLIENT, - { - urlPath: '/', - url: undefined, - fragment: undefined, - query: undefined, - hasRoute: false, - }, - ], - [ - 'uses target without query for server request', - { - [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_TARGET]: '/?what=true', - [SEMATTRS_HTTP_HOST]: 'example.com:80', - [SEMATTRS_HTTP_STATUS_CODE]: 200, - }, - SpanKind.SERVER, - { - urlPath: '/', - url: 'http://example.com/', - fragment: undefined, - query: '?what=true', - hasRoute: false, - }, - ], - [ - 'uses target without hash for server request', - { - [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_TARGET]: '/sub#hash', - [SEMATTRS_HTTP_HOST]: 'example.com:80', - [SEMATTRS_HTTP_STATUS_CODE]: 200, - }, - SpanKind.SERVER, - { - urlPath: '/sub', - url: 'http://example.com/', - fragment: undefined, - query: '?what=true', - hasRoute: false, - }, - ], - [ - 'uses route for server request if available', - { - [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_TARGET]: '/?what=true', - [ATTR_HTTP_ROUTE]: '/my-route', - [SEMATTRS_HTTP_HOST]: 'example.com:80', - [SEMATTRS_HTTP_STATUS_CODE]: 200, - }, - SpanKind.SERVER, - { - urlPath: '/my-route', - url: 'http://example.com/', - fragment: undefined, - query: '?what=true', - hasRoute: true, - }, - ], - ])('%s', (_, attributes, kind, expected) => { - const actual = getSanitizedUrl(attributes, kind); - - expect(actual).toEqual(expected); - }); -}); - describe('getUserUpdatedNameAndSource', () => { it('returns param name if `SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME` attribute is not set', () => { expect(getUserUpdatedNameAndSource('base name', {})).toEqual({ description: 'base name', source: 'custom' });