fix(instrumentation-grpc): always set grpc semcov status code attribu…

…te with numeric value (open-telemetry#3076)
RazGvili · Nov 30, 2022 · b152497 · b152497
1 parent db0ecc3
commit b152497
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 23 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -15,6 +15,7 @@ All notable changes to experimental packages in this project will be documented
 ### :bug: (Bug Fix)
 
 * 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
 
 ### :books: (Refine Doc)
 
@@ -151,10 +152,11 @@ All notable changes to experimental packages in this project will be documented
 ### :bug: (Bug Fix)
 
 * fix(histogram): fix maximum when only values < -1 are provided [#3086](https://github.com/open-telemetry/opentelemetry-js/pull/3086) @pichlermarc
-* fix(sdk-metrics-base): fix PeriodicExportingMetricReader keeping Node.js process from exiting
-  [#3106](https://github.com/open-telemetry/opentelemetry-js/pull/3106) @seemk
-* fix(otlp-proto): fixes [#2791](https://github.com/open-telemetry/opentelemetry-js/issues/2791) otlp proto exporters no longer share a single global proto definition
-  [#3098](https://github.com/open-telemetry/opentelemetry-js/pull/3098) @legendecas
+* fix(instrumentation-grpc): always set grpc semcov status code attribute with numeric value [#3076](https://github.com/open-telemetry/opentelemetry-js/pull/3076) @blumamir
+
+### :books: (Refine Doc)
+
+### :house: (Internal)
 
 ## 0.30.0
 

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts
@@ -34,6 +34,7 @@ import { EventEmitter } from 'events';
 import { AttributeNames } from '../enums/AttributeNames';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import { metadataCaptureType } from '../types';
+import { GRPC_STATUS_CODE_OK } from '../status-code';
 
 /**
  * Parse a package method list and return a list of methods to patch
@@ -93,7 +94,7 @@ export function makeGrpcClientRemoteCall(
           span.setStatus(_grpcStatusCodeToSpanStatus(err.code));
           span.setAttribute(
             SemanticAttributes.RPC_GRPC_STATUS_CODE,
-            err.code.toString()
+            err.code
           );
         }
         span.setAttributes({
@@ -104,7 +105,7 @@ export function makeGrpcClientRemoteCall(
         span.setStatus({ code: SpanStatusCode.UNSET });
         span.setAttribute(
           SemanticAttributes.RPC_GRPC_STATUS_CODE,
-          SpanStatusCode.UNSET.toString()
+          GRPC_STATUS_CODE_OK
         );
       }
 
@@ -160,6 +161,7 @@ export function makeGrpcClientRemoteCall(
         span.setAttributes({
           [AttributeNames.GRPC_ERROR_NAME]: err.name,
           [AttributeNames.GRPC_ERROR_MESSAGE]: err.message,
+          [SemanticAttributes.RPC_GRPC_STATUS_CODE]: err.code,
         });
 
         endSpan();
@@ -172,6 +174,7 @@ export function makeGrpcClientRemoteCall(
         call[CALL_SPAN_ENDED] = true;
 
         span.setStatus(_grpcStatusCodeToSpanStatus(status.code));
+        span.setAttribute(SemanticAttributes.RPC_GRPC_STATUS_CODE, status.code);
 
         endSpan();
       });

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/serverUtils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/serverUtils.ts
@@ -35,6 +35,7 @@ import {
 import { IgnoreMatcher } from '../types';
 import { AttributeNames } from '../enums/AttributeNames';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
+import { GRPC_STATUS_CODE_OK } from '../status-code';
 
 export const CALL_SPAN_ENDED = Symbol('opentelemetry call span ended');
 
@@ -72,7 +73,7 @@ function serverStreamAndBidiHandler<RequestType, ResponseType>(
     });
     span.setAttribute(
       SemanticAttributes.RPC_GRPC_STATUS_CODE,
-      SpanStatusCode.OK.toString()
+      GRPC_STATUS_CODE_OK
     );
 
     endSpan();
@@ -93,6 +94,7 @@ function serverStreamAndBidiHandler<RequestType, ResponseType>(
     span.setAttributes({
       [AttributeNames.GRPC_ERROR_NAME]: err.name,
       [AttributeNames.GRPC_ERROR_MESSAGE]: err.message,
+      [SemanticAttributes.RPC_GRPC_STATUS_CODE]: err.code,
     });
     endSpan();
   });
@@ -124,7 +126,7 @@ function clientStreamAndUnaryHandler<RequestType, ResponseType>(
         });
         span.setAttribute(
           SemanticAttributes.RPC_GRPC_STATUS_CODE,
-          err.code.toString()
+          err.code
         );
       }
       span.setAttributes({
@@ -135,7 +137,7 @@ function clientStreamAndUnaryHandler<RequestType, ResponseType>(
       span.setStatus({ code: SpanStatusCode.UNSET });
       span.setAttribute(
         SemanticAttributes.RPC_GRPC_STATUS_CODE,
-        SpanStatusCode.OK.toString()
+        GRPC_STATUS_CODE_OK
       );
     }
 

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,
-  SpanStatus,
   propagation,
 } from '@opentelemetry/api';
 import {
@@ -32,13 +31,13 @@ import {
 } from '../utils';
 import { AttributeNames } from '../enums/AttributeNames';
 import { metadataCaptureType } from '../types';
+import { GRPC_STATUS_CODE_OK } from '../status-code';
 
 /**
  * This method handles the client remote call
  */
 export const makeGrpcClientRemoteCall = function (
   metadataCapture: metadataCaptureType,
-  grpcClient: typeof grpcTypes,
   original: GrpcClientFunc,
   args: any[],
   metadata: grpcTypes.Metadata,
@@ -59,7 +58,7 @@ export const makeGrpcClientRemoteCall = function (
           span.setStatus(_grpcStatusCodeToSpanStatus(err.code));
           span.setAttribute(
             SemanticAttributes.RPC_GRPC_STATUS_CODE,
-            err.code.toString()
+            err.code
           );
         }
         span.setAttributes({
@@ -70,7 +69,7 @@ export const makeGrpcClientRemoteCall = function (
         span.setStatus({ code: SpanStatusCode.UNSET });
         span.setAttribute(
           SemanticAttributes.RPC_GRPC_STATUS_CODE,
-          grpcClient.status.OK.toString()
+          GRPC_STATUS_CODE_OK
         );
       }
 
@@ -133,17 +132,20 @@ export const makeGrpcClientRemoteCall = function (
             [AttributeNames.GRPC_ERROR_NAME]: err.name,
             [AttributeNames.GRPC_ERROR_MESSAGE]: err.message,
           });
+          if(err.code != null) {
+            span.setAttribute(SemanticAttributes.RPC_GRPC_STATUS_CODE, err.code);
+          }
           endSpan();
         }
       );
 
       ((call as unknown) as events.EventEmitter).on(
         'status',
-        (status: SpanStatus) => {
+        (status: grpcTypes.StatusObject) => {
           span.setStatus({ code: SpanStatusCode.UNSET });
           span.setAttribute(
             SemanticAttributes.RPC_GRPC_STATUS_CODE,
-            status.code.toString()
+            status.code
           );
           endSpan();
         }

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts
@@ -80,7 +80,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
           this._wrap(
             moduleExports.Server.prototype,
             'register',
-            this._patchServer(moduleExports) as any
+            this._patchServer() as any
           );
           // Wrap the externally exported client constructor
           if (isWrapped(moduleExports.makeGenericClientConstructor)) {
@@ -149,7 +149,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
     ];
   }
 
-  private _patchServer(grpcModule: typeof grpcTypes) {
+  private _patchServer() {
     const instrumentation = this;
     return (originalRegister: typeof grpcTypes.Server.prototype.register) => {
       instrumentation._diag.debug('patched gRPC server');
@@ -219,7 +219,6 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
                       case 'unary':
                       case 'client_stream':
                         return clientStreamAndUnaryHandler(
-                          grpcModule,
                           span,
                           call,
                           callback,
@@ -320,7 +319,6 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
         return context.with(trace.setSpan(context.active(), span), () =>
           makeGrpcClientRemoteCall(
             instrumentation._metadataCapture,
-            grpcClient,
             original,
             args,
             metadata,

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/serverUtils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/grpc/serverUtils.ts
@@ -25,9 +25,9 @@ import {
 } from '../utils';
 import { AttributeNames } from '../enums/AttributeNames';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
+import { GRPC_STATUS_CODE_OK } from '../status-code';
 
 export const clientStreamAndUnaryHandler = function <RequestType, ResponseType>(
-  grpcClient: typeof grpcTypes,
   span: Span,
   call: ServerCallWithMeta,
   callback: SendUnaryDataCallback,
@@ -50,7 +50,7 @@ export const clientStreamAndUnaryHandler = function <RequestType, ResponseType>(
         });
         span.setAttribute(
           SemanticAttributes.RPC_GRPC_STATUS_CODE,
-          err.code.toString()
+          err.code
         );
       }
       span.setAttributes({
@@ -61,7 +61,7 @@ export const clientStreamAndUnaryHandler = function <RequestType, ResponseType>(
       span.setStatus({ code: SpanStatusCode.UNSET });
       span.setAttribute(
         SemanticAttributes.RPC_GRPC_STATUS_CODE,
-        grpcClient.status.OK.toString()
+        GRPC_STATUS_CODE_OK
       );
     }
     span.addEvent('received');
@@ -94,7 +94,7 @@ export const serverStreamAndBidiHandler = function <RequestType, ResponseType>(
     span.setStatus(_grpcStatusCodeToSpanStatus(call.status.code));
     span.setAttribute(
       SemanticAttributes.RPC_GRPC_STATUS_CODE,
-      call.status.code.toString()
+      call.status.code
     );
 
     // if there is an error, span will be ended on error event, otherwise end it here
@@ -114,6 +114,9 @@ export const serverStreamAndBidiHandler = function <RequestType, ResponseType>(
       [AttributeNames.GRPC_ERROR_NAME]: err.name,
       [AttributeNames.GRPC_ERROR_MESSAGE]: err.message,
     });
+    if(err.code != null) {
+      span.setAttribute(SemanticAttributes.RPC_GRPC_STATUS_CODE, err.code);
+    }
     endSpan();
   });
 

diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/status-code.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/status-code.ts
@@ -0,0 +1,16 @@
+/*
+ * 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.
+ */
+export const GRPC_STATUS_CODE_OK = 0;
diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/test/utils/assertionUtils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/test/utils/assertionUtils.ts
@@ -23,6 +23,7 @@ import {
   hrTimeToMilliseconds,
   hrTimeToMicroseconds,
 } from '@opentelemetry/core';
+import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 
 export const grpcStatusCodeToOpenTelemetryStatusCode = (
   status: grpc.status | grpcJs.status
@@ -61,6 +62,7 @@ export const assertSpan = (
     span.status.code,
     grpcStatusCodeToOpenTelemetryStatusCode(validations.status)
   );
+  assert.strictEqual(span.attributes[SemanticAttributes.RPC_GRPC_STATUS_CODE], validations.status);
 };
 
 // Check if sourceSpan was propagated to targetSpan