From bf42243ce5ad0f07bcd937dabf51dc9cf73f98c9 Mon Sep 17 00:00:00 2001 From: andrewzenkov Date: Fri, 29 Jul 2022 15:45:34 +0200 Subject: [PATCH 1/3] Added necessary attributes. A little fixes in convention --- .../src/enums/AttributeNames.ts | 14 +++++++---- .../src/enums/AttributeValues.ts | 23 +++++++++++++++++++ .../src/grpc-js/clientUtils.ts | 6 ----- .../src/grpc-js/index.ts | 22 ++++++++++++++---- .../src/grpc/clientUtils.ts | 5 ---- .../src/grpc/index.ts | 21 +++++++++++++---- .../src/utils.ts | 15 ++++++++++++ 7 files changed, 80 insertions(+), 26 deletions(-) create mode 100644 experimental/packages/opentelemetry-instrumentation-grpc/src/enums/AttributeValues.ts diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/enums/AttributeNames.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/enums/AttributeNames.ts index 9dfd3e0923..9d0facd20f 100644 --- 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 = { + 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 new file mode 100644 index 0000000000..0cdcb14311 --- /dev/null +++ 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 = { + 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 index 5671d3a815..946cc20185 100644 --- 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 index 7e854d7b77..4a1c8e9f3a 100644 --- 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 index 331e744344..5f8fa8af44 100644 --- 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 index cbc6bb7560..b31d9ef55c 100644 --- 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 index 14e0a86eb9..16bf81a728 100644 --- 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 + }); +}; From 1d7ad65c6a3f00254a35644cb60deaebba46b282 Mon Sep 17 00:00:00 2001 From: andrewzenkov Date: Fri, 29 Jul 2022 17:11:54 +0200 Subject: [PATCH 2/3] Updated change log --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c18cad260..9117107889 100644 --- 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) From 3dd925e9d892cda017e97006012395217e7e1eba Mon Sep 17 00:00:00 2001 From: andrewzenkov Date: Fri, 29 Jul 2022 20:39:04 +0200 Subject: [PATCH 3/3] Added tests for extractMethodandService util --- .../utils/extractMethodAndServiceUtils.ts | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 experimental/packages/opentelemetry-instrumentation-grpc/test/utils/extractMethodAndServiceUtils.ts diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/test/utils/extractMethodAndServiceUtils.ts b/experimental/packages/opentelemetry-instrumentation-grpc/test/utils/extractMethodAndServiceUtils.ts new file mode 100644 index 0000000000..2d4faf744e --- /dev/null +++ b/experimental/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 }); + }); + }); +});