From dc7f2832d9d0c3da461c87823148bf266f9d80ca Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Sun, 11 Sep 2022 12:26:14 -0700 Subject: [PATCH 1/5] Introduce addHostnameAttribute to dns.instrumentation Defaults to false for compatibility --- .../src/enums/AttributeNames.ts | 1 + .../src/instrumentation.ts | 21 +++++- .../src/types.ts | 4 ++ .../test/integrations/dns-lookup.test.ts | 56 +++++++++++++--- .../integrations/dnspromise-lookup.test.ts | 64 +++++++++++++++---- .../test/utils/assertSpan.ts | 9 ++- 6 files changed, 128 insertions(+), 27 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-dns/src/enums/AttributeNames.ts b/plugins/node/opentelemetry-instrumentation-dns/src/enums/AttributeNames.ts index 2364b3769c..39b9a467bf 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/src/enums/AttributeNames.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/src/enums/AttributeNames.ts @@ -18,4 +18,5 @@ export enum AttributeNames { DNS_ERROR_CODE = 'dns.error_code', DNS_ERROR_NAME = 'dns.error_name', DNS_ERROR_MESSAGE = 'dns.error_message', + DNS_HOSTNAME = 'dns.hostname', } diff --git a/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts index d47cf7565b..f12a724c39 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts @@ -32,6 +32,7 @@ import { } from './types'; import * as utils from './utils'; import { VERSION } from './version'; +import { AttributeNames } from './enums/AttributeNames'; /** * Dns instrumentation for Opentelemetry @@ -110,9 +111,23 @@ export class DnsInstrumentation extends InstrumentationBase { const argsCount = args.length; diag.debug('wrap lookup callback function and starts span'); const name = utils.getOperationName('lookup'); - const span = plugin.tracer.startSpan(name, { - kind: SpanKind.CLIENT, - }); + let opts = null; + if (plugin._config?.addHostnameAttribute) { + opts = { + attributes: { + [AttributeNames.DNS_HOSTNAME]: hostname, + }, + }; + } + const span = plugin.tracer.startSpan( + name, + Object.assign( + { + kind: SpanKind.CLIENT, + }, + opts + ) + ); const originalCallback = args[argsCount - 1]; if (typeof originalCallback === 'function') { diff --git a/plugins/node/opentelemetry-instrumentation-dns/src/types.ts b/plugins/node/opentelemetry-instrumentation-dns/src/types.ts index 70f4f6d54e..c563a5a2f6 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/src/types.ts @@ -96,5 +96,9 @@ export type LookupCallbackSignature = LookupSimpleCallback & ) => void); export interface DnsInstrumentationConfig extends InstrumentationConfig { + /** List of hostname matchers used to decide whether to skip creating a span */ ignoreHostnames?: IgnoreMatcher[]; + + /** Add target of dns lookup as span attribute in dns.lookup, default is false */ + addHostnameAttribute?: boolean; } diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts index 8bd5ad698a..c307fd5338 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts @@ -20,7 +20,7 @@ import { } from '@opentelemetry/sdk-trace-base'; import * as assert from 'assert'; import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; -import { DnsInstrumentation } from '../../src'; +import { DnsInstrumentation, DnsInstrumentationConfig } from '../../src'; import * as dns from 'dns'; import * as utils from '../utils/utils'; import { assertSpan } from '../utils/assertSpan'; @@ -46,7 +46,7 @@ describe('dns.lookup()', () => { utils.checkInternet(isConnected => { if (!isConnected) { this.skip(); - // don't disturbe people + // don't disturb people } done(); }); @@ -75,7 +75,7 @@ describe('dns.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }], hostname }); + assertSpan(span, { addresses: [{ address, family }] }); done(); }); }); @@ -93,7 +93,7 @@ describe('dns.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }], hostname }); + assertSpan(span, { addresses: [{ address, family }] }); done(); }); }); @@ -136,7 +136,6 @@ describe('dns.lookup()', () => { assert.strictEqual(spans.length, 1); assertSpan(span, { addresses: [], - hostname, forceStatus: { code: SpanStatusCode.ERROR, message: error!.message, @@ -157,8 +156,6 @@ describe('dns.lookup()', () => { assert.strictEqual(spans.length, 1); assertSpan(span, { addresses: [], - // tslint:disable-next-line:no-any - hostname: hostname as any, forceStatus: { code: SpanStatusCode.ERROR, message: error!.message, @@ -166,7 +163,46 @@ describe('dns.lookup()', () => { }); } }); + + it('should omit dns.hostname attribute by default', done => { + const hostname = 'google.com'; + const config: DnsInstrumentationConfig = {}; + instrumentation.setConfig(config); + + dns.lookup(hostname, (err, address, family) => { + assert.strictEqual(err, null); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + const [span] = spans; + assertSpan(span, { + addresses: [{ address, family }], + hostname: undefined, + }); + done(); + }); + }); + + it('should include dns.hostname attribute if requested', done => { + const hostname = 'google.com'; + const config: DnsInstrumentationConfig = { + addHostnameAttribute: true, + }; + instrumentation.setConfig(config); + + dns.lookup(hostname, (err, address, family) => { + assert.strictEqual(err, null); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + const [span] = spans; + assertSpan(span, { + addresses: [{ address, family }], + hostname, + }); + done(); + }); + }); }); + describe('with options param', () => { [4, 6].forEach(family => { it(`should export a valid span with "family" to ${family}`, done => { @@ -180,7 +216,7 @@ describe('dns.lookup()', () => { const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }], hostname }); + assertSpan(span, { addresses: [{ address, family }] }); done(); }); }); @@ -199,7 +235,7 @@ describe('dns.lookup()', () => { const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }], hostname }); + assertSpan(span, { addresses: [{ address, family }] }); done(); } ); @@ -218,7 +254,7 @@ describe('dns.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses, hostname }); + assertSpan(span, { addresses }); done(); } ); diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts index bb9988ee1f..892f00ec90 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts @@ -20,7 +20,7 @@ import { } from '@opentelemetry/sdk-trace-base'; import * as assert from 'assert'; import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; -import { DnsInstrumentation } from '../../src'; +import { DnsInstrumentation, DnsInstrumentationConfig } from '../../src'; import * as dns from 'dns'; import * as utils from '../utils/utils'; import { assertSpan } from '../utils/assertSpan'; @@ -92,7 +92,7 @@ describe('dns.promises.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }], hostname }); + assertSpan(span, { addresses: [{ address, family }] }); }); }); }); @@ -108,18 +108,18 @@ describe('dns.promises.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }], hostname }); + assertSpan(span, { addresses: [{ address, family }] }); }); describe('extended timeout', function () { + const unresolvableHostname = 'ᚕ'; // Extending the default timeout as some environments are taking longer than 2 seconds to fail // So rather than fail the test -- just take a little longer this.timeout(10000); it('should export a valid span with error NOT_FOUND', async () => { - const hostname = 'ᚕ'; try { - await lookupPromise(hostname); + await lookupPromise(unresolvableHostname); assert.fail(); } catch (error) { const spans = memoryExporter.getFinishedSpans(); @@ -128,7 +128,6 @@ describe('dns.promises.lookup()', () => { assert.strictEqual(spans.length, 1); assertSpan(span, { addresses: [], - hostname, forceStatus: { code: SpanStatusCode.ERROR, message: error!.message, @@ -150,8 +149,6 @@ describe('dns.promises.lookup()', () => { assert.strictEqual(spans.length, 1); assertSpan(span, { addresses: [], - // tslint:disable-next-line:no-any - hostname: hostname as any, forceStatus: { code: SpanStatusCode.ERROR, message: error!.message, @@ -173,8 +170,6 @@ describe('dns.promises.lookup()', () => { assert.strictEqual(spans.length, 1); assertSpan(span, { addresses: [], - // tslint:disable-next-line:no-any - hostname: hostname as any, forceStatus: { code: SpanStatusCode.ERROR, message: error!.message, @@ -182,7 +177,50 @@ describe('dns.promises.lookup()', () => { }); } }); + + it('should omit dns.hostname attribute by default', async () => { + const hostname = 'google.com'; + const config: DnsInstrumentationConfig = {}; + instrumentation.setConfig(config); + + try { + await lookupPromise(hostname as any, { family: 4 }); + assert.fail(); + } catch (error) { + const spans = memoryExporter.getFinishedSpans(); + const [span] = spans; + + assert.strictEqual(spans.length, 1); + assertSpan(span, { + addresses: [], + hostname: undefined, + }); + } + }); + + it('should include dns.hostname attribute if configured', async () => { + const hostname = 'google.com'; + const config: DnsInstrumentationConfig = { + addHostnameAttribute: true, + }; + instrumentation.setConfig(config); + + try { + await lookupPromise(hostname as any, { family: 4 }); + assert.fail(); + } catch (error) { + const spans = memoryExporter.getFinishedSpans(); + const [span] = spans; + + assert.strictEqual(spans.length, 1); + assertSpan(span, { + addresses: [], + hostname, + }); + } + }); }); + describe('with options param', () => { [4, 6].forEach(ipversion => { it(`should export a valid span with "family" to ${ipversion}`, async () => { @@ -198,7 +236,7 @@ describe('dns.promises.lookup()', () => { const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }], hostname }); + assertSpan(span, { addresses: [{ address, family }] }); }); it(`should export a valid span when setting "verbatim" property to true and "family" to ${ipversion}`, async () => { @@ -215,7 +253,7 @@ describe('dns.promises.lookup()', () => { const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }], hostname }); + assertSpan(span, { addresses: [{ address, family }] }); }); }); @@ -228,7 +266,7 @@ describe('dns.promises.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses, hostname }); + assertSpan(span, { addresses }); }); }); }); diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/utils/assertSpan.ts b/plugins/node/opentelemetry-instrumentation-dns/test/utils/assertSpan.ts index 15faefa34a..90473bc78c 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/utils/assertSpan.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/utils/assertSpan.ts @@ -26,7 +26,7 @@ export const assertSpan = ( span: ReadableSpan, validations: { addresses: LookupAddress[]; - hostname: string; + hostname?: string; forceStatus?: SpanStatus; } ) => { @@ -51,6 +51,13 @@ export const assertSpan = ( ); }); + if (validations.hostname !== undefined) { + assert.strictEqual( + span.attributes[AttributeNames.DNS_HOSTNAME], + validations.hostname + ); + } + assert.ok(span.endTime); assert.strictEqual(span.links.length, 0); assert.strictEqual(span.events.length, 0); From 55c81e71f316e288d4b5d641eb5bea08002eff6d Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Thu, 15 Sep 2022 09:48:17 -0700 Subject: [PATCH 2/5] Change configuration setting to `includeHostname` --- .../opentelemetry-instrumentation-dns/src/instrumentation.ts | 2 +- plugins/node/opentelemetry-instrumentation-dns/src/types.ts | 2 +- .../test/integrations/dns-lookup.test.ts | 2 +- .../test/integrations/dnspromise-lookup.test.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts index f12a724c39..2d9e071e64 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts @@ -112,7 +112,7 @@ export class DnsInstrumentation extends InstrumentationBase { diag.debug('wrap lookup callback function and starts span'); const name = utils.getOperationName('lookup'); let opts = null; - if (plugin._config?.addHostnameAttribute) { + if (plugin._config?.includeHostname) { opts = { attributes: { [AttributeNames.DNS_HOSTNAME]: hostname, diff --git a/plugins/node/opentelemetry-instrumentation-dns/src/types.ts b/plugins/node/opentelemetry-instrumentation-dns/src/types.ts index c563a5a2f6..5a0ac3cd2d 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/src/types.ts @@ -100,5 +100,5 @@ export interface DnsInstrumentationConfig extends InstrumentationConfig { ignoreHostnames?: IgnoreMatcher[]; /** Add target of dns lookup as span attribute in dns.lookup, default is false */ - addHostnameAttribute?: boolean; + includeHostname?: boolean; } diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts index c307fd5338..c05f552a60 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts @@ -185,7 +185,7 @@ describe('dns.lookup()', () => { it('should include dns.hostname attribute if requested', done => { const hostname = 'google.com'; const config: DnsInstrumentationConfig = { - addHostnameAttribute: true, + includeHostname: true, }; instrumentation.setConfig(config); diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts index 892f00ec90..004668a213 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts @@ -201,7 +201,7 @@ describe('dns.promises.lookup()', () => { it('should include dns.hostname attribute if configured', async () => { const hostname = 'google.com'; const config: DnsInstrumentationConfig = { - addHostnameAttribute: true, + includeHostname: true, }; instrumentation.setConfig(config); From 504480138e44b72bbe7d1b0ea82361f88faea185 Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Sat, 8 Oct 2022 18:37:46 -0700 Subject: [PATCH 3/5] fix(dns): correct check for empty dns.hostname attribute --- .../test/integrations/dns-lookup.test.ts | 63 ++++++++-------- .../integrations/dnspromise-lookup.test.ts | 75 ++++++++++--------- .../test/utils/assertSpan.ts | 2 + 3 files changed, 75 insertions(+), 65 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts index c05f552a60..1272ad3d40 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts @@ -114,7 +114,6 @@ describe('dns.lookup()', () => { assert.strictEqual(spans.length, 1); assertSpan(span, { addresses: [{ address, family }], - hostname, forceStatus: { code: SpanStatusCode.ERROR, message: err!.message, @@ -164,41 +163,45 @@ describe('dns.lookup()', () => { } }); - it('should omit dns.hostname attribute by default', done => { - const hostname = 'google.com'; - const config: DnsInstrumentationConfig = {}; - instrumentation.setConfig(config); + describe('dns.hostname attribute', () => { + afterEach(() => { + instrumentation.setConfig(); + }); - dns.lookup(hostname, (err, address, family) => { - assert.strictEqual(err, null); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 1); - const [span] = spans; - assertSpan(span, { - addresses: [{ address, family }], - hostname: undefined, + it('should be omitted by default', done => { + const hostname = 'google.com'; + instrumentation.setConfig(); + + dns.lookup(hostname, (err, address, family) => { + assert.strictEqual(err, null); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + const [span] = spans; + assertSpan(span, { + addresses: [{ address, family }], + hostname: undefined, + }); + done(); }); - done(); }); - }); - it('should include dns.hostname attribute if requested', done => { - const hostname = 'google.com'; - const config: DnsInstrumentationConfig = { - includeHostname: true, - }; - instrumentation.setConfig(config); + it('should be included if includeHostname is true', done => { + const hostname = 'google.com'; + instrumentation.setConfig({ + includeHostname: true, + } as DnsInstrumentationConfig); - dns.lookup(hostname, (err, address, family) => { - assert.strictEqual(err, null); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 1); - const [span] = spans; - assertSpan(span, { - addresses: [{ address, family }], - hostname, + dns.lookup(hostname, (err, address, family) => { + assert.strictEqual(err, null); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + const [span] = spans; + assertSpan(span, { + addresses: [{ address, family }], + hostname, + }); + done(); }); - done(); }); }); }); diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts index 004668a213..1372eacaa7 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts @@ -178,46 +178,51 @@ describe('dns.promises.lookup()', () => { } }); - it('should omit dns.hostname attribute by default', async () => { - const hostname = 'google.com'; - const config: DnsInstrumentationConfig = {}; - instrumentation.setConfig(config); + describe('dns.hostname attribute', () => { + afterEach(() => { + instrumentation.setConfig(); + }); - try { - await lookupPromise(hostname as any, { family: 4 }); - assert.fail(); - } catch (error) { - const spans = memoryExporter.getFinishedSpans(); - const [span] = spans; + it('should be omitted by default', async () => { + const hostname = 'google.com'; + const config: DnsInstrumentationConfig = {}; + instrumentation.setConfig(config); - assert.strictEqual(spans.length, 1); - assertSpan(span, { - addresses: [], - hostname: undefined, - }); - } - }); + try { + await lookupPromise(hostname as any, { family: 4 }); + assert.fail(); + } catch (error) { + const spans = memoryExporter.getFinishedSpans(); + const [span] = spans; - it('should include dns.hostname attribute if configured', async () => { - const hostname = 'google.com'; - const config: DnsInstrumentationConfig = { - includeHostname: true, - }; - instrumentation.setConfig(config); + assert.strictEqual(spans.length, 1); + assertSpan(span, { + addresses: [], + hostname: undefined, + }); + } + }); - try { - await lookupPromise(hostname as any, { family: 4 }); - assert.fail(); - } catch (error) { - const spans = memoryExporter.getFinishedSpans(); - const [span] = spans; + it('should be included if includeHostname is true', async () => { + const hostname = 'google.com'; + instrumentation.setConfig({ + includeHostname: true, + } as DnsInstrumentationConfig); - assert.strictEqual(spans.length, 1); - assertSpan(span, { - addresses: [], - hostname, - }); - } + try { + await lookupPromise(hostname as any, { family: 4 }); + assert.fail(); + } catch (error) { + const spans = memoryExporter.getFinishedSpans(); + const [span] = spans; + + assert.strictEqual(spans.length, 1); + assertSpan(span, { + addresses: [], + hostname, + }); + } + }); }); }); diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/utils/assertSpan.ts b/plugins/node/opentelemetry-instrumentation-dns/test/utils/assertSpan.ts index 90473bc78c..472a1143ce 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/utils/assertSpan.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/utils/assertSpan.ts @@ -56,6 +56,8 @@ export const assertSpan = ( span.attributes[AttributeNames.DNS_HOSTNAME], validations.hostname ); + } else { + assert.strictEqual(span.attributes[AttributeNames.DNS_HOSTNAME], undefined); } assert.ok(span.endTime); From 4af465f93b484ffe488fbbbefd0070c04c7d6407 Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Sun, 9 Oct 2022 13:37:03 -0700 Subject: [PATCH 4/5] fix(dns): flip includeHostname to omitHostnameAttribute Default to false i.e. dns.hostname hostname will be included on any span created for `dns.lookup()` or `dns.promises.lookup()` --- .../README.md | 5 ++-- .../src/instrumentation.ts | 25 +++++++++++-------- .../src/types.ts | 4 +-- .../test/integrations/dns-lookup.test.ts | 23 +++++++++-------- .../integrations/dnspromise-lookup.test.ts | 23 +++++++++-------- 5 files changed, 45 insertions(+), 35 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-dns/README.md b/plugins/node/opentelemetry-instrumentation-dns/README.md index 565a071455..b0fbed4c40 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/README.md +++ b/plugins/node/opentelemetry-instrumentation-dns/README.md @@ -36,11 +36,12 @@ registerInstrumentations({ ### Dns Instrumentation Options -Dns instrumentation has currently one option. You can set the following: +Dns instrumentation currently has two options. You can set the following: | Options | Type | Description | | ------- | ---- | ----------- | -| [`ignoreHostnames`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-dns/src/types.ts#L99) | `IgnoreMatcher[]` | Dns instrumentation will not trace all requests that match hostnames | +| [`ignoreHostnames`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-dns/src/types.ts#L99) | `IgnoreMatcher[]` | Dns instrumentation will exclude creating spans for requests that match hostnames | +| [`omitHostnameAttribute`] | `boolean` | Dns instrumentation will exclude the `dns.hostname` attribute from spans, default when unset is false | ## Useful links diff --git a/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts index 2d9e071e64..f830d364ab 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts @@ -34,12 +34,20 @@ import * as utils from './utils'; import { VERSION } from './version'; import { AttributeNames } from './enums/AttributeNames'; +const DEFAULT_CONFIG: DnsInstrumentationConfig = { + omitHostnameAttribute: false, +}; + /** * Dns instrumentation for Opentelemetry */ export class DnsInstrumentation extends InstrumentationBase { constructor(protected override _config: DnsInstrumentationConfig = {}) { - super('@opentelemetry/instrumentation-dns', VERSION, _config); + super( + '@opentelemetry/instrumentation-dns', + VERSION, + Object.assign({}, DEFAULT_CONFIG, _config) + ); } init(): InstrumentationNodeModuleDefinition[] { @@ -112,22 +120,17 @@ export class DnsInstrumentation extends InstrumentationBase { diag.debug('wrap lookup callback function and starts span'); const name = utils.getOperationName('lookup'); let opts = null; - if (plugin._config?.includeHostname) { + if (!plugin._config?.omitHostnameAttribute) { opts = { attributes: { [AttributeNames.DNS_HOSTNAME]: hostname, }, }; } - const span = plugin.tracer.startSpan( - name, - Object.assign( - { - kind: SpanKind.CLIENT, - }, - opts - ) - ); + const span = plugin.tracer.startSpan(name, { + kind: SpanKind.CLIENT, + ...opts, + }); const originalCallback = args[argsCount - 1]; if (typeof originalCallback === 'function') { diff --git a/plugins/node/opentelemetry-instrumentation-dns/src/types.ts b/plugins/node/opentelemetry-instrumentation-dns/src/types.ts index 5a0ac3cd2d..591c687d69 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/src/types.ts @@ -99,6 +99,6 @@ export interface DnsInstrumentationConfig extends InstrumentationConfig { /** List of hostname matchers used to decide whether to skip creating a span */ ignoreHostnames?: IgnoreMatcher[]; - /** Add target of dns lookup as span attribute in dns.lookup, default is false */ - includeHostname?: boolean; + /** Omit adding hostname of target as span attribute, default is false */ + omitHostnameAttribute?: boolean; } diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts index 1272ad3d40..2c55809775 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dns-lookup.test.ts @@ -75,7 +75,7 @@ describe('dns.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }] }); + assertSpan(span, { addresses: [{ address, family }], hostname }); done(); }); }); @@ -93,7 +93,7 @@ describe('dns.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }] }); + assertSpan(span, { addresses: [{ address, family }], hostname }); done(); }); }); @@ -118,6 +118,7 @@ describe('dns.lookup()', () => { code: SpanStatusCode.ERROR, message: err!.message, }, + hostname, }); done(); }); @@ -139,6 +140,7 @@ describe('dns.lookup()', () => { code: SpanStatusCode.ERROR, message: error!.message, }, + hostname, }); } }); @@ -159,6 +161,7 @@ describe('dns.lookup()', () => { code: SpanStatusCode.ERROR, message: error!.message, }, + hostname: hostname as any, }); } }); @@ -168,7 +171,7 @@ describe('dns.lookup()', () => { instrumentation.setConfig(); }); - it('should be omitted by default', done => { + it('should be included by default', done => { const hostname = 'google.com'; instrumentation.setConfig(); @@ -179,16 +182,16 @@ describe('dns.lookup()', () => { const [span] = spans; assertSpan(span, { addresses: [{ address, family }], - hostname: undefined, + hostname, }); done(); }); }); - it('should be included if includeHostname is true', done => { + it('should be excluded if omitHostname is true', done => { const hostname = 'google.com'; instrumentation.setConfig({ - includeHostname: true, + omitHostnameAttribute: true, } as DnsInstrumentationConfig); dns.lookup(hostname, (err, address, family) => { @@ -198,7 +201,7 @@ describe('dns.lookup()', () => { const [span] = spans; assertSpan(span, { addresses: [{ address, family }], - hostname, + hostname: undefined, }); done(); }); @@ -219,7 +222,7 @@ describe('dns.lookup()', () => { const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }] }); + assertSpan(span, { addresses: [{ address, family }], hostname }); done(); }); }); @@ -238,7 +241,7 @@ describe('dns.lookup()', () => { const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }] }); + assertSpan(span, { addresses: [{ address, family }], hostname }); done(); } ); @@ -257,7 +260,7 @@ describe('dns.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses }); + assertSpan(span, { addresses, hostname }); done(); } ); diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts index 1372eacaa7..ada4d9ca39 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts @@ -92,7 +92,7 @@ describe('dns.promises.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }] }); + assertSpan(span, { addresses: [{ address, family }], hostname }); }); }); }); @@ -108,7 +108,7 @@ describe('dns.promises.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }] }); + assertSpan(span, { addresses: [{ address, family }], hostname }); }); describe('extended timeout', function () { @@ -132,6 +132,7 @@ describe('dns.promises.lookup()', () => { code: SpanStatusCode.ERROR, message: error!.message, }, + hostname: unresolvableHostname, }); } }); @@ -153,6 +154,7 @@ describe('dns.promises.lookup()', () => { code: SpanStatusCode.ERROR, message: error!.message, }, + hostname, }); } }); @@ -174,6 +176,7 @@ describe('dns.promises.lookup()', () => { code: SpanStatusCode.ERROR, message: error!.message, }, + hostname: hostname as any, }); } }); @@ -183,7 +186,7 @@ describe('dns.promises.lookup()', () => { instrumentation.setConfig(); }); - it('should be omitted by default', async () => { + it('should be included by default', async () => { const hostname = 'google.com'; const config: DnsInstrumentationConfig = {}; instrumentation.setConfig(config); @@ -198,15 +201,15 @@ describe('dns.promises.lookup()', () => { assert.strictEqual(spans.length, 1); assertSpan(span, { addresses: [], - hostname: undefined, + hostname, }); } }); - it('should be included if includeHostname is true', async () => { + it('should be excluded if omitHostname is true', async () => { const hostname = 'google.com'; instrumentation.setConfig({ - includeHostname: true, + omitHostnameAttribute: true, } as DnsInstrumentationConfig); try { @@ -219,7 +222,7 @@ describe('dns.promises.lookup()', () => { assert.strictEqual(spans.length, 1); assertSpan(span, { addresses: [], - hostname, + hostname: undefined, }); } }); @@ -241,7 +244,7 @@ describe('dns.promises.lookup()', () => { const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }] }); + assertSpan(span, { addresses: [{ address, family }], hostname }); }); it(`should export a valid span when setting "verbatim" property to true and "family" to ${ipversion}`, async () => { @@ -258,7 +261,7 @@ describe('dns.promises.lookup()', () => { const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses: [{ address, family }] }); + assertSpan(span, { addresses: [{ address, family }], hostname }); }); }); @@ -271,7 +274,7 @@ describe('dns.promises.lookup()', () => { const spans = memoryExporter.getFinishedSpans(); const [span] = spans; assert.strictEqual(spans.length, 1); - assertSpan(span, { addresses }); + assertSpan(span, { addresses, hostname }); }); }); }); From bfa6cae73d80f542adb8b0c8212fa0965bdc0233 Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Mon, 10 Oct 2022 10:23:56 -0700 Subject: [PATCH 5/5] fixup! Merge remote-tracking branch 'upstream/main' into dns.hostname --- .../test/integrations/dnspromise-lookup.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts index 71e870b998..860861dca3 100644 --- a/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts +++ b/plugins/node/opentelemetry-instrumentation-dns/test/integrations/dnspromise-lookup.test.ts @@ -175,7 +175,7 @@ describe('dns.promises.lookup()', () => { instrumentation.setConfig(config); try { - await lookupPromise(hostname as any, { family: 4 }); + await dns.promises.lookup(hostname as any, { family: 4 }); assert.fail(); } catch (error) { const spans = memoryExporter.getFinishedSpans(); @@ -196,7 +196,7 @@ describe('dns.promises.lookup()', () => { } as DnsInstrumentationConfig); try { - await lookupPromise(hostname as any, { family: 4 }); + await dns.promises.lookup(hostname as any, { family: 4 }); assert.fail(); } catch (error) { const spans = memoryExporter.getFinishedSpans();