fix(gql): conform GraphQL span name to spec

Rename graphql "execute" spans to "<operation.type> <operation.name>" to conform to the spec. In addition to conforming to the spec, this change can provide additional value for monitoring purposes. In cases where o11y providers generate metrics from pre-ingested traces (without all the attributes), the new naming convention makes it easier to group and filter metrics by operation type and name, since it is now reflected in the span name. This can be particularly useful for setting up monitors or alerts. The query steps other than "execute"'s spans are out of scope as they do not need to be renamed.
open-telemetry · Mar 29, 2023 · 87d3fd2 · 87d3fd2
1 parent 9605528
commit 87d3fd2
Show file tree

Hide file tree

Showing 2 changed files with 22 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,19 +404,20 @@ 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
-      );
+      const operationName = nameNode?.value;
 
-      if (operationDefinition.name) {
-        span.setAttribute(
-          AttributeNames.OPERATION_NAME,
-          operationDefinition.name.value
-        );
+      span.setAttribute(AttributeNames.OPERATION_TYPE, operationType);
+
+      if (operationName) {
+        span.setAttribute(AttributeNames.OPERATION_NAME, operationName);
       }
+
+      // 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>.
+      span.updateName(`${operationType} ${operationName ?? ''}`.trim());
     } else {
       let operationName = ' ';
       if (processedArgs.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);
     });
   });