GRPC instrumentation. Added new attributes according to the OpenTelem…

…etry rpc convention (open-telemetry#3127) Co-authored-by: Valentin Marchaud <[email protected]>
weyert · Jul 30, 2022 · e76cb8b · e76cb8b
1 parent fc4dcba
commit e76cb8b
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,7 +8,8 @@ All notable changes to this project will be documented in this file.
 
 ### :rocket: (Enhancement)
 
-feat(sdk-trace-base): move Sampler declaration into sdk-trace-base [#3088](https://github.com/open-telemetry/opentelemetry-js/pull/3088) @legendecas
+* feat(sdk-trace-base): move Sampler declaration into sdk-trace-base [#3088](https://github.com/open-telemetry/opentelemetry-js/pull/3088) @legendecas
+* fix(grpc-instrumentation): added grpc attributes in instrumentation [#3127](https://github.com/open-telemetry/opentelemetry-js/pull/3127) @andrewzenkov
 
 ### :bug: (Bug Fix)
 

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/enums/AttributeNames.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/enums/AttributeNames.ts
@@ -17,9 +17,13 @@
 /**
  * https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md
  */
-export enum AttributeNames {
-  GRPC_KIND = 'grpc.kind', // SERVER or CLIENT
-  GRPC_METHOD = 'grpc.method',
-  GRPC_ERROR_NAME = 'grpc.error_name',
-  GRPC_ERROR_MESSAGE = 'grpc.error_message',
+
+interface AttributesType {
+  GRPC_ERROR_NAME: string;
+  GRPC_ERROR_MESSAGE: string;
 }
+
+export const AttributeNames: Readonly<AttributesType> = {
+  GRPC_ERROR_NAME: 'grpc.error_name',
+  GRPC_ERROR_MESSAGE: 'grpc.error_message',
+};
diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/enums/AttributeValues.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/enums/AttributeValues.ts
@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+
+interface AttributeValuesType {
+  RPC_SYSTEM: string
+}
+
+export const AttributeValues: Readonly<AttributeValuesType> = {
+  RPC_SYSTEM: 'grpc'
+};
diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts
@@ -17,7 +17,6 @@
 import { GrpcJsInstrumentation } from './';
 import type { GrpcClientFunc, SendUnaryDataCallback } from './types';
 import {
-  SpanKind,
   Span,
   SpanStatusCode,
   SpanStatus,
@@ -127,11 +126,6 @@ export function makeGrpcClientRemoteCall(
       }
     }
 
-    span.setAttributes({
-      [AttributeNames.GRPC_METHOD]: original.path,
-      [AttributeNames.GRPC_KIND]: SpanKind.CLIENT,
-    });
-
     setSpanContext(metadata);
     const call = original.apply(self, args);
 

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,9 @@ import {
   getMetadata,
 } from './clientUtils';
 import { EventEmitter } from 'events';
-import { AttributeNames } from '../enums/AttributeNames';
+import { _extractMethodAndService } from '../utils';
+import { AttributeValues } from '../enums/AttributeValues';
+import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 
 export class GrpcJsInstrumentation extends InstrumentationBase {
   constructor(
@@ -188,10 +190,14 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
                   keys: carrier => Object.keys(carrier.getMap()),
                 }),
                 () => {
+                  const { service, method } = _extractMethodAndService(name);
+
                   const span = instrumentation.tracer
                     .startSpan(spanName, spanOptions)
                     .setAttributes({
-                      [AttributeNames.GRPC_KIND]: spanOptions.kind,
+                      [SemanticAttributes.RPC_SYSTEM]: AttributeValues.RPC_SYSTEM,
+                      [SemanticAttributes.RPC_METHOD]: method,
+                      [SemanticAttributes.RPC_SERVICE]: service,
                     });
 
                   context.with(trace.setSpan(context.active(), span), () => {
@@ -283,9 +289,15 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
           original,
           args
         );
-        const span = instrumentation.tracer.startSpan(name, {
-          kind: SpanKind.CLIENT,
-        });
+        const { service, method } = _extractMethodAndService(original.path);
+
+        const span = instrumentation.tracer
+          .startSpan(name, { kind: SpanKind.CLIENT })
+          .setAttributes({
+            [SemanticAttributes.RPC_SYSTEM]: 'grpc',
+            [SemanticAttributes.RPC_METHOD]: method,
+            [SemanticAttributes.RPC_SERVICE]: service,
+          });
         return context.with(trace.setSpan(context.active(), span), () =>
           makeGrpcClientRemoteCall(original, args, metadata, this)(span)
         );

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/clientUtils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/clientUtils.ts
@@ -22,7 +22,6 @@ import {
   context,
   Span,
   SpanStatusCode,
-  SpanKind,
   SpanStatus,
   propagation,
 } from '@opentelemetry/api';
@@ -99,10 +98,6 @@ export const makeGrpcClientRemoteCall = function (
     }
 
     span.addEvent('sent');
-    span.setAttributes({
-      [AttributeNames.GRPC_METHOD]: original.path,
-      [AttributeNames.GRPC_KIND]: SpanKind.CLIENT,
-    });
 
     setSpanContext(metadata);
     const call = original.apply(self, args);

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts
@@ -41,8 +41,9 @@ import {
   serverStreamAndBidiHandler,
 } from './serverUtils';
 import { makeGrpcClientRemoteCall, getMetadata } from './clientUtils';
-import { _methodIsIgnored } from '../utils';
-import { AttributeNames } from '../enums/AttributeNames';
+import { _extractMethodAndService, _methodIsIgnored } from '../utils';
+import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
+import {AttributeValues} from '../enums/AttributeValues';
 
 /**
  * Holding reference to grpc module here to access constant of grpc modules
@@ -52,7 +53,7 @@ let grpcClient: typeof grpcTypes;
 
 export class GrpcNativeInstrumentation extends InstrumentationBase<
   typeof grpcTypes
-> {
+  > {
   constructor(
     name: string,
     version: string,
@@ -195,10 +196,14 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
                   keys: metadata => Object.keys(metadata.getMap()),
                 }),
                 () => {
+                  const { service, method } = _extractMethodAndService(name);
+
                   const span = instrumentation.tracer
                     .startSpan(spanName, spanOptions)
                     .setAttributes({
-                      [AttributeNames.GRPC_KIND]: spanOptions.kind,
+                      [SemanticAttributes.RPC_SYSTEM]: AttributeValues.RPC_SYSTEM,
+                      [SemanticAttributes.RPC_METHOD]: method,
+                      [SemanticAttributes.RPC_SERVICE]: service,
                     });
 
                   context.with(trace.setSpan(context.active(), span), () => {
@@ -292,9 +297,15 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
         )}`;
         const args = Array.prototype.slice.call(arguments);
         const metadata = getMetadata(grpcClient, original, args);
+        const { service, method } = _extractMethodAndService(original.path);
         const span = instrumentation.tracer.startSpan(name, {
           kind: SpanKind.CLIENT,
-        });
+        })
+          .setAttributes({
+            [SemanticAttributes.RPC_SYSTEM]: AttributeValues.RPC_SYSTEM,
+            [SemanticAttributes.RPC_METHOD]: method,
+            [SemanticAttributes.RPC_SERVICE]: service,
+          });
         return context.with(trace.setSpan(context.active(), span), () =>
           makeGrpcClientRemoteCall(
             grpcClient,

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/utils.ts
@@ -95,3 +95,18 @@ export const _methodIsIgnored = (
 
   return false;
 };
+
+/**
+ * Return method and service values getting from grpc name/path
+ * @param name the grpc name/path
+ */
+export const _extractMethodAndService = (name: string): { service: string, method: string } => {
+  const serviceMethod = name.replace(/^\//, '').split('/');
+  const service = serviceMethod.shift() || '';
+  const method = serviceMethod.join('/');
+
+  return ({
+    service,
+    method
+  });
+};
diff --git a/...al/packages/opentelemetry-instrumentation-grpc/test/utils/extractMethodAndServiceUtils.ts b/...al/packages/opentelemetry-instrumentation-grpc/test/utils/extractMethodAndServiceUtils.ts
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+import { _extractMethodAndService } from '../../src/utils';
+import * as assert from 'assert';
+
+
+const cases = [
+  { value: 'readBooks/BookStorage.Book', result: { method: 'BookStorage.Book', service: 'readBooks' } },
+  { value: 'readBooks//BookStorage.Book', result: { method: '/BookStorage.Book', service: 'readBooks' } },
+  { value: 'readBooks/BookStorage/.Book', result: { method: 'BookStorage/.Book', service: 'readBooks' } },
+  { value: '/readBooks/BookStorage/.Book/Book', result: { method: 'BookStorage/.Book/Book', service: 'readBooks' } },
+];
+
+describe('ExtractMethodAndService Util', () => {
+
+  cases.forEach(({ value, result }) => {
+    it(`Should resolve use case correctly for: ${value}`, () => {
+      const { method, service } = _extractMethodAndService(value);
+
+      assert.deepStrictEqual({ method, service }, { method: result.method, service: result.service });
+    });
+  });
+});