refactor: standardise implementation of non-official attributes (open…

…-telemetry#480)

Co-authored-by: Valentin Marchaud <[email protected]>

plugins/node/opentelemetry-instrumentation-express/src/enums/AttributeNames.ts

@@ -0,0 +1,19 @@
+/*
+ * 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 enum AttributeNames {
+  EXPRESS_TYPE = 'express.type',
+  EXPRESS_NAME = 'express.name',
+}

plugins/node/opentelemetry-instrumentation-express/src/enums/ExpressLayerType.ts

@@ -0,0 +1,20 @@
+/*
+ * 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 enum ExpressLayerType {
+  ROUTER = 'router',
+  MIDDLEWARE = 'middleware',
+  REQUEST_HANDLER = 'request_handler',
+}

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

@@ -20,13 +20,13 @@ import * as express from 'express';
 import {
   ExpressLayer,
   ExpressRouter,
-  CustomAttributeNames,
   PatchedRequest,
   _LAYERS_STORE_PROPERTY,
   ExpressInstrumentationConfig,
-  ExpressLayerType,
   ExpressInstrumentationSpan,
 } from './types';
+import { ExpressLayerType } from './enums/ExpressLayerType';
+import { AttributeNames } from './enums/AttributeNames';
 import { getLayerMetadata, storeLayerPath, isLayerIgnored } from './utils';
 import { VERSION } from './version';
 import {
@@ -187,13 +187,13 @@ export class ExpressInstrumentation extends InstrumentationBase<
         };
         const metadata = getLayerMetadata(layer, layerPath);
         const type = metadata.attributes[
-          CustomAttributeNames.EXPRESS_TYPE
+          AttributeNames.EXPRESS_TYPE
         ] as ExpressLayerType;
 
         // Rename the root http span in case we haven't done it already
         // once we reach the request handler
         if (
-          metadata.attributes[CustomAttributeNames.EXPRESS_TYPE] ===
+          metadata.attributes[AttributeNames.EXPRESS_TYPE] ===
           ExpressLayerType.REQUEST_HANDLER
         ) {
           const parent = getSpan(
@@ -226,7 +226,7 @@ export class ExpressInstrumentation extends InstrumentationBase<
         // If we found anything that isnt a middleware, there no point of measuring
         // their time since they dont have callback.
         if (
-          metadata.attributes[CustomAttributeNames.EXPRESS_TYPE] !==
+          metadata.attributes[AttributeNames.EXPRESS_TYPE] !==
           ExpressLayerType.MIDDLEWARE
         ) {
           span.end(startTime);

plugins/node/opentelemetry-instrumentation-express/src/index.ts

@@ -15,4 +15,5 @@
  */
 
 export * from './express';
-export { ExpressInstrumentationConfig, ExpressLayerType } from './types';
+export { ExpressLayerType } from './enums/ExpressLayerType';
+export { ExpressInstrumentationConfig } from './types';

plugins/node/opentelemetry-instrumentation-express/src/types.ts

@@ -18,6 +18,7 @@ import { kLayerPatched } from './express';
 import { Request } from 'express';
 import { SpanAttributes, Span } from '@opentelemetry/api';
 import { InstrumentationConfig } from '@opentelemetry/instrumentation';
+import { ExpressLayerType } from './enums/ExpressLayerType';
 
 /**
  * This const define where on the `request` object the Instrumentation will mount the
@@ -67,17 +68,6 @@ export type LayerMetadata = {
   name: string;
 };
 
-export enum CustomAttributeNames {
-  EXPRESS_TYPE = 'express.type',
-  EXPRESS_NAME = 'express.name',
-}
-
-export enum ExpressLayerType {
-  ROUTER = 'router',
-  MIDDLEWARE = 'middleware',
-  REQUEST_HANDLER = 'request_handler',
-}
-
 export type IgnoreMatcher = string | RegExp | ((name: string) => boolean);
 
 /**

plugins/node/opentelemetry-instrumentation-express/src/utils.ts

@@ -17,13 +17,13 @@
 import { SpanAttributes } from '@opentelemetry/api';
 import {
   ExpressLayer,
-  CustomAttributeNames,
   PatchedRequest,
   _LAYERS_STORE_PROPERTY,
-  ExpressLayerType,
   IgnoreMatcher,
   ExpressInstrumentationConfig,
 } from './types';
+import { ExpressLayerType } from './enums/ExpressLayerType';
+import { AttributeNames } from './enums/AttributeNames';
 
 /**
  * Store layers path in the request to be able to construct route later
@@ -56,24 +56,24 @@ export const getLayerMetadata = (
   if (layer.name === 'router') {
     return {
       attributes: {
-        [CustomAttributeNames.EXPRESS_NAME]: layerPath,
-        [CustomAttributeNames.EXPRESS_TYPE]: ExpressLayerType.ROUTER,
+        [AttributeNames.EXPRESS_NAME]: layerPath,
+        [AttributeNames.EXPRESS_TYPE]: ExpressLayerType.ROUTER,
       },
       name: `router - ${layerPath}`,
     };
   } else if (layer.name === 'bound dispatch') {
     return {
       attributes: {
-        [CustomAttributeNames.EXPRESS_NAME]: layerPath ?? 'request handler',
-        [CustomAttributeNames.EXPRESS_TYPE]: ExpressLayerType.REQUEST_HANDLER,
+        [AttributeNames.EXPRESS_NAME]: layerPath ?? 'request handler',
+        [AttributeNames.EXPRESS_TYPE]: ExpressLayerType.REQUEST_HANDLER,
       },
       name: `request handler${layer.path ? ` - ${layerPath}` : ''}`,
     };
   } else {
     return {
       attributes: {
-        [CustomAttributeNames.EXPRESS_NAME]: layer.name,
-        [CustomAttributeNames.EXPRESS_TYPE]: ExpressLayerType.MIDDLEWARE,
+        [AttributeNames.EXPRESS_NAME]: layer.name,
+        [AttributeNames.EXPRESS_TYPE]: ExpressLayerType.MIDDLEWARE,
       },
       name: `middleware - ${layer.name}`,
     };

plugins/node/opentelemetry-instrumentation-express/test/custom-config.test.ts

@@ -23,11 +23,9 @@ import {
 } from '@opentelemetry/tracing';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import * as assert from 'assert';
-import {
-  CustomAttributeNames,
-  ExpressInstrumentationSpan,
-  ExpressLayerType,
-} from '../src/types';
+import { ExpressInstrumentationSpan } from '../src/types';
+import { ExpressLayerType } from '../src/enums/ExpressLayerType';
+import { AttributeNames } from '../src/enums/AttributeNames';
 import { ExpressInstrumentation } from '../src';
 
 const instrumentation = new ExpressInstrumentation({
@@ -115,7 +113,7 @@ describe('ExpressInstrumentation', () => {
             .getFinishedSpans()
             .filter(
               span =>
-                span.attributes[CustomAttributeNames.EXPRESS_TYPE] ===
+                span.attributes[AttributeNames.EXPRESS_TYPE] ===
                 ExpressLayerType.MIDDLEWARE
             ).length,
           0
@@ -164,7 +162,7 @@ describe('ExpressInstrumentation', () => {
         );
 
         assert.strictEqual(
-          requestHandlerSpan?.attributes[CustomAttributeNames.EXPRESS_TYPE],
+          requestHandlerSpan?.attributes[AttributeNames.EXPRESS_TYPE],
           'request_handler'
         );
         const exportedRootSpan = spans.find(span => span.name === 'GET /mw');

plugins/node/opentelemetry-instrumentation-express/test/express.test.ts

@@ -22,7 +22,8 @@ import {
   SimpleSpanProcessor,
 } from '@opentelemetry/tracing';
 import * as assert from 'assert';
-import { CustomAttributeNames, ExpressInstrumentationSpan } from '../src/types';
+import { ExpressInstrumentationSpan } from '../src/types';
+import { AttributeNames } from '../src/enums/AttributeNames';
 import { ExpressInstrumentation } from '../src';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 
@@ -168,7 +169,7 @@ describe('ExpressInstrumentation', () => {
           '/toto/:id'
         );
         assert.strictEqual(
-          requestHandlerSpan?.attributes[CustomAttributeNames.EXPRESS_TYPE],
+          requestHandlerSpan?.attributes[AttributeNames.EXPRESS_TYPE],
           'request_handler'
         );
         const exportedRootSpan = memoryExporter

plugins/node/opentelemetry-instrumentation-express/test/ignore-all.test.ts

@@ -22,7 +22,8 @@ import {
   SimpleSpanProcessor,
 } from '@opentelemetry/tracing';
 import * as assert from 'assert';
-import { CustomAttributeNames, ExpressInstrumentationSpan } from '../src/types';
+import { ExpressInstrumentationSpan } from '../src/types';
+import { AttributeNames } from '../src/enums/AttributeNames';
 import { ExpressInstrumentation, ExpressLayerType } from '../src';
 
 const instrumentation = new ExpressInstrumentation({
@@ -120,11 +121,11 @@ describe('ExpressInstrumentation', () => {
             .getFinishedSpans()
             .filter(
               span =>
-                span.attributes[CustomAttributeNames.EXPRESS_TYPE] ===
+                span.attributes[AttributeNames.EXPRESS_TYPE] ===
                   ExpressLayerType.MIDDLEWARE ||
-                span.attributes[CustomAttributeNames.EXPRESS_TYPE] ===
+                span.attributes[AttributeNames.EXPRESS_TYPE] ===
                   ExpressLayerType.ROUTER ||
-                span.attributes[CustomAttributeNames.EXPRESS_TYPE] ===
+                span.attributes[AttributeNames.EXPRESS_TYPE] ===
                   ExpressLayerType.REQUEST_HANDLER
             ).length,
           0

plugins/node/opentelemetry-instrumentation-express/test/utils.test.ts

@@ -16,12 +16,9 @@
 
 import * as utils from '../src/utils';
 import * as assert from 'assert';
-import {
-  ExpressLayerType,
-  ExpressInstrumentationConfig,
-  ExpressLayer,
-  CustomAttributeNames,
-} from '../src/types';
+import { ExpressInstrumentationConfig, ExpressLayer } from '../src/types';
+import { ExpressLayerType } from '../src/enums/ExpressLayerType';
+import { AttributeNames } from '../src/enums/AttributeNames';
 
 describe('Utils', () => {
   describe('isLayerIgnored()', () => {
@@ -103,8 +100,8 @@ describe('Utils', () => {
         ),
         {
           attributes: {
-            [CustomAttributeNames.EXPRESS_NAME]: '/test',
-            [CustomAttributeNames.EXPRESS_TYPE]: 'router',
+            [AttributeNames.EXPRESS_NAME]: '/test',
+            [AttributeNames.EXPRESS_TYPE]: 'router',
           },
           name: 'router - /test',
         }
@@ -121,8 +118,8 @@ describe('Utils', () => {
         ),
         {
           attributes: {
-            [CustomAttributeNames.EXPRESS_NAME]: '/:id',
-            [CustomAttributeNames.EXPRESS_TYPE]: 'request_handler',
+            [AttributeNames.EXPRESS_NAME]: '/:id',
+            [AttributeNames.EXPRESS_TYPE]: 'request_handler',
           },
           name: 'request handler',
         }
@@ -136,8 +133,8 @@ describe('Utils', () => {
         } as ExpressLayer),
         {
           attributes: {
-            [CustomAttributeNames.EXPRESS_NAME]: 'bodyParser',
-            [CustomAttributeNames.EXPRESS_TYPE]: 'middleware',
+            [AttributeNames.EXPRESS_NAME]: 'bodyParser',
+            [AttributeNames.EXPRESS_TYPE]: 'middleware',
           },
           name: 'middleware - bodyParser',
         }

plugins/node/opentelemetry-instrumentation-graphql/src/enum.ts

@@ -45,17 +45,6 @@ export enum TokenKind {
   COMMENT = 'Comment',
 }
 
-export enum SpanAttributes {
-  COMPONENT = 'graphql',
-  SOURCE = 'graphql.source',
-  FIELD_NAME = 'graphql.field.name',
-  FIELD_PATH = 'graphql.field.path',
-  FIELD_TYPE = 'graphql.field.type',
-  OPERATION = 'graphql.operation.name',
-  VARIABLES = 'graphql.variables.',
-  ERROR_VALIDATION_NAME = 'graphql.validation.error',
-}
-
 export enum SpanNames {
   EXECUTE = 'graphql.execute',
   PARSE = 'graphql.parse',

plugins/node/opentelemetry-instrumentation-graphql/src/enums/AttributeNames.ts

@@ -0,0 +1,25 @@
+/*
+ * 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 enum AttributeNames {
+  COMPONENT = 'graphql',
+  SOURCE = 'graphql.source',
+  FIELD_NAME = 'graphql.field.name',
+  FIELD_PATH = 'graphql.field.path',
+  FIELD_TYPE = 'graphql.field.type',
+  OPERATION = 'graphql.operation.name',
+  VARIABLES = 'graphql.variables.',
+  ERROR_VALIDATION_NAME = 'graphql.validation.error',
+}

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

@@ -25,7 +25,8 @@ import {
 } from '@opentelemetry/instrumentation';
 import type * as graphqlTypes from 'graphql';
 import { GraphQLFieldResolver } from 'graphql/type/definition';
-import { SpanAttributes, SpanNames } from './enum';
+import { SpanNames } from './enum';
+import { AttributeNames } from './enums/AttributeNames';
 import { OTEL_GRAPHQL_DATA_SYMBOL } from './symbols';
 
 import {
@@ -334,7 +335,7 @@ export class GraphQLInstrumentation extends InstrumentationBase {
           }
           if (errors && errors.length) {
             span.recordException({
-              name: SpanAttributes.ERROR_VALIDATION_NAME,
+              name: AttributeNames.ERROR_VALIDATION_NAME,
               message: JSON.stringify(errors),
             });
           }
@@ -355,7 +356,7 @@ export class GraphQLInstrumentation extends InstrumentationBase {
       const name = (operation as graphqlTypes.OperationDefinitionNode)
         .operation;
       if (name) {
-        span.setAttribute(SpanAttributes.OPERATION, name);
+        span.setAttribute(AttributeNames.OPERATION, name);
       }
     } else {
       let operationName = ' ';
@@ -366,7 +367,7 @@ export class GraphQLInstrumentation extends InstrumentationBase {
         '$operationName$',
         operationName
       );
-      span.setAttribute(SpanAttributes.OPERATION, operationName);
+      span.setAttribute(AttributeNames.OPERATION, operationName);
     }
 
     if (processedArgs.document?.loc) {
@@ -375,7 +376,7 @@ export class GraphQLInstrumentation extends InstrumentationBase {
 
     if (processedArgs.variableValues && config.allowValues) {
       Object.entries(processedArgs.variableValues).forEach(([key, value]) => {
-        span.setAttribute(`${SpanAttributes.VARIABLES}${String(key)}`, value);
+        span.setAttribute(`${AttributeNames.VARIABLES}${String(key)}`, value);
       });
     }