Skip to content

Commit 4154867

Browse files
committed
fix(dns): preserve shorthand rrtype with options (#9624)
* fix(dns): preserve shorthand rrtype with options DNS shorthand calls with an options object were captured as [hostname, options, rrtype], so the plugin read the options object and fell back to A even when Node performed an AAAA query. Keep the synthetic record type in the plugin context slot while preserving the original options. Fixes: #9621 * fix(dns): instrument CAA and TLSA resolve shorthands ## Summary Node 18 exposes resolveCaa and newer releases expose resolveTlsa, but both were absent from the shorthand table and emitted no DNS spans. ## Why In-place argument shifting measured 15.4 ns/context versus 32.2 ns/context for splice on Node 24.18.0 / V8 13.6 (two seven-trial runs, dropping the best and worst). ## Test plan - packages/datadog-plugin-dns/test/index.spec.js Refs: #9621
1 parent 7c4e309 commit 4154867

2 files changed

Lines changed: 156 additions & 4 deletions

File tree

packages/datadog-instrumentations/src/dns.js

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const rrtypes = {
99
resolveAny: 'ANY',
1010
resolve4: 'A',
1111
resolve6: 'AAAA',
12+
resolveCaa: 'CAA',
1213
resolveCname: 'CNAME',
1314
resolveMx: 'MX',
1415
resolveNs: 'NS',
@@ -17,6 +18,7 @@ const rrtypes = {
1718
resolvePtr: 'PTR',
1819
resolveNaptr: 'NAPTR',
1920
resolveSoa: 'SOA',
21+
resolveTlsa: 'TLSA',
2022
}
2123

2224
// `dns.promises` and `require('dns/promises')` resolve to the same exports object. Both
@@ -81,7 +83,7 @@ function buildCallbackArgsContext (rrtype) {
8183
const captured = [...args]
8284
captured.pop() // remove the callback
8385
if (rrtype) {
84-
captured.push(rrtype)
86+
insertRrtype(captured, rrtype)
8587
}
8688
return { args: captured }
8789
}
@@ -91,8 +93,25 @@ function buildPromiseArgsContext (rrtype) {
9193
return function (_, args) {
9294
const captured = [...args]
9395
if (rrtype) {
94-
captured.push(rrtype)
96+
insertRrtype(captured, rrtype)
9597
}
9698
return { args: captured }
9799
}
98100
}
101+
102+
/**
103+
* @param {unknown[]} captured
104+
* @param {string} rrtype
105+
*/
106+
function insertRrtype (captured, rrtype) {
107+
if (captured.length === 0) {
108+
captured.push(rrtype)
109+
return
110+
}
111+
112+
captured.push(rrtype)
113+
for (let index = captured.length - 1; index > 1; index--) {
114+
captured[index] = captured[index - 1]
115+
}
116+
captured[1] = rrtype
117+
}

packages/datadog-plugin-dns/test/index.spec.js

Lines changed: 135 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,71 @@ describe('Plugin', () => {
163163
dns.resolveAny('localhost', err => err && done(err))
164164
})
165165

166+
it('should preserve the shorthand rrtype when callback options are passed', () => {
167+
const tracePromise = agent.assertSomeTraces(traces => {
168+
assertObjectContains(traces[0][0], {
169+
name: 'dns.resolve',
170+
service: 'test',
171+
resource: 'AAAA fakedomain.faketld',
172+
})
173+
assertObjectContains(traces[0][0].meta, {
174+
component: 'dns',
175+
'span.kind': 'client',
176+
'dns.hostname': 'fakedomain.faketld',
177+
'dns.rrtype': 'AAAA',
178+
})
179+
})
180+
181+
return Promise.all([
182+
tracePromise,
183+
assert.rejects(promisify(dns.resolve6)('fakedomain.faketld', { ttl: true })),
184+
])
185+
})
186+
187+
it('should instrument resolveCaa with options', () => {
188+
const tracePromise = agent.assertSomeTraces(traces => {
189+
assertObjectContains(traces[0][0], {
190+
name: 'dns.resolve',
191+
service: 'test',
192+
resource: 'CAA fakedomain.faketld',
193+
})
194+
assertObjectContains(traces[0][0].meta, {
195+
component: 'dns',
196+
'span.kind': 'client',
197+
'dns.hostname': 'fakedomain.faketld',
198+
'dns.rrtype': 'CAA',
199+
})
200+
})
201+
202+
return Promise.all([
203+
tracePromise,
204+
assert.rejects(promisify(dns.resolveCaa)('fakedomain.faketld', { ttl: true })),
205+
])
206+
})
207+
208+
it('should preserve the shorthand rrtype on callback Resolver instances when options are passed', () => {
209+
const resolver = new dns.Resolver()
210+
211+
const tracePromise = agent.assertSomeTraces(traces => {
212+
assertObjectContains(traces[0][0], {
213+
name: 'dns.resolve',
214+
service: 'test',
215+
resource: 'AAAA fakedomain.faketld',
216+
})
217+
assertObjectContains(traces[0][0].meta, {
218+
component: 'dns',
219+
'span.kind': 'client',
220+
'dns.hostname': 'fakedomain.faketld',
221+
'dns.rrtype': 'AAAA',
222+
})
223+
})
224+
225+
return Promise.all([
226+
tracePromise,
227+
assert.rejects(promisify(resolver.resolve6).call(resolver, 'fakedomain.faketld', { ttl: true })),
228+
])
229+
})
230+
166231
it('should instrument reverse', done => {
167232
agent
168233
.assertSomeTraces(traces => {
@@ -377,6 +442,75 @@ describe('Plugin', () => {
377442
])
378443
})
379444

445+
it('should preserve the shorthand rrtype when promise options are passed', () => {
446+
const tracePromise = agent.assertSomeTraces(traces => {
447+
assertObjectContains(traces[0][0], {
448+
name: 'dns.resolve',
449+
service: 'test',
450+
resource: 'AAAA fakedomain.faketld',
451+
})
452+
assertObjectContains(traces[0][0].meta, {
453+
component: 'dns',
454+
'span.kind': 'client',
455+
'dns.hostname': 'fakedomain.faketld',
456+
'dns.rrtype': 'AAAA',
457+
})
458+
})
459+
460+
return Promise.all([
461+
tracePromise,
462+
assert.rejects(dns.promises.resolve6('fakedomain.faketld', { ttl: true })),
463+
])
464+
})
465+
466+
it('should instrument resolveTlsa with options when supported', function () {
467+
if (typeof dns.promises.resolveTlsa !== 'function') {
468+
this.skip()
469+
}
470+
471+
const tracePromise = agent.assertSomeTraces(traces => {
472+
assertObjectContains(traces[0][0], {
473+
name: 'dns.resolve',
474+
service: 'test',
475+
resource: 'TLSA fakedomain.faketld',
476+
})
477+
assertObjectContains(traces[0][0].meta, {
478+
component: 'dns',
479+
'span.kind': 'client',
480+
'dns.hostname': 'fakedomain.faketld',
481+
'dns.rrtype': 'TLSA',
482+
})
483+
})
484+
485+
return Promise.all([
486+
tracePromise,
487+
assert.rejects(dns.promises.resolveTlsa('fakedomain.faketld', { ttl: true })),
488+
])
489+
})
490+
491+
it('should preserve the shorthand rrtype on promise Resolver instances when options are passed', () => {
492+
const resolver = new dns.promises.Resolver()
493+
494+
const tracePromise = agent.assertSomeTraces(traces => {
495+
assertObjectContains(traces[0][0], {
496+
name: 'dns.resolve',
497+
service: 'test',
498+
resource: 'AAAA fakedomain.faketld',
499+
})
500+
assertObjectContains(traces[0][0].meta, {
501+
component: 'dns',
502+
'span.kind': 'client',
503+
'dns.hostname': 'fakedomain.faketld',
504+
'dns.rrtype': 'AAAA',
505+
})
506+
})
507+
508+
return Promise.all([
509+
tracePromise,
510+
assert.rejects(resolver.resolve6('fakedomain.faketld', { ttl: true })),
511+
])
512+
})
513+
380514
it('should instrument reverse', () => {
381515
const tracePromise = agent.assertSomeTraces(traces => {
382516
assertObjectContains(traces[0][0], {
@@ -407,9 +541,8 @@ describe('Plugin', () => {
407541
})
408542

409543
it('should rethrow synchronous errors from the underlying call', () => {
410-
// dns.promises.lookup validates `hostname` synchronously and throws ERR_INVALID_ARG_TYPE
411-
// rather than returning a rejected promise; the wrapper must propagate that.
412544
assert.throws(() => dns.promises.lookup({}), { code: 'ERR_INVALID_ARG_TYPE' })
545+
assert.throws(() => dns.promises.resolve6(), { code: 'ERR_INVALID_ARG_TYPE' })
413546
})
414547

415548
it('should instrument Resolver instances', () => {

0 commit comments

Comments
 (0)