Merge branch 'main' into ritm-singleton-core-subpaths

open-telemetry · Dec 5, 2022 · 8905ad0 · 8905ad0
2 parents 92de678 + 6ea4b41
commit 8905ad0
Show file tree

Hide file tree

Showing 10 changed files with 194 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 ### :rocket: (Enhancement)
 
 * feat(api): add `getActiveBaggage` API [#3385](https://github.com/open-telemetry/opentelemetry-js/pull/3385)
+* feat(instrumentation-grpc): set net.peer.name and net.peer.port on client spans [#3430](https://github.com/open-telemetry/opentelemetry-js/pull/3430)
 
 ### :bug: (Bug Fix)
 

diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -17,6 +17,7 @@ All notable changes to experimental packages in this project will be documented
 * fix(instrumentation-xhr): http.url attribute should be absolute [#3200](https://github.com/open-telemetry/opentelemetry-js/pull/3200) @t2t2
 * fix(instrumentation-grpc): always set grpc semcov status code attribute with numeric value [#3076](https://github.com/open-telemetry/opentelemetry-js/pull/3076) @blumamir
 * fix(instrumentation): only call `onRequire` for full matches on core modules with sub-paths [#3451](https://github.com/open-telemetry/opentelemetry-js/pull/3451) @mhassan1
+* fix(instrumentation): add back support for absolute paths via `require-in-the-middle` [#3457](https://github.com/open-telemetry/opentelemetry-js/pull/3457) @mhassan1
 
 ### :books: (Refine Doc)
 

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts
@@ -49,7 +49,7 @@ import {
   getMetadata,
 } from './clientUtils';
 import { EventEmitter } from 'events';
-import { _extractMethodAndService, metadataCapture } from '../utils';
+import { _extractMethodAndService, metadataCapture, URI_REGEX } from '../utils';
 import { AttributeValues } from '../enums/AttributeValues';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 
@@ -309,6 +309,18 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
             [SemanticAttributes.RPC_METHOD]: method,
             [SemanticAttributes.RPC_SERVICE]: service,
           });
+        // set net.peer.* from target (e.g., "dns:otel-productcatalogservice:8080") as a hint to APMs
+        const parsedUri = URI_REGEX.exec(this.getChannel().getTarget());
+        if (parsedUri != null && parsedUri.groups != null) {
+          span.setAttribute(
+            SemanticAttributes.NET_PEER_NAME,
+            parsedUri.groups['name']
+          );
+          span.setAttribute(
+            SemanticAttributes.NET_PEER_PORT,
+            parseInt(parsedUri.groups['port'])
+          );
+        }
 
         instrumentation._metadataCapture.client.captureRequestMetadata(
           span,

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts
@@ -45,6 +45,7 @@ import {
   _extractMethodAndService,
   _methodIsIgnored,
   metadataCapture,
+  URI_REGEX,
 } from '../utils';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import { AttributeValues } from '../enums/AttributeValues';
@@ -321,6 +322,18 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
             [SemanticAttributes.RPC_METHOD]: method,
             [SemanticAttributes.RPC_SERVICE]: service,
           });
+        // set net.peer.* from target (e.g., "dns:otel-productcatalogservice:8080") as a hint to APMs
+        const parsedUri = URI_REGEX.exec(this.getChannel().getTarget());
+        if (parsedUri != null && parsedUri.groups != null) {
+          span.setAttribute(
+            SemanticAttributes.NET_PEER_NAME,
+            parsedUri.groups['name']
+          );
+          span.setAttribute(
+            SemanticAttributes.NET_PEER_PORT,
+            parseInt(parsedUri.groups['port'])
+          );
+        }
 
         instrumentation._metadataCapture.client.captureRequestMetadata(
           span,

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/utils.ts
@@ -19,6 +19,10 @@ import type * as grpcTypes from 'grpc';
 import type * as grpcJsTypes from '@grpc/grpc-js';
 import { IgnoreMatcher } from './types';
 
+// e.g., "dns:otel-productcatalogservice:8080" or "otel-productcatalogservice:8080" or "127.0.0.1:8080"
+export const URI_REGEX =
+  /(?:([A-Za-z0-9+.-]+):(?:\/\/)?)?(?<name>[A-Za-z0-9+.-]+):(?<port>[0-9+.-]+)$/;
+
 // Equivalent to lodash _.findIndex
 export const findIndex: <T>(args: T[], fn: (arg: T) => boolean) => number = (
   args,

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/test/helper.ts b/experimental/packages/opentelemetry-instrumentation-grpc/test/helper.ts
@@ -515,6 +515,8 @@ export const runTests = (
       const validations = {
         name: `grpc.pkg_test.GrpcTester/${methodName}`,
         status: grpc.status.OK,
+        netPeerName: 'localhost',
+        netPeerPort: grpcPort,
       };
 
       assertSpan(moduleName, serverSpan, SpanKind.SERVER, validations);
@@ -699,6 +701,8 @@ export const runTests = (
             const validations = {
               name: `grpc.pkg_test.GrpcTester/${method.methodName}`,
               status: errorCode,
+              netPeerName: 'localhost',
+              netPeerPort: grpcPort,
             };
             const serverRoot = spans[0];
             const clientRoot = spans[1];
@@ -738,6 +742,8 @@ export const runTests = (
               const validations = {
                 name: `grpc.pkg_test.GrpcTester/${method.methodName}`,
                 status: errorCode,
+                netPeerName: 'localhost',
+                netPeerPort: grpcPort,
               };
               assertSpan(moduleName, serverSpan, SpanKind.SERVER, validations);
               assertSpan(moduleName, clientSpan, SpanKind.CLIENT, validations);

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/test/utils/assertionUtils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/test/utils/assertionUtils.ts
@@ -38,7 +38,12 @@ export const assertSpan = (
   component: string,
   span: ReadableSpan,
   kind: SpanKind,
-  validations: { name: string; status: grpc.status | grpcJs.status }
+  validations: {
+    name: string;
+    status: grpc.status | grpcJs.status;
+    netPeerName?: string;
+    netPeerPort?: number;
+  }
 ) => {
   assert.strictEqual(span.spanContext().traceId.length, 32);
   assert.strictEqual(span.spanContext().spanId.length, 16);
@@ -56,6 +61,21 @@ export const assertSpan = (
     assert.ok(span.spanContext());
   }
 
+  if (
+    span.kind === SpanKind.CLIENT &&
+    validations.netPeerName !== undefined &&
+    validations.netPeerPort !== undefined
+  ) {
+    assert.strictEqual(
+      span.attributes[SemanticAttributes.NET_PEER_NAME],
+      validations.netPeerName
+    );
+    assert.strictEqual(
+      span.attributes[SemanticAttributes.NET_PEER_PORT],
+      validations.netPeerPort
+    );
+  }
+
   // validations
   assert.strictEqual(span.name, validations.name);
   assert.strictEqual(

diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts
@@ -24,6 +24,7 @@ import {
 } from './RequireInTheMiddleSingleton';
 import { InstrumentationModuleDefinition } from './types';
 import { diag } from '@opentelemetry/api';
+import * as RequireInTheMiddle from 'require-in-the-middle';
 
 /**
  * Base abstract class for instrumenting node plugins
@@ -33,7 +34,7 @@ export abstract class InstrumentationBase<T = any>
   implements types.Instrumentation
 {
   private _modules: InstrumentationModuleDefinition<T>[];
-  private _hooks: Hooked[] = [];
+  private _hooks: (Hooked | RequireInTheMiddle.Hooked)[] = [];
   private _requireInTheMiddleSingleton: RequireInTheMiddleSingleton =
     RequireInTheMiddleSingleton.getInstance();
   private _enabled = false;
@@ -166,21 +167,26 @@ export abstract class InstrumentationBase<T = any>
 
     this._warnOnPreloadedModules();
     for (const module of this._modules) {
-      this._hooks.push(
-        this._requireInTheMiddleSingleton.register(
-          module.name,
-          (exports, name, baseDir) => {
-            return this._onRequire<typeof exports>(
-              module as unknown as InstrumentationModuleDefinition<
-                typeof exports
-              >,
-              exports,
-              name,
-              baseDir
-            );
-          }
-        )
-      );
+      const onRequire: RequireInTheMiddle.OnRequireFn = (
+        exports,
+        name,
+        baseDir
+      ) => {
+        return this._onRequire<typeof exports>(
+          module as unknown as InstrumentationModuleDefinition<typeof exports>,
+          exports,
+          name,
+          baseDir
+        );
+      };
+
+      // `RequireInTheMiddleSingleton` does not support absolute paths.
+      // For an absolute paths, we must create a separate instance of `RequireInTheMiddle`.
+      const hook = path.isAbsolute(module.name)
+        ? RequireInTheMiddle([module.name], { internals: true }, onRequire)
+        : this._requireInTheMiddleSingleton.register(module.name, onRequire);
+
+      this._hooks.push(hook);
     }
   }
 

diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts
@@ -16,9 +16,12 @@
 
 import * as assert from 'assert';
 import * as sinon from 'sinon';
+import * as path from 'path';
 import {
   InstrumentationBase,
   InstrumentationModuleDefinition,
+  InstrumentationNodeModuleDefinition,
+  InstrumentationNodeModuleFile,
 } from '../../src';
 
 const MODULE_NAME = 'test-module';
@@ -284,4 +287,97 @@ describe('InstrumentationBase', () => {
       });
     });
   });
+
+  describe('enable/disable', () => {
+    describe('AND a normal module name', () => {
+      type Exports = Record<string, unknown>;
+      type ExportsPatched = Exports & { __patched?: boolean };
+      const moduleName = 'net';
+      class TestInstrumentation extends InstrumentationBase<Exports> {
+        constructor() {
+          super('@opentelemetry/instrumentation-net-test', '0.0.0', {
+            enabled: false,
+          });
+        }
+        init(): InstrumentationNodeModuleDefinition<Exports>[] {
+          return [
+            new InstrumentationNodeModuleDefinition<Exports>(
+              moduleName,
+              ['*'],
+              (exports: ExportsPatched) => {
+                exports.__patched = true;
+                return exports;
+              },
+              (exports: ExportsPatched) => {
+                exports.__patched = false;
+                return exports;
+              }
+            ),
+          ];
+        }
+      }
+
+      const instrumentation = new TestInstrumentation();
+
+      it('should patch the module', () => {
+        instrumentation.enable();
+        const exportsPatched = require(moduleName);
+        assert.equal(exportsPatched.__patched, true, 'after enable');
+        instrumentation.disable();
+        assert.equal(exportsPatched.__patched, false, 'after disable');
+        instrumentation.enable();
+        assert.equal(exportsPatched.__patched, true, 'after re-enable');
+      });
+    });
+
+    describe('AND an absolute path module name', () => {
+      type Exports = Record<string, unknown>;
+      type ExportsPatched = Exports & { __patched?: boolean };
+      const moduleName = 'absolutePathTestFixture';
+      const fileName = path.join(__dirname, 'fixtures', `${moduleName}.js`);
+      class TestInstrumentation extends InstrumentationBase<Exports> {
+        constructor() {
+          super('@opentelemetry/instrumentation-absolute-path-test', '0.0.0', {
+            enabled: false,
+          });
+        }
+        init(): InstrumentationNodeModuleDefinition<Exports>[] {
+          return [
+            new InstrumentationNodeModuleDefinition<Exports>(
+              fileName,
+              ['*'],
+              undefined,
+              undefined,
+              [
+                new InstrumentationNodeModuleFile(
+                  moduleName,
+                  ['*'],
+                  (exports: ExportsPatched) => {
+                    exports.__patched = true;
+                    return exports;
+                  },
+                  (exports?: ExportsPatched) => {
+                    if (exports) exports.__patched = false;
+                    return exports;
+                  }
+                ),
+              ]
+            ),
+          ];
+        }
+      }
+
+      const instrumentation = new TestInstrumentation();
+
+      it('should patch the module', () => {
+        instrumentation.enable();
+        const exportsPatched = require(fileName);
+        assert.equal(exportsPatched.__patched, true, 'after enable');
+        instrumentation.disable();
+        assert.equal(exportsPatched.__patched, false, 'after disable');
+        instrumentation.enable();
+        assert.equal(exportsPatched.__patched, true, 'after re-enable');
+      });
+    });
+  });
 });
diff --git a/...ntal/packages/opentelemetry-instrumentation/test/node/fixtures/absolutePathTestFixture.js b/...ntal/packages/opentelemetry-instrumentation/test/node/fixtures/absolutePathTestFixture.js
@@ -0,0 +1,17 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+module.exports = {};