diff --git a/crates/capabilities/src/http_transport.js b/crates/capabilities/src/http_transport.js index acd1c5c..45c6fe7 100644 --- a/crates/capabilities/src/http_transport.js +++ b/crates/capabilities/src/http_transport.js @@ -1,6 +1,11 @@ -const http = require('node:http') -const https = require('node:https') -const fs = require('node:fs') +// NOTE: `node:http`, `node:https` and `node:fs` are deliberately NOT required at +// module load. This transport is loaded during the tracer's own init (before +// user code runs), and requiring an instrumented builtin here makes dd-trace +// wrap it in place immediately — so a user app that imports `http` afterwards +// (e.g. an ESM app under `--require`) sees the wrapped builtin and gets +// instrumented when it should not (breaks the init/guardrail expectations). The +// JS agent exporter requires these lazily (at first send) for the same reason; +// mirror that by requiring them inside the functions that use them below. let storage = f => f() @@ -28,6 +33,7 @@ const entityReg = new RegExp(String.raw`.*(${uuidSource}|${containerSource}|${ta // Detect the entity headers. Parameterized for unit testing; production callers // use the cached `getEntityHeaders()` with the real cgroup paths / environment. function detectEntityHeaders (opts = {}) { + const fs = require('node:fs') const cgroupPath = opts.cgroupPath ?? '/proc/self/cgroup' const cgroupMount = opts.cgroupMount ?? '/sys/fs/cgroup' const externalEnv = 'externalEnv' in opts ? opts.externalEnv : process.env.DD_EXTERNAL_ENV @@ -147,6 +153,15 @@ module.exports.httpRequest = function (host, port, isHttps, socketPath, head_ptr // A non-empty socketPath routes over a Unix domain socket (or Windows named // pipe) instead of TCP. Sockets are always plaintext HTTP/1.1, so https is // ignored in that mode. + const http = require('node:http') + const https = require('node:https') + // libdatadog derives `host` from the agent URI, which keeps the brackets for + // an IPv6 literal (e.g. `[::1]`). Node's `http.request` treats the `host` + // option as a hostname to resolve, so `[::1]` fails with ENOTFOUND. Strip the + // brackets so the IPv6 address is used directly (Node accepts `::1`). + if (typeof host === 'string' && host.length > 1 && host[0] === '[' && host.at(-1) === ']') { + host = host.slice(1, -1) + } const useSocket = typeof socketPath === 'string' && socketPath.length > 0 const transport = useSocket ? http : (isHttps ? https : http) diff --git a/crates/pipeline/src/lib.rs b/crates/pipeline/src/lib.rs index 54b03e8..181589a 100644 --- a/crates/pipeline/src/lib.rs +++ b/crates/pipeline/src/lib.rs @@ -280,7 +280,12 @@ impl WasmSpanState { let change_buffer_state = ChangeBufferState::new( change_buffer, tracer_service.into(), - lang.into(), + // The change buffer stamps this onto every span's `language` meta tag. + // For the Node.js tracer that must be "javascript" (matching the JS + // pipeline's span_format), NOT the `Datadog-Meta-Lang: nodejs` header + // value (`lang`) that identifies the tracer library to the agent. The + // two legitimately differ for Node, so we can't reuse `lang` here. + "javascript".into(), pid, ); diff --git a/crates/pipeline/src/stats.rs b/crates/pipeline/src/stats.rs index 24580bb..58c7e68 100644 --- a/crates/pipeline/src/stats.rs +++ b/crates/pipeline/src/stats.rs @@ -96,7 +96,16 @@ impl StatsCollector { let body = rmp_serde::encode::to_vec_named(&payload) .map_err(|e| format!("stats msgpack encode error: {e}"))?; - let stats_url = format!("{}{}", self.agent_url, STATS_ENDPOINT_PATH); + // `agent_url` typically ends in `/` (Node's `URL.toString()` normalizes a + // bare authority that way), so a naive concat yields `//v0.6/stats`. The + // agent records the raw request path, so the double slash makes the + // test-agent (and stats tooling) miss the request entirely. Trim the + // trailing slash so the path is exactly `/v0.6/stats`. + let stats_url = format!( + "{}{}", + self.agent_url.trim_end_matches('/'), + STATS_ENDPOINT_PATH + ); let uri: http::Uri = stats_url .parse() .map_err(|e| format!("invalid stats URL: {e}"))?; diff --git a/package.json b/package.json index da5806c..53e8912 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@datadog/libdatadog", - "version": "0.14.0", + "version": "0.15.0", "description": "Node.js binding for libdatadog", "main": "index.js", "scripts": { diff --git a/test/http_transport.js b/test/http_transport.js index 057f5f0..5a97a58 100644 --- a/test/http_transport.js +++ b/test/http_transport.js @@ -130,6 +130,76 @@ describe('http_transport response header observer', () => { }) }) +// libdatadog derives the request host from the agent URI, which keeps the +// brackets for an IPv6 literal (`[::1]`). Node's http.request treats `host` as a +// name to resolve, so `[::1]` fails with ENOTFOUND; the transport must strip the +// brackets. Verified by connecting to an IPv6 loopback server with a bracketed +// host. Skipped where IPv6 loopback isn't available. +describe('http_transport IPv6 host', () => { + let server + let port + let ipv6Available = true + + before(async () => { + server = http.createServer((req, res) => { + req.on('data', () => {}) + req.on('end', () => res.end(RESPONSE_BODY)) + }) + try { + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '::1', resolve) + }) + port = server.address().port + } catch { + ipv6Available = false + } + }) + + after(() => new Promise(resolve => (server ? server.close(resolve) : resolve()))) + + it('strips brackets from an IPv6 host so http.request can connect', async function () { + if (!ipv6Available) return this.skip?.() + const head = Buffer.from( + `POST /v0.4/traces HTTP/1.1\r\nHost: [::1]:${port}\r\n` + + 'Content-Length: 0\r\nConnection: close\r\n\r\n', + 'utf8', + ) + // Bracketed IPv6 host, exactly as libdatadog passes it from the agent URI. + const [status] = await transport.httpRequest('[::1]', port, false, '', 0, head.length, 0, 0, fakeWasmMemory(head)) + assert.strictEqual(status, 200) + }) +}) + +// The transport must NOT require instrumentable builtins (node:http/https/fs) at +// module load: it is loaded during the tracer's own init, before user code, so +// an eager require makes dd-trace wrap the builtin in place and leaks +// instrumentation into a user app that imports it afterwards (breaks the +// dd-trace-js init/guardrail expectations). They must be required lazily, inside +// the functions that use them. +describe('http_transport lazy builtin requires', () => { + it('does not require node:http/https/fs at module load', () => { + const Module = require('node:module') + const modPath = require.resolve('../crates/capabilities/src/http_transport') + const orig = Module.prototype.require + const seen = [] + Module.prototype.require = function (id) { + seen.push(id) + return Reflect.apply(orig, this, arguments) + } + try { + delete require.cache[modPath] + require(modPath) + } finally { + Module.prototype.require = orig + delete require.cache[modPath] + } + for (const builtin of ['node:http', 'node:https', 'node:fs', 'http', 'https', 'fs']) { + assert.ok(!seen.includes(builtin), `${builtin} must not be required at module load`) + } + }) +}) + // Unix-domain-socket transport: a non-empty socketPath must route the request // over the socket instead of TCP. Skipped on Windows (no AF_UNIX path here). describe('http_transport unix socket', { skip: process.platform === 'win32' }, () => { diff --git a/test/pipeline.js b/test/pipeline.js index cad84e8..2c77389 100644 --- a/test/pipeline.js +++ b/test/pipeline.js @@ -814,6 +814,12 @@ describe('pipeline', { skip }, () => { assert(result, 'exporter returned an agent response') assert(payloads.length > 0, 'agent received a trace payload') assert(payloads[0].length > 0, 'trace payload is non-empty') + // process_span stamps the `language` meta at flush. For the Node tracer + // it must be "javascript" (matching the JS pipeline), NOT the `nodejs` + // header/tracer-lang. The v0.4 span payload has no other nodejs/javascript + // string, so a byte check unambiguously distinguishes the two. + assert(payloads[0].includes(Buffer.from('javascript')), 'span meta carries language=javascript') + assert(!payloads[0].includes(Buffer.from('nodejs')), 'span meta must not carry language=nodejs') } finally { server.closeAllConnections?.() server.close()