Remove op from getHttpSpanDetailsFromUrlObject func

Author: AbhiPrasad

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);

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,15 +146,13 @@ function getHttpSpanNameFromUrlObject(
  */
 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) {

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:[email protected]', 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:[email protected]: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);