feat(dns): Add option to include name as attribute in dns lookup

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
 

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',
 }

plugins/node/opentelemetry-instrumentation-dns/src/instrumentation.ts

@@ -32,13 +32,22 @@ import {
 } from './types';
 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<Dns> {
   constructor(protected override _config: DnsInstrumentationConfig = {}) {
-    super('@opentelemetry/instrumentation-dns', VERSION, _config);
+    super(
+      '@opentelemetry/instrumentation-dns',
+      VERSION,
+      Object.assign({}, DEFAULT_CONFIG, _config)
+    );
   }
 
   init(): InstrumentationNodeModuleDefinition<Dns>[] {
@@ -110,8 +119,17 @@ export class DnsInstrumentation extends InstrumentationBase<Dns> {
       const argsCount = args.length;
       diag.debug('wrap lookup callback function and starts span');
       const name = utils.getOperationName('lookup');
+      let opts = null;
+      if (!plugin._config?.omitHostnameAttribute) {
+        opts = {
+          attributes: {
+            [AttributeNames.DNS_HOSTNAME]: hostname,
+          },
+        };
+      }
       const span = plugin.tracer.startSpan(name, {
         kind: SpanKind.CLIENT,
+        ...opts,
       });
 
       const originalCallback = args[argsCount - 1];

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[];
+
+  /** Omit adding hostname of target as span attribute, default is false */
+  omitHostnameAttribute?: boolean;
 }

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();
     });
@@ -114,11 +114,11 @@ describe('dns.lookup()', () => {
           assert.strictEqual(spans.length, 1);
           assertSpan(span, {
             addresses: [{ address, family }],
-            hostname,
             forceStatus: {
               code: SpanStatusCode.ERROR,
               message: err!.message,
             },
+            hostname,
           });
           done();
         });
@@ -136,11 +136,11 @@ describe('dns.lookup()', () => {
         assert.strictEqual(spans.length, 1);
         assertSpan(span, {
           addresses: [],
-          hostname,
           forceStatus: {
             code: SpanStatusCode.ERROR,
             message: error!.message,
           },
+          hostname,
         });
       }
     });
@@ -157,16 +157,58 @@ 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,
           },
+          hostname: hostname as any,
         });
       }
     });
+
+    describe('dns.hostname attribute', () => {
+      afterEach(() => {
+        instrumentation.setConfig();
+      });
+
+      it('should be included 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,
+          });
+          done();
+        });
+      });
+
+      it('should be excluded if omitHostname is true', done => {
+        const hostname = 'google.com';
+        instrumentation.setConfig({
+          omitHostnameAttribute: 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: undefined,
+          });
+          done();
+        });
+      });
+    });
   });
+
   describe('with options param', () => {
     [4, 6].forEach(family => {
       it(`should export a valid span with "family" to ${family}`, done => {

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';
@@ -96,14 +96,14 @@ describe('dns.promises.lookup()', () => {
     });
 
     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 dns.promises.lookup(hostname);
+          await dns.promises.lookup(unresolvableHostname);
           assert.fail();
         } catch (error) {
           const spans = memoryExporter.getFinishedSpans();
@@ -112,11 +112,11 @@ describe('dns.promises.lookup()', () => {
           assert.strictEqual(spans.length, 1);
           assertSpan(span, {
             addresses: [],
-            hostname,
             forceStatus: {
               code: SpanStatusCode.ERROR,
               message: error!.message,
             },
+            hostname: unresolvableHostname,
           });
         }
       });
@@ -134,12 +134,11 @@ 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,
           },
+          hostname,
         });
       }
     });
@@ -156,16 +155,63 @@ 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,
           },
+          hostname: hostname as any,
         });
       }
     });
+
+    describe('dns.hostname attribute', () => {
+      afterEach(() => {
+        instrumentation.setConfig();
+      });
+
+      it('should be included by default', async () => {
+        const hostname = 'google.com';
+        const config: DnsInstrumentationConfig = {};
+        instrumentation.setConfig(config);
+
+        try {
+          await dns.promises.lookup(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,
+          });
+        }
+      });
+
+      it('should be excluded if omitHostname is true', async () => {
+        const hostname = 'google.com';
+        instrumentation.setConfig({
+          omitHostnameAttribute: true,
+        } as DnsInstrumentationConfig);
+
+        try {
+          await dns.promises.lookup(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,
+          });
+        }
+      });
+    });
   });
+
   describe('with options param', () => {
     [4, 6].forEach(ipversion => {
       it(`should export a valid span with "family" to ${ipversion}`, async () => {

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,15 @@ export const assertSpan = (
     );
   });
 
+  if (validations.hostname !== undefined) {
+    assert.strictEqual(
+      span.attributes[AttributeNames.DNS_HOSTNAME],
+      validations.hostname
+    );
+  } else {
+    assert.strictEqual(span.attributes[AttributeNames.DNS_HOSTNAME], undefined);
+  }
+
   assert.ok(span.endTime);
   assert.strictEqual(span.links.length, 0);
   assert.strictEqual(span.events.length, 0);