From 3bcfd0a03818708e18c64dd615e6f0702ca0fc9b Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 7 May 2025 19:06:22 -0400 Subject: [PATCH 1/5] feat(cloudflare): Improve http span data --- packages/cloudflare/src/request.ts | 32 ++-------- packages/core/src/index.ts | 1 + packages/core/src/utils-hoist/url.ts | 91 ++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 27 deletions(-) diff --git a/packages/cloudflare/src/request.ts b/packages/cloudflare/src/request.ts index 8e2f3de06df0..a4cfde50ccf2 100644 --- a/packages/cloudflare/src/request.ts +++ b/packages/cloudflare/src/request.ts @@ -1,17 +1,12 @@ import type { ExecutionContext, IncomingRequestCfProperties } from '@cloudflare/workers-types'; -import type { SpanAttributes } from '@sentry/core'; import { captureException, continueTrace, flush, - SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD, - SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, - SEMANTIC_ATTRIBUTE_URL_FULL, + getHttpSpanDetailsFromUrlObject, + parseStringToURLObject, setHttpStatus, startSpan, - stripUrlQueryAndFragment, withIsolationScope, } from '@sentry/core'; import type { CloudflareOptions } from './client'; @@ -42,29 +37,14 @@ export function wrapRequestHandler( const client = init(options); isolationScope.setClient(client); - const attributes: SpanAttributes = { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.cloudflare', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', - [SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD]: request.method, - [SEMANTIC_ATTRIBUTE_URL_FULL]: request.url, - }; + const urlObject = parseStringToURLObject(request.url); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'auto.http.cloudflare', request); const contentLength = request.headers.get('content-length'); if (contentLength) { attributes['http.request.body.size'] = parseInt(contentLength, 10); } - let pathname = ''; - try { - const url = new URL(request.url); - pathname = url.pathname; - attributes['server.address'] = url.hostname; - attributes['url.scheme'] = url.protocol.replace(':', ''); - } catch { - // skip - } - addCloudResourceContext(isolationScope); if (request) { addRequest(isolationScope, request); @@ -74,8 +54,6 @@ export function wrapRequestHandler( } } - const routeName = `${request.method} ${pathname ? stripUrlQueryAndFragment(pathname) : '/'}`; - // Do not capture spans for OPTIONS and HEAD requests if (request.method === 'OPTIONS' || request.method === 'HEAD') { try { @@ -96,7 +74,7 @@ export function wrapRequestHandler( // See: https://developers.cloudflare.com/workers/runtime-apis/performance/ return startSpan( { - name: routeName, + name, attributes, }, async span => { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 6c35ea212b94..6d281fde0ac9 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -255,6 +255,7 @@ export { parseUrl, stripUrlQueryAndFragment, parseStringToURLObject, + getHttpSpanDetailsFromUrlObject, isURLObjectRelative, getSanitizedUrlStringFromUrlObject, } from './utils-hoist/url'; diff --git a/packages/core/src/utils-hoist/url.ts b/packages/core/src/utils-hoist/url.ts index 7a7893a36b68..d65231b692f3 100644 --- a/packages/core/src/utils-hoist/url.ts +++ b/packages/core/src/utils-hoist/url.ts @@ -1,3 +1,12 @@ +import { + SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + SEMANTIC_ATTRIBUTE_URL_FULL, +} from '../semanticAttributes'; +import type { SpanAttributes } from '../types-hoist/span'; + type PartialURL = { host?: string; path?: string; @@ -107,6 +116,88 @@ export function getSanitizedUrlStringFromUrlObject(url: URLObject): string { return newUrl.toString(); } +type PartialRequest = { + method?: string; +}; + +function getHttpSpanNameFromUrlObject( + urlObject: URLObject | undefined, + request?: PartialRequest, + routeName?: string, +): string { + const method = request?.method?.toUpperCase() ?? 'GET'; + const route = routeName ? routeName : urlObject ? getSanitizedUrlStringFromUrlObject(urlObject) : '/'; + + return `${method} ${route}`; +} + +/** + * Takes a parsed URL object and returns a set of attributes for the span + * that represents the HTTP request for that url. This is used for both server + * and client http spans. + * + * Follows https://opentelemetry.io/docs/specs/semconv/http/. + * + * @param urlObject - see {@link parseStringToURLObject} + * @param httpType - The type of HTTP operation (server or client) + * @param spanOrigin - The origin of the span + * @param request - The request object, see {@link PartialRequest} + * @param routeName - The name of the route, must be low cardinality + * @returns The span name and attributes for the HTTP operation + */ +export function getHttpSpanDetailsFromUrlObject( + urlObject: URLObject | undefined, + httpType: 'server' | 'client', + spanOrigin: string, + request?: PartialRequest, + routeName?: string, +): [name: string, attributes: SpanAttributes] { + const attributes: SpanAttributes = { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `http.${httpType}`, + }; + + if (routeName) { + attributes['http.route'] = routeName; + attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'route'; + } + + if (request?.method) { + attributes[SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD] = request.method.toUpperCase(); + } + + if (urlObject) { + if (urlObject.search) { + attributes['url.query'] = urlObject.search; + } + if (urlObject.hash) { + attributes['url.fragment'] = urlObject.hash; + } + if (urlObject.pathname) { + attributes['url.path'] = urlObject.pathname; + if (urlObject.pathname === '/') { + attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'route'; + } + } + + if (!isURLObjectRelative(urlObject)) { + attributes[SEMANTIC_ATTRIBUTE_URL_FULL] = urlObject.href; + if (urlObject.port) { + attributes['url.port'] = urlObject.port; + } + if (urlObject.protocol) { + attributes['url.scheme'] = urlObject.protocol; + } + if (urlObject.hostname) { + attributes['server.address'] = urlObject.hostname; + } + } + } + + return [getHttpSpanNameFromUrlObject(urlObject, request, routeName), attributes]; +} + /** * Parses string form of URL into an object * // borrowed from https://tools.ietf.org/html/rfc3986#appendix-B From 9ee29136ebd870ff6ec54cd7448317813223abf0 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 8 May 2025 10:51:54 -0400 Subject: [PATCH 2/5] Remove op from `getHttpSpanDetailsFromUrlObject` func --- packages/cloudflare/src/request.ts | 5 +- packages/core/src/utils-hoist/url.ts | 5 +- packages/core/test/utils-hoist/url.test.ts | 98 +++++++++++++++++----- 3 files changed, 83 insertions(+), 25 deletions(-) diff --git a/packages/cloudflare/src/request.ts b/packages/cloudflare/src/request.ts index a4cfde50ccf2..bb0c1b8aa88b 100644 --- a/packages/cloudflare/src/request.ts +++ b/packages/cloudflare/src/request.ts @@ -5,6 +5,7 @@ import { flush, getHttpSpanDetailsFromUrlObject, parseStringToURLObject, + SEMANTIC_ATTRIBUTE_SENTRY_OP, setHttpStatus, startSpan, withIsolationScope, @@ -38,13 +39,15 @@ export function wrapRequestHandler( isolationScope.setClient(client); const urlObject = parseStringToURLObject(request.url); - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'auto.http.cloudflare', request); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'auto.http.cloudflare', request); const contentLength = request.headers.get('content-length'); if (contentLength) { attributes['http.request.body.size'] = parseInt(contentLength, 10); } + attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] = 'http.server'; + addCloudResourceContext(isolationScope); if (request) { addRequest(isolationScope, request); diff --git a/packages/core/src/utils-hoist/url.ts b/packages/core/src/utils-hoist/url.ts index d65231b692f3..084308062772 100644 --- a/packages/core/src/utils-hoist/url.ts +++ b/packages/core/src/utils-hoist/url.ts @@ -1,6 +1,5 @@ import { SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD, - SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_URL_FULL, @@ -62,7 +61,7 @@ export function isURLObjectRelative(url: URLObject): url is RelativeURL { * @returns The parsed URL object or undefined if the URL is invalid */ export function parseStringToURLObject(url: string, urlBase?: string | URL | undefined): URLObject | undefined { - const isRelative = url.startsWith('/'); + const isRelative = url.indexOf('://') <= 0 && url.indexOf('//') !== 0; const base = urlBase ?? (isRelative ? DEFAULT_BASE_URL : undefined); try { // Use `canParse` to short-circuit the URL constructor if it's not a valid URL @@ -147,7 +146,6 @@ function getHttpSpanNameFromUrlObject( */ export function getHttpSpanDetailsFromUrlObject( urlObject: URLObject | undefined, - httpType: 'server' | 'client', spanOrigin: string, request?: PartialRequest, routeName?: string, @@ -155,7 +153,6 @@ export function getHttpSpanDetailsFromUrlObject( const attributes: SpanAttributes = { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `http.${httpType}`, }; if (routeName) { diff --git a/packages/core/test/utils-hoist/url.test.ts b/packages/core/test/utils-hoist/url.test.ts index 7f1d1dae8b40..44e6445d0818 100644 --- a/packages/core/test/utils-hoist/url.test.ts +++ b/packages/core/test/utils-hoist/url.test.ts @@ -203,28 +203,86 @@ describe('parseUrl', () => { }); describe('parseStringToURLObject', () => { - it('returns undefined for invalid URLs', () => { - expect(parseStringToURLObject('invalid-url')).toBeUndefined(); - }); - - it('returns a URL object for valid URLs', () => { - expect(parseStringToURLObject('https://somedomain.com')).toBeInstanceOf(URL); - }); - - it('returns a URL object for valid URLs with a base URL', () => { - expect(parseStringToURLObject('https://somedomain.com', 'https://base.com')).toBeInstanceOf(URL); - }); - - it('returns a relative URL object for relative URLs', () => { - expect(parseStringToURLObject('/path/to/happiness')).toEqual({ - isRelative: true, - pathname: '/path/to/happiness', - search: '', - hash: '', - }); + it.each([ + [ + 'invalid URL', + 'invalid-url', + { + isRelative: true, + pathname: '/invalid-url', + search: '', + hash: '', + }, + ], + ['valid absolute URL', 'https://somedomain.com', expect.any(URL)], + ['valid absolute URL with base', 'https://somedomain.com', expect.any(URL), 'https://base.com'], + [ + 'relative URL', + '/path/to/happiness', + { + isRelative: true, + pathname: '/path/to/happiness', + search: '', + hash: '', + }, + ], + [ + 'relative URL with query', + '/path/to/happiness?q=1', + { + isRelative: true, + pathname: '/path/to/happiness', + search: '?q=1', + hash: '', + }, + ], + [ + 'relative URL with hash', + '/path/to/happiness#section', + { + isRelative: true, + pathname: '/path/to/happiness', + search: '', + hash: '#section', + }, + ], + [ + 'relative URL with query and hash', + '/path/to/happiness?q=1#section', + { + isRelative: true, + pathname: '/path/to/happiness', + search: '?q=1', + hash: '#section', + }, + ], + ['URL with port', 'https://somedomain.com:8080/path', expect.any(URL)], + ['URL with auth', 'https://user:pass@somedomain.com', expect.any(URL)], + ['URL with special chars', 'https://somedomain.com/path/with spaces/and/special@chars', expect.any(URL)], + ['URL with unicode', 'https://somedomain.com/path/with/unicode/测试', expect.any(URL)], + ['URL with multiple query params', 'https://somedomain.com/path?q1=1&q2=2&q3=3', expect.any(URL)], + ['URL with encoded chars', 'https://somedomain.com/path/%20%2F%3F%23', expect.any(URL)], + ['URL with IPv4', 'https://192.168.1.1/path', expect.any(URL)], + ['URL with IPv6', 'https://[2001:db8::1]/path', expect.any(URL)], + ['URL with subdomain', 'https://sub.somedomain.com/path', expect.any(URL)], + ['URL with multiple subdomains', 'https://sub1.sub2.somedomain.com/path', expect.any(URL)], + ['URL with trailing slash', 'https://somedomain.com/path/', expect.any(URL)], + ['URL with empty path', 'https://somedomain.com', expect.any(URL)], + ['URL with root path', 'https://somedomain.com/', expect.any(URL)], + ['URL with file extension', 'https://somedomain.com/path/file.html', expect.any(URL)], + ['URL with custom protocol', 'custom://somedomain.com/path', expect.any(URL)], + ['URL with query containing special chars', 'https://somedomain.com/path?q=hello+world&x=1/2', expect.any(URL)], + ['URL with hash containing special chars', 'https://somedomain.com/path#section/1/2', expect.any(URL)], + [ + 'URL with all components', + 'https://user:pass@sub.somedomain.com:8080/path/file.html?q=1#section', + expect.any(URL), + ], + ])('handles %s', (_, url: string, expected: any, base?: string) => { + expect(parseStringToURLObject(url, base)).toEqual(expected); }); - it('does not throw an error if URl.canParse is not defined', () => { + it('does not throw an error if URL.canParse is not defined', () => { const canParse = (URL as any).canParse; delete (URL as any).canParse; expect(parseStringToURLObject('https://somedomain.com')).toBeInstanceOf(URL); From cfc1f8b29b32e672bd8988a5d9654f54ea94250a Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 8 May 2025 11:09:06 -0400 Subject: [PATCH 3/5] add more tests --- packages/core/test/utils-hoist/url.test.ts | 267 +++++++++++++++++++++ 1 file changed, 267 insertions(+) diff --git a/packages/core/test/utils-hoist/url.test.ts b/packages/core/test/utils-hoist/url.test.ts index 44e6445d0818..a7f088b22d1c 100644 --- a/packages/core/test/utils-hoist/url.test.ts +++ b/packages/core/test/utils-hoist/url.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; import { + getHttpSpanDetailsFromUrlObject, getSanitizedUrlString, getSanitizedUrlStringFromUrlObject, isURLObjectRelative, @@ -344,6 +345,48 @@ describe('getSanitizedUrlStringFromUrlObject', () => { ['url with port 4433', 'http://172.31.12.144:4433/test', 'http://172.31.12.144:4433/test'], ['url with port 443', 'http://172.31.12.144:443/test', 'http://172.31.12.144/test'], ['url with IP and port 80', 'http://172.31.12.144:80/test', 'http://172.31.12.144/test'], + ['invalid URL', 'invalid-url', '/invalid-url'], + ['valid absolute URL with base', 'https://somedomain.com', 'https://somedomain.com/'], + ['relative URL', '/path/to/happiness', '/path/to/happiness'], + ['relative URL with query', '/path/to/happiness?q=1', '/path/to/happiness'], + ['relative URL with hash', '/path/to/happiness#section', '/path/to/happiness'], + ['relative URL with query and hash', '/path/to/happiness?q=1#section', '/path/to/happiness'], + [ + 'URL with special chars', + 'https://somedomain.com/path/with spaces/and/special@chars', + 'https://somedomain.com/path/with%20spaces/and/special@chars', + ], + [ + 'URL with unicode', + 'https://somedomain.com/path/with/unicode/测试', + 'https://somedomain.com/path/with/unicode/%E6%B5%8B%E8%AF%95', + ], + ['URL with multiple query params', 'https://somedomain.com/path?q1=1&q2=2&q3=3', 'https://somedomain.com/path'], + ['URL with encoded chars', 'https://somedomain.com/path/%20%2F%3F%23', 'https://somedomain.com/path/%20%2F%3F%23'], + ['URL with IPv4', 'https://192.168.1.1/path', 'https://192.168.1.1/path'], + ['URL with IPv6', 'https://[2001:db8::1]/path', 'https://[2001:db8::1]/path'], + ['URL with subdomain', 'https://sub.somedomain.com/path', 'https://sub.somedomain.com/path'], + ['URL with multiple subdomains', 'https://sub1.sub2.somedomain.com/path', 'https://sub1.sub2.somedomain.com/path'], + ['URL with trailing slash', 'https://somedomain.com/path/', 'https://somedomain.com/path/'], + ['URL with empty path', 'https://somedomain.com', 'https://somedomain.com/'], + ['URL with root path', 'https://somedomain.com/', 'https://somedomain.com/'], + ['URL with file extension', 'https://somedomain.com/path/file.html', 'https://somedomain.com/path/file.html'], + ['URL with custom protocol', 'custom://somedomain.com/path', 'custom://somedomain.com/path'], + [ + 'URL with query containing special chars', + 'https://somedomain.com/path?q=hello+world&x=1/2', + 'https://somedomain.com/path', + ], + [ + 'URL with hash containing special chars', + 'https://somedomain.com/path#section/1/2', + 'https://somedomain.com/path', + ], + [ + 'URL with all components', + 'https://user:pass@sub.somedomain.com:8080/path/file.html?q=1#section', + 'https://%filtered%:%filtered%@sub.somedomain.com:8080/path/file.html', + ], ])('returns a sanitized URL for a %s', (_, rawUrl: string, sanitizedURL: string) => { const urlObject = parseStringToURLObject(rawUrl); if (!urlObject) { @@ -352,3 +395,227 @@ describe('getSanitizedUrlStringFromUrlObject', () => { expect(getSanitizedUrlStringFromUrlObject(urlObject)).toEqual(sanitizedURL); }); }); + +describe('getHttpSpanDetailsFromUrlObject', () => { + it('handles undefined URL object', () => { + const [name, attributes] = getHttpSpanDetailsFromUrlObject(undefined, 'test-origin'); + expect(name).toBe('GET /'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + }); + }); + + it('handles relative URL object', () => { + const urlObject = parseStringToURLObject('/api/users')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + expect(name).toBe('GET /api/users'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + 'url.path': '/api/users', + }); + }); + + it('handles absolute URL object', () => { + const urlObject = parseStringToURLObject('https://example.com/api/users?q=test#section')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + expect(name).toBe('GET https://example.com/api/users'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + 'url.path': '/api/users', + 'url.query': '?q=test', + 'url.fragment': '#section', + 'url.full': 'https://example.com/api/users?q=test#section', + 'server.address': 'example.com', + 'url.scheme': 'https:', + }); + }); + + it('handles URL object with request method', () => { + const urlObject = parseStringToURLObject('https://example.com/api/users')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', { method: 'POST' }); + expect(name).toBe('POST https://example.com/api/users'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + 'url.path': '/api/users', + 'url.full': 'https://example.com/api/users', + 'server.address': 'example.com', + 'url.scheme': 'https:', + 'http.request.method': 'POST', + }); + }); + + it('handles URL object with route name', () => { + const urlObject = parseStringToURLObject('https://example.com/api/users')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', undefined, '/api/users/:id'); + expect(name).toBe('GET /api/users/:id'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'route', + 'url.path': '/api/users', + 'url.full': 'https://example.com/api/users', + 'server.address': 'example.com', + 'url.scheme': 'https:', + 'http.route': '/api/users/:id', + }); + }); + + it('handles root path URL', () => { + const urlObject = parseStringToURLObject('https://example.com/')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + expect(name).toBe('GET https://example.com/'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'route', + 'url.path': '/', + 'url.full': 'https://example.com/', + 'server.address': 'example.com', + 'url.scheme': 'https:', + }); + }); + + it('handles URL with port', () => { + const urlObject = parseStringToURLObject('https://example.com:8080/api/users')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + expect(name).toBe('GET https://example.com:8080/api/users'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + 'url.path': '/api/users', + 'url.full': 'https://example.com:8080/api/users', + 'server.address': 'example.com', + 'url.scheme': 'https:', + 'url.port': '8080', + }); + }); + + it('handles URL with non-standard port and request method', () => { + const urlObject = parseStringToURLObject('https://example.com:3000/api/users')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', { method: 'PUT' }); + expect(name).toBe('PUT https://example.com:3000/api/users'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + 'url.path': '/api/users', + 'url.full': 'https://example.com:3000/api/users', + 'server.address': 'example.com', + 'url.scheme': 'https:', + 'url.port': '3000', + 'http.request.method': 'PUT', + }); + }); + + it('handles URL with route name and request method', () => { + const urlObject = parseStringToURLObject('https://example.com/api/users/123')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject( + urlObject, + 'test-origin', + { method: 'PATCH' }, + '/api/users/:id', + ); + expect(name).toBe('PATCH /api/users/:id'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'route', + 'url.path': '/api/users/123', + 'url.full': 'https://example.com/api/users/123', + 'server.address': 'example.com', + 'url.scheme': 'https:', + 'http.route': '/api/users/:id', + 'http.request.method': 'PATCH', + }); + }); + + it('handles URL with query params and route name', () => { + const urlObject = parseStringToURLObject('https://example.com/api/search?q=test&page=1')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', undefined, '/api/search'); + expect(name).toBe('GET /api/search'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'route', + 'url.path': '/api/search', + 'url.query': '?q=test&page=1', + 'url.full': 'https://example.com/api/search?q=test&page=1', + 'server.address': 'example.com', + 'url.scheme': 'https:', + 'http.route': '/api/search', + }); + }); + + it('handles URL with fragment and route name', () => { + const urlObject = parseStringToURLObject('https://example.com/api/docs#section-1')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', undefined, '/api/docs'); + expect(name).toBe('GET /api/docs'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'route', + 'url.path': '/api/docs', + 'url.fragment': '#section-1', + 'url.full': 'https://example.com/api/docs#section-1', + 'server.address': 'example.com', + 'url.scheme': 'https:', + 'http.route': '/api/docs', + }); + }); + + it('handles URL with auth credentials', () => { + const urlObject = parseStringToURLObject('https://user:pass@example.com/api/users')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + expect(name).toBe('GET https://%filtered%:%filtered%@example.com/api/users'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + 'url.path': '/api/users', + 'url.full': 'https://user:pass@example.com/api/users', + 'server.address': 'example.com', + 'url.scheme': 'https:', + }); + }); + + it('handles URL with IPv4 address', () => { + const urlObject = parseStringToURLObject('https://192.168.1.1:8080/api/users')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + expect(name).toBe('GET https://192.168.1.1:8080/api/users'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + 'url.path': '/api/users', + 'url.full': 'https://192.168.1.1:8080/api/users', + 'server.address': '192.168.1.1', + 'url.scheme': 'https:', + 'url.port': '8080', + }); + }); + + it('handles URL with IPv6 address', () => { + const urlObject = parseStringToURLObject('https://[2001:db8::1]:8080/api/users')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + expect(name).toBe('GET https://[2001:db8::1]:8080/api/users'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + 'url.path': '/api/users', + 'url.full': 'https://[2001:db8::1]:8080/api/users', + 'server.address': '[2001:db8::1]', + 'url.scheme': 'https:', + 'url.port': '8080', + }); + }); + + it('handles URL with subdomain', () => { + const urlObject = parseStringToURLObject('https://api.example.com/users')!; + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + expect(name).toBe('GET https://api.example.com/users'); + expect(attributes).toEqual({ + 'sentry.origin': 'test-origin', + 'sentry.source': 'url', + 'url.path': '/users', + 'url.full': 'https://api.example.com/users', + 'server.address': 'api.example.com', + 'url.scheme': 'https:', + }); + }); +}); From 52b7884f49402abf0c19822d2d137529722bd6ec Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 8 May 2025 18:37:48 -0400 Subject: [PATCH 4/5] re-introduce http kind --- packages/cloudflare/src/request.ts | 2 +- packages/cloudflare/test/request.test.ts | 7 +++- packages/core/src/utils-hoist/url.ts | 18 ++++++--- packages/core/test/utils-hoist/url.test.ts | 47 +++++++++++++++------- 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/packages/cloudflare/src/request.ts b/packages/cloudflare/src/request.ts index bb0c1b8aa88b..f1905609fb94 100644 --- a/packages/cloudflare/src/request.ts +++ b/packages/cloudflare/src/request.ts @@ -39,7 +39,7 @@ export function wrapRequestHandler( isolationScope.setClient(client); const urlObject = parseStringToURLObject(request.url); - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'auto.http.cloudflare', request); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'auto.http.cloudflare', request); const contentLength = request.headers.get('content-length'); if (contentLength) { diff --git a/packages/cloudflare/test/request.test.ts b/packages/cloudflare/test/request.test.ts index a778b60befeb..4fc9b308ec54 100644 --- a/packages/cloudflare/test/request.test.ts +++ b/packages/cloudflare/test/request.test.ts @@ -254,12 +254,13 @@ describe('withSentry', () => { data: { 'sentry.origin': 'auto.http.cloudflare', 'sentry.op': 'http.server', - 'sentry.source': 'url', + 'sentry.source': 'route', 'http.request.method': 'GET', 'url.full': 'https://example.com/', 'server.address': 'example.com', 'network.protocol.name': 'HTTP/1.1', - 'url.scheme': 'https', + 'url.scheme': 'https:', + 'url.path': '/', 'sentry.sample_rate': 1, 'http.response.status_code': 200, 'http.request.body.size': 10, @@ -269,6 +270,8 @@ describe('withSentry', () => { span_id: expect.stringMatching(/[a-f0-9]{16}/), status: 'ok', trace_id: expect.stringMatching(/[a-f0-9]{32}/), + parent_span_id: undefined, + links: undefined, }); }); }); diff --git a/packages/core/src/utils-hoist/url.ts b/packages/core/src/utils-hoist/url.ts index 084308062772..9a51e20b7807 100644 --- a/packages/core/src/utils-hoist/url.ts +++ b/packages/core/src/utils-hoist/url.ts @@ -121,11 +121,18 @@ type PartialRequest = { function getHttpSpanNameFromUrlObject( urlObject: URLObject | undefined, + kind: 'server' | 'client', request?: PartialRequest, routeName?: string, ): string { const method = request?.method?.toUpperCase() ?? 'GET'; - const route = routeName ? routeName : urlObject ? getSanitizedUrlStringFromUrlObject(urlObject) : '/'; + const route = routeName + ? routeName + : urlObject + ? kind === 'client' + ? getSanitizedUrlStringFromUrlObject(urlObject) + : urlObject.pathname + : '/'; return `${method} ${route}`; } @@ -138,7 +145,7 @@ function getHttpSpanNameFromUrlObject( * Follows https://opentelemetry.io/docs/specs/semconv/http/. * * @param urlObject - see {@link parseStringToURLObject} - * @param httpType - The type of HTTP operation (server or client) + * @param kind - The type of HTTP operation (server or client) * @param spanOrigin - The origin of the span * @param request - The request object, see {@link PartialRequest} * @param routeName - The name of the route, must be low cardinality @@ -146,6 +153,7 @@ function getHttpSpanNameFromUrlObject( */ export function getHttpSpanDetailsFromUrlObject( urlObject: URLObject | undefined, + kind: 'server' | 'client', spanOrigin: string, request?: PartialRequest, routeName?: string, @@ -156,7 +164,7 @@ export function getHttpSpanDetailsFromUrlObject( }; if (routeName) { - attributes['http.route'] = routeName; + attributes[kind === 'client' ? 'http.route' : 'url.route'] = routeName; attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'route'; } @@ -187,12 +195,12 @@ export function getHttpSpanDetailsFromUrlObject( attributes['url.scheme'] = urlObject.protocol; } if (urlObject.hostname) { - attributes['server.address'] = urlObject.hostname; + attributes[kind === 'server' ? 'server.address' : 'url.domain'] = urlObject.hostname; } } } - return [getHttpSpanNameFromUrlObject(urlObject, request, routeName), attributes]; + return [getHttpSpanNameFromUrlObject(urlObject, kind, request, routeName), attributes]; } /** diff --git a/packages/core/test/utils-hoist/url.test.ts b/packages/core/test/utils-hoist/url.test.ts index a7f088b22d1c..a77e3e9baf16 100644 --- a/packages/core/test/utils-hoist/url.test.ts +++ b/packages/core/test/utils-hoist/url.test.ts @@ -398,7 +398,7 @@ describe('getSanitizedUrlStringFromUrlObject', () => { describe('getHttpSpanDetailsFromUrlObject', () => { it('handles undefined URL object', () => { - const [name, attributes] = getHttpSpanDetailsFromUrlObject(undefined, 'test-origin'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(undefined, 'server', 'test-origin'); expect(name).toBe('GET /'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -408,7 +408,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles relative URL object', () => { const urlObject = parseStringToURLObject('/api/users')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); expect(name).toBe('GET /api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -419,7 +419,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles absolute URL object', () => { const urlObject = parseStringToURLObject('https://example.com/api/users?q=test#section')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); expect(name).toBe('GET https://example.com/api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -435,7 +435,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL object with request method', () => { const urlObject = parseStringToURLObject('https://example.com/api/users')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', { method: 'POST' }); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin', { method: 'POST' }); expect(name).toBe('POST https://example.com/api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -450,7 +450,13 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL object with route name', () => { const urlObject = parseStringToURLObject('https://example.com/api/users')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', undefined, '/api/users/:id'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject( + urlObject, + 'server', + 'test-origin', + undefined, + '/api/users/:id', + ); expect(name).toBe('GET /api/users/:id'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -465,7 +471,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles root path URL', () => { const urlObject = parseStringToURLObject('https://example.com/')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); expect(name).toBe('GET https://example.com/'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -479,7 +485,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with port', () => { const urlObject = parseStringToURLObject('https://example.com:8080/api/users')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); expect(name).toBe('GET https://example.com:8080/api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -494,7 +500,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with non-standard port and request method', () => { const urlObject = parseStringToURLObject('https://example.com:3000/api/users')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', { method: 'PUT' }); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin', { method: 'PUT' }); expect(name).toBe('PUT https://example.com:3000/api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -512,6 +518,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { const urlObject = parseStringToURLObject('https://example.com/api/users/123')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject( urlObject, + 'server', 'test-origin', { method: 'PATCH' }, '/api/users/:id', @@ -531,7 +538,13 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with query params and route name', () => { const urlObject = parseStringToURLObject('https://example.com/api/search?q=test&page=1')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', undefined, '/api/search'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject( + urlObject, + 'server', + 'test-origin', + undefined, + '/api/search', + ); expect(name).toBe('GET /api/search'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -547,7 +560,13 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with fragment and route name', () => { const urlObject = parseStringToURLObject('https://example.com/api/docs#section-1')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin', undefined, '/api/docs'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject( + urlObject, + 'server', + 'test-origin', + undefined, + '/api/docs', + ); expect(name).toBe('GET /api/docs'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -563,7 +582,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with auth credentials', () => { const urlObject = parseStringToURLObject('https://user:pass@example.com/api/users')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); expect(name).toBe('GET https://%filtered%:%filtered%@example.com/api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -577,7 +596,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with IPv4 address', () => { const urlObject = parseStringToURLObject('https://192.168.1.1:8080/api/users')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); expect(name).toBe('GET https://192.168.1.1:8080/api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -592,7 +611,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with IPv6 address', () => { const urlObject = parseStringToURLObject('https://[2001:db8::1]:8080/api/users')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); expect(name).toBe('GET https://[2001:db8::1]:8080/api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', @@ -607,7 +626,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with subdomain', () => { const urlObject = parseStringToURLObject('https://api.example.com/users')!; - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'test-origin'); + const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); expect(name).toBe('GET https://api.example.com/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', From 72f50061075fba8087e6f1b7717d25d1186dc83e Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 8 May 2025 20:53:03 -0400 Subject: [PATCH 5/5] fix tests --- packages/core/src/utils-hoist/url.ts | 3 ++- packages/core/test/utils-hoist/url.test.ts | 18 +++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/core/src/utils-hoist/url.ts b/packages/core/src/utils-hoist/url.ts index 9a51e20b7807..ca09e6e8b5e7 100644 --- a/packages/core/src/utils-hoist/url.ts +++ b/packages/core/src/utils-hoist/url.ts @@ -164,7 +164,8 @@ export function getHttpSpanDetailsFromUrlObject( }; if (routeName) { - attributes[kind === 'client' ? 'http.route' : 'url.route'] = routeName; + // This is based on https://opentelemetry.io/docs/specs/semconv/http/http-spans/#name + attributes[kind === 'server' ? 'http.route' : 'url.template'] = routeName; attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'route'; } diff --git a/packages/core/test/utils-hoist/url.test.ts b/packages/core/test/utils-hoist/url.test.ts index a77e3e9baf16..67ec8b31644f 100644 --- a/packages/core/test/utils-hoist/url.test.ts +++ b/packages/core/test/utils-hoist/url.test.ts @@ -420,7 +420,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles absolute URL object', () => { const urlObject = parseStringToURLObject('https://example.com/api/users?q=test#section')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); - expect(name).toBe('GET https://example.com/api/users'); + expect(name).toBe('GET /api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', 'sentry.source': 'url', @@ -436,7 +436,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL object with request method', () => { const urlObject = parseStringToURLObject('https://example.com/api/users')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin', { method: 'POST' }); - expect(name).toBe('POST https://example.com/api/users'); + expect(name).toBe('POST /api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', 'sentry.source': 'url', @@ -472,7 +472,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles root path URL', () => { const urlObject = parseStringToURLObject('https://example.com/')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); - expect(name).toBe('GET https://example.com/'); + expect(name).toBe('GET /'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', 'sentry.source': 'route', @@ -486,7 +486,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with port', () => { const urlObject = parseStringToURLObject('https://example.com:8080/api/users')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); - expect(name).toBe('GET https://example.com:8080/api/users'); + expect(name).toBe('GET /api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', 'sentry.source': 'url', @@ -501,7 +501,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with non-standard port and request method', () => { const urlObject = parseStringToURLObject('https://example.com:3000/api/users')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin', { method: 'PUT' }); - expect(name).toBe('PUT https://example.com:3000/api/users'); + expect(name).toBe('PUT /api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', 'sentry.source': 'url', @@ -583,7 +583,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with auth credentials', () => { const urlObject = parseStringToURLObject('https://user:pass@example.com/api/users')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); - expect(name).toBe('GET https://%filtered%:%filtered%@example.com/api/users'); + expect(name).toBe('GET /api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', 'sentry.source': 'url', @@ -597,7 +597,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with IPv4 address', () => { const urlObject = parseStringToURLObject('https://192.168.1.1:8080/api/users')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); - expect(name).toBe('GET https://192.168.1.1:8080/api/users'); + expect(name).toBe('GET /api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', 'sentry.source': 'url', @@ -612,7 +612,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with IPv6 address', () => { const urlObject = parseStringToURLObject('https://[2001:db8::1]:8080/api/users')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); - expect(name).toBe('GET https://[2001:db8::1]:8080/api/users'); + expect(name).toBe('GET /api/users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', 'sentry.source': 'url', @@ -627,7 +627,7 @@ describe('getHttpSpanDetailsFromUrlObject', () => { it('handles URL with subdomain', () => { const urlObject = parseStringToURLObject('https://api.example.com/users')!; const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'test-origin'); - expect(name).toBe('GET https://api.example.com/users'); + expect(name).toBe('GET /users'); expect(attributes).toEqual({ 'sentry.origin': 'test-origin', 'sentry.source': 'url',