fix(span-stats,exporters): derive socketPath from the agent URL (#9042)

* fix(span-stats): derive socketPath from the agent URL

The span-stats writer hand-copied protocol/hostname/port into the request
options and never passed the agent URL, so request() could not reach its
unix-socket branch and left protocol set to 'unix:'. With a named-pipe or
UDS agent URL, http.request then threw ERR_INVALID_PROTOCOL and every span
stats payload was dropped. Passing `url` lets request() map a unix: URL onto
options.socketPath, the same way the trace agent exporter already does.

* fix(exporters): recompose named-pipe socket paths in one shared helper

The CI-visibility and profiling exporters built request options from
`url.pathname` directly, so a Windows named-pipe agent URL
(`unix://./pipe/...`) lost its `//.` authority and the request hit the wrong
socket path. parseUrl now lives in exporters/common/url.js and folds that
authority back into the path, so all three request paths derive the socket
path the same way and can no longer drift.

Drive-by fix:

* The common request spec restores its sinon sandbox instead of resetting it,
  so the `http.request` spy no longer leaks into sibling suites.

* refactor(telemetry): send the agentless backend URL as a URL object

The agent-telemetry fallback passed the agentless intake endpoint to `request`
as a string while the primary agentless path already constructed a `URL`. Build
it with `new URL` up front and guard it the same way, so an invalid endpoint is
logged and the request skipped rather than throwing from inside the request
helper.

Author: BridgeAR

.github/CODEOWNERS

@@ -346,7 +346,7 @@
 /packages/datadog-core @DataDog/lang-platform-js
 /packages/datadog-shimmer @DataDog/lang-platform-js
 /packages/dd-trace/*/crashtracking @DataDog/lang-platform-js
-/packages/dd-trace/src/exporters/common/retry.js @DataDog/lang-platform-js
+/packages/dd-trace/src/exporters/common/ @DataDog/lang-platform-js
 /packages/dd-trace/test/agent/ @DataDog/lang-platform-js
 /packages/dd-trace/test/dd-trace.spec.js @DataDog/lang-platform-js
 /packages/dd-trace/test/dogstatsd.spec.js @DataDog/lang-platform-js

packages/dd-trace/src/ci-visibility/requests/request.js

@@ -12,26 +12,10 @@ const {
   isRetriableNetworkError,
   singleJitteredDelay,
 } = require('../../exporters/common/retry')
-const { urlToHttpOptions } = require('../../exporters/common/url-to-http-options-polyfill')
+const { parseUrl } = require('../../exporters/common/url')
 
 const legacyStorage = storage('legacy')
 
-function parseUrl (urlObjOrString) {
-  if (urlObjOrString !== null && typeof urlObjOrString === 'object') {
-    return urlToHttpOptions(urlObjOrString)
-  }
-
-  const url = urlToHttpOptions(new URL(urlObjOrString))
-
-  if (url.protocol === 'unix:' && url.hostname === '.') {
-    const udsPath = urlObjOrString.slice(5)
-    url.path = udsPath
-    url.pathname = udsPath
-  }
-
-  return url
-}
-
 /**
  * Simplified HTTP request for test optimization (library config). Uses common HTTP agents.
  * Retries: 429 (with X-ratelimit-reset, max 30s wait),

packages/dd-trace/src/exporters/common/request.js

@@ -11,7 +11,7 @@ const zlib = require('zlib')
 
 const { storage } = require('../../../../datadog-core')
 const log = require('../../log')
-const { urlToHttpOptions } = require('./url-to-http-options-polyfill')
+const { parseUrl } = require('./url')
 const docker = require('./docker')
 const { httpAgent, httpsAgent } = require('./agents')
 const {
@@ -27,25 +27,6 @@ const maxActiveBufferSize = 1024 * 1024 * 64
 
 let activeBufferSize = 0
 
-/**
- * @param {string|URL|object} urlObjOrString
- * @returns {object}
- */
-function parseUrl (urlObjOrString) {
-  if (urlObjOrString !== null && typeof urlObjOrString === 'object') return urlToHttpOptions(urlObjOrString)
-
-  const url = urlToHttpOptions(new URL(urlObjOrString))
-
-  // Special handling if we're using named pipes on Windows
-  if (url.protocol === 'unix:' && url.hostname === '.') {
-    const udsPath = urlObjOrString.slice(5)
-    url.path = udsPath
-    url.pathname = udsPath
-  }
-
-  return url
-}
-
 /**
  * @param {string} hostname Host as resolved by {@link parseUrl}; IPv6 is unbracketed (`::1`).
  */

packages/dd-trace/src/exporters/common/url.js

@@ -0,0 +1,34 @@
+'use strict'
+
+const { urlToHttpOptions } = require('./url-to-http-options-polyfill')
+
+/**
+ * Convert an agent/intake URL into Node http(s) request options.
+ *
+ * A Windows named pipe (`unix://./pipe/<name>`) parses the `.` as the URL
+ * authority, dropping it from the path. Fold it back so the socket path stays
+ * `//./pipe/<name>`; otherwise it collapses to `/pipe/<name>` and misses the
+ * pipe. Exporters receive `config.url` as a URL object, so this must run for
+ * both string and object input.
+ *
+ * @param {string|URL|object} urlObjOrString
+ * @returns {object}
+ */
+function parseUrl (urlObjOrString) {
+  // `urlToHttpOptions` returns `pathname` at runtime, but @types/node narrows it
+  // to `ClientRequestArgs`, which omits it; cast so the named-pipe fold below can
+  // read and rewrite it.
+  const url = /** @type {import('node:http').ClientRequestArgs & { pathname: string }} */ (
+    urlObjOrString !== null && typeof urlObjOrString === 'object'
+      ? urlToHttpOptions(urlObjOrString)
+      : urlToHttpOptions(new URL(urlObjOrString))
+  )
+
+  if (url.protocol === 'unix:' && url.hostname === '.') {
+    url.path = url.pathname = `//.${url.pathname}`
+  }
+
+  return url
+}
+
+module.exports = { parseUrl }

packages/dd-trace/src/exporters/span-stats/writer.js

@@ -37,9 +37,7 @@ function makeRequest (data, url, cb) {
       'Datadog-Meta-Tracer-Version': pkg.version,
       'Content-Type': 'application/msgpack',
     },
-    protocol: url.protocol,
-    hostname: url.hostname,
-    port: url.port,
+    url,
   }
 
   log.debug('Request to the intake: %j', options)

packages/dd-trace/src/profiling/exporters/agent.js

@@ -3,12 +3,12 @@
 const { request: httpRequest } = require('http')
 const { request: httpsRequest } = require('https')
 const perf = require('perf_hooks').performance
-const { urlToHttpOptions } = require('url')
 
 const retry = require('../../../../../vendor/dist/retry')
 // TODO: avoid using dd-trace internals. Make this a separate module?
 const docker = require('../../exporters/common/docker')
 const FormData = require('../../exporters/common/form-data')
+const { parseUrl } = require('../../exporters/common/url')
 const log = require('../../log')
 const { storage } = require('../../../../datadog-core')
 const version = require('../../../../../package.json').version
@@ -156,13 +156,13 @@ class AgentExporter extends EventSerializer {
 
         docker.inject(options.headers)
 
-        if (this._url.protocol === 'unix:') {
-          options.socketPath = this._url.pathname
+        const url = parseUrl(this._url)
+        if (url.protocol === 'unix:') {
+          options.socketPath = url.pathname
         } else {
-          const httpOptions = urlToHttpOptions(this._url)
-          options.protocol = httpOptions.protocol
-          options.hostname = httpOptions.hostname
-          options.port = httpOptions.port
+          options.protocol = url.protocol
+          options.hostname = url.hostname
+          options.port = url.port
         }
 
         // eslint-disable-next-line eslint-rules/eslint-log-printf-style

packages/dd-trace/src/telemetry/send-data.js

@@ -182,23 +182,25 @@ function sendData (config, application, host, reqType, payload = {}, cb = () =>
         agentTelemetry = false
       }
       // figure out which data center to send to
-      const backendUrl = getAgentlessTelemetryEndpoint(config.site)
+      let backendUrl
+      try {
+        backendUrl = new URL(getAgentlessTelemetryEndpoint(config.site))
+      } catch {
+        log.error('Invalid Telemetry URL')
+        return
+      }
       const backendHeader = { ...options.headers, 'DD-API-KEY': config.apiKey }
       const backendOptions = {
         ...options,
         url: backendUrl,
         headers: backendHeader,
         path: '/api/v2/apmtelemetry',
       }
-      if (backendUrl) {
-        request(data, backendOptions, (error) => {
-          if (error) {
-            log.error('Error sending telemetry data', error)
-          }
-        })
-      } else {
-        log.error('Invalid Telemetry URL')
-      }
+      request(data, backendOptions, (error) => {
+        if (error) {
+          log.error('Error sending telemetry data', error)
+        }
+      })
     }
 
     if (!error && !agentTelemetry) {

packages/dd-trace/test/ci-visibility/requests/request.spec.js

@@ -1,6 +1,7 @@
 'use strict'
 
 const assert = require('node:assert/strict')
+const http = require('node:http')
 
 const { describe, it, beforeEach, afterEach } = require('mocha')
 const nock = require('nock')
@@ -78,4 +79,22 @@ describe('ci-visibility/requests/request', () => {
       done()
     })
   })
+
+  // A Windows named pipe URL object must reach http.request as a single
+  // `//./pipe/<name>` socket path; the connection itself fails on the test host,
+  // which is fine — we only pin the options the request was built with.
+  it('derives the socket path for a Windows named pipe URL object', (done) => {
+    const requestSpy = sinon.spy(http, 'request')
+
+    request('{}', { url: new URL('unix://./pipe/datadog'), path: '/path' }, () => {
+      requestSpy.restore()
+      try {
+        assert.ok(requestSpy.called)
+        assert.strictEqual(requestSpy.getCall(0).args[0].socketPath, '//./pipe/datadog')
+      } catch (error) {
+        return done(error)
+      }
+      done()
+    })
+  })
 })

packages/dd-trace/test/exporters/common/request.spec.js

@@ -431,6 +431,30 @@ describe('request', function () {
       })
   })
 
+  // Config always hands exporters a URL object (`new URL(...)`), not a string,
+  // so the object branch of parseUrl must apply the same named-pipe handling.
+  // The URL parser splits `unix://./pipe/foo` into authority `.` + path
+  // `/pipe/foo`; without folding `.` back the socket path collapses to
+  // `/pipe/foo` and misses the pipe.
+  it('should parse windows named pipes given as a URL object properly', (done) => {
+    const sandbox = sinon.createSandbox()
+    sandbox.spy(http, 'request')
+
+    maxAttempts = 1
+
+    request(
+      Buffer.from(''), {
+        url: new URL('unix://./pipe/datadogtrace'),
+        method: 'PUT',
+      },
+      () => {
+        const callOptions = http.request.getCall(0).args[0]
+        sandbox.restore()
+        assert.strictEqual(callOptions.socketPath, '//./pipe/datadogtrace')
+        done()
+      })
+  })
+
   it('should calculate correct Content-Length header for multi-byte characters', (done) => {
     const sandbox = sinon.createSandbox()
     sandbox.spy(http, 'request')
@@ -470,7 +494,7 @@ describe('request', function () {
     })
 
     afterEach(() => {
-      sandbox.reset()
+      sandbox.restore()
     })
 
     it('should properly set request host with IPv6', (done) => {

packages/dd-trace/test/exporters/common/url.spec.js

@@ -0,0 +1,72 @@
+'use strict'
+
+const assert = require('node:assert/strict')
+
+const { describe, it } = require('mocha')
+
+require('../../setup/core')
+
+const { parseUrl } = require('../../../src/exporters/common/url')
+
+describe('exporters/common/url parseUrl', () => {
+  describe('unix domain sockets', () => {
+    it('keeps the socket path for a string URL', () => {
+      const url = parseUrl('unix:///var/run/datadog/apm.socket')
+
+      assert.strictEqual(url.protocol, 'unix:')
+      assert.strictEqual(url.pathname, '/var/run/datadog/apm.socket')
+    })
+
+    it('keeps the socket path for a URL object', () => {
+      const url = parseUrl(new URL('unix:///var/run/datadog/apm.socket'))
+
+      assert.strictEqual(url.protocol, 'unix:')
+      assert.strictEqual(url.pathname, '/var/run/datadog/apm.socket')
+    })
+  })
+
+  // The `.` authority of a `unix://./pipe/<name>` URL is parsed out of the path,
+  // so it must be folded back into `//./pipe/<name>`. Both branches matter: the
+  // string form is what tests/CLIs pass, the object form is what config hands
+  // every exporter.
+  describe('windows named pipes', () => {
+    it('folds the authority back for a string URL', () => {
+      const url = parseUrl('unix://./pipe/datadog')
+
+      assert.strictEqual(url.protocol, 'unix:')
+      assert.strictEqual(url.pathname, '//./pipe/datadog')
+    })
+
+    it('folds the authority back for a URL object', () => {
+      const url = parseUrl(new URL('unix://./pipe/datadog'))
+
+      assert.strictEqual(url.protocol, 'unix:')
+      assert.strictEqual(url.pathname, '//./pipe/datadog')
+    })
+
+    it('keeps the backslash form untouched', () => {
+      const url = parseUrl(new URL('unix:\\\\.\\pipe\\datadog'))
+
+      assert.strictEqual(url.protocol, 'unix:')
+      assert.strictEqual(url.pathname, '\\\\.\\pipe\\datadog')
+    })
+  })
+
+  describe('http(s) urls', () => {
+    it('maps protocol, hostname and port for a string URL', () => {
+      const url = parseUrl('https://127.0.0.1:8126/path')
+
+      assert.strictEqual(url.protocol, 'https:')
+      assert.strictEqual(url.hostname, '127.0.0.1')
+      assert.strictEqual(String(url.port), '8126')
+    })
+
+    it('maps protocol, hostname and port for a URL object', () => {
+      const url = parseUrl(new URL('http://localhost:8126'))
+
+      assert.strictEqual(url.protocol, 'http:')
+      assert.strictEqual(url.hostname, 'localhost')
+      assert.strictEqual(String(url.port), '8126')
+    })
+  })
+})