fix(gql)!: conform GraphQL span name to spec (open-telemetry#1444)

dynatrace-oss-contrib · Apr 2, 2023 · 7d070db · 7d070db
1 parent 9268716
commit 7d070db
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 21 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts
@@ -404,18 +404,21 @@ export class GraphQLInstrumentation extends InstrumentationBase {
 
     const span = this.tracer.startSpan(SpanNames.EXECUTE, {});
     if (operation) {
-      const operationDefinition =
+      const { operation: operationType, name: nameNode } =
         operation as graphqlTypes.OperationDefinitionNode;
-      span.setAttribute(
-        AttributeNames.OPERATION_TYPE,
-        operationDefinition.operation
-      );
 
-      if (operationDefinition.name) {
-        span.setAttribute(
-          AttributeNames.OPERATION_NAME,
-          operationDefinition.name.value
-        );
+      span.setAttribute(AttributeNames.OPERATION_TYPE, operationType);
+
+      const operationName = nameNode?.value;
+
+      // https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/instrumentation/graphql/
+      // > The span name MUST be of the format <graphql.operation.type> <graphql.operation.name> provided that graphql.operation.type and graphql.operation.name are available.
+      // > If graphql.operation.name is not available, the span SHOULD be named <graphql.operation.type>.
+      if (operationName) {
+        span.setAttribute(AttributeNames.OPERATION_NAME, operationName);
+        span.updateName(`${operationType} ${operationName}`);
+      } else {
+        span.updateName(operationType);
       }
     } else {
       let operationName = ' ';

diff --git a/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts b/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts
@@ -169,7 +169,7 @@ describe('graphql', () => {
           executeSpan.attributes[AttributeNames.OPERATION_NAME],
           undefined
         );
-        assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);
+        assert.deepStrictEqual(executeSpan.name, 'query');
         assert.deepStrictEqual(executeSpan.parentSpanId, undefined);
       });
 
@@ -297,7 +297,7 @@ describe('graphql', () => {
           executeSpan.attributes[AttributeNames.OPERATION_NAME],
           undefined
         );
-        assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);
+        assert.deepStrictEqual(executeSpan.name, 'query');
         assert.deepStrictEqual(executeSpan.parentSpanId, undefined);
       });
 
@@ -395,7 +395,7 @@ describe('graphql', () => {
           executeSpan.attributes[`${AttributeNames.VARIABLES}id`],
           undefined
         );
-        assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);
+        assert.deepStrictEqual(executeSpan.name, 'query Query1');
         assert.deepStrictEqual(executeSpan.parentSpanId, undefined);
       });
 
@@ -487,7 +487,7 @@ describe('graphql', () => {
           executeSpan.attributes[AttributeNames.OPERATION_NAME],
           undefined
         );
-        assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);
+        assert.deepStrictEqual(executeSpan.name, 'query');
         assert.deepStrictEqual(executeSpan.parentSpanId, undefined);
       });
     });
@@ -555,7 +555,7 @@ describe('graphql', () => {
           executeSpan.attributes[AttributeNames.OPERATION_NAME],
           undefined
         );
-        assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);
+        assert.deepStrictEqual(executeSpan.name, 'query');
         assert.deepStrictEqual(executeSpan.parentSpanId, undefined);
       });
     });
@@ -752,7 +752,7 @@ describe('graphql', () => {
           executeSpan.attributes[AttributeNames.OPERATION_NAME],
           undefined
         );
-        assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);
+        assert.deepStrictEqual(executeSpan.name, 'query');
         assert.deepStrictEqual(executeSpan.parentSpanId, undefined);
       });
 
@@ -848,7 +848,7 @@ describe('graphql', () => {
           executeSpan.attributes[AttributeNames.OPERATION_NAME],
           'AddBook'
         );
-        assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);
+        assert.deepStrictEqual(executeSpan.name, 'mutation AddBook');
         assert.deepStrictEqual(executeSpan.parentSpanId, undefined);
       });
 
@@ -946,7 +946,7 @@ describe('graphql', () => {
           executeSpan.attributes[`${AttributeNames.VARIABLES}id`],
           2
         );
-        assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);
+        assert.deepStrictEqual(executeSpan.name, 'query Query1');
         assert.deepStrictEqual(executeSpan.parentSpanId, undefined);
       });
 
@@ -1044,7 +1044,7 @@ describe('graphql', () => {
         executeSpan.attributes[AttributeNames.OPERATION_NAME],
         'AddBook'
       );
-      assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);
+      assert.deepStrictEqual(executeSpan.name, 'mutation AddBook');
       assert.deepStrictEqual(executeSpan.parentSpanId, undefined);
     });
 
@@ -1317,7 +1317,7 @@ describe('graphql', () => {
 
       // validate execute span is present
       const spans = exporter.getFinishedSpans();
-      const executeSpans = spans.filter(s => s.name === SpanNames.EXECUTE);
+      const executeSpans = spans.filter(s => s.name === 'query');
       assert.deepStrictEqual(executeSpans.length, 1);
       const [executeSpan] = executeSpans;
       assert.deepStrictEqual(
@@ -1371,7 +1371,7 @@ describe('graphql', () => {
       );
 
       // single execute span
-      const executeSpans = spans.filter(s => s.name === SpanNames.EXECUTE);
+      const executeSpans = spans.filter(s => s.name === 'query');
       assert.deepStrictEqual(executeSpans.length, 1);
     });
   });