From 552926146c838efd7e2b778ae6fb815e9e304965 Mon Sep 17 00:00:00 2001 From: Dominic Chambers Date: Thu, 24 Feb 2022 22:15:43 +0000 Subject: [PATCH] fix(graphql): fix `graphql.operation.name` field (#903) Co-authored-by: Valentin Marchaud Co-authored-by: Daniel Dyla --- .../src/enums/AttributeNames.ts | 3 +- .../src/instrumentation.ts | 18 +++-- .../test/graphql.test.ts | 68 ++++++++++++++----- 3 files changed, 67 insertions(+), 22 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-graphql/src/enums/AttributeNames.ts b/plugins/node/opentelemetry-instrumentation-graphql/src/enums/AttributeNames.ts index b5a1027a419..d264930721a 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/src/enums/AttributeNames.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/src/enums/AttributeNames.ts @@ -19,7 +19,8 @@ export enum AttributeNames { FIELD_NAME = 'graphql.field.name', FIELD_PATH = 'graphql.field.path', FIELD_TYPE = 'graphql.field.type', - OPERATION = 'graphql.operation.name', + OPERATION_TYPE = 'graphql.operation.type', + OPERATION_NAME = 'graphql.operation.name', VARIABLES = 'graphql.variables.', ERROR_VALIDATION_NAME = 'graphql.validation.error', } diff --git a/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts index 7e0700f6a60..52c7465a2f4 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts @@ -401,10 +401,18 @@ export class GraphQLInstrumentation extends InstrumentationBase { const span = this.tracer.startSpan(SpanNames.EXECUTE, {}); if (operation) { - const name = (operation as graphqlTypes.OperationDefinitionNode) - .operation; - if (name) { - span.setAttribute(AttributeNames.OPERATION, name); + const operationDefinition = + operation as graphqlTypes.OperationDefinitionNode; + span.setAttribute( + AttributeNames.OPERATION_TYPE, + operationDefinition.operation + ); + + if (operationDefinition.name) { + span.setAttribute( + AttributeNames.OPERATION_NAME, + operationDefinition.name.value + ); } } else { let operationName = ' '; @@ -415,7 +423,7 @@ export class GraphQLInstrumentation extends InstrumentationBase { '$operationName$', operationName ); - span.setAttribute(AttributeNames.OPERATION, operationName); + span.setAttribute(AttributeNames.OPERATION_NAME, operationName); } if (processedArgs.document?.loc) { diff --git a/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts b/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts index fb3c6421813..2829792f146 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts @@ -61,7 +61,7 @@ const sourceBookById = ` `; const sourceAddBook = ` - mutation { + mutation AddBook { addBook( name: "Fifth Book" authorIds: "0,2" @@ -155,9 +155,13 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_TYPE], 'query' ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + undefined + ); assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE); assert.deepStrictEqual(executeSpan.parentSpanId, undefined); }); @@ -278,9 +282,13 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_TYPE], 'query' ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + undefined + ); assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE); assert.deepStrictEqual(executeSpan.parentSpanId, undefined); }); @@ -364,9 +372,13 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_TYPE], 'query' ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + 'Query1' + ); assert.deepStrictEqual( executeSpan.attributes[`${AttributeNames.VARIABLES}id`], undefined @@ -456,9 +468,13 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_TYPE], 'query' ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + undefined + ); assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE); assert.deepStrictEqual(executeSpan.parentSpanId, undefined); }); @@ -520,9 +536,13 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_TYPE], 'query' ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + undefined + ); assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE); assert.deepStrictEqual(executeSpan.parentSpanId, undefined); }); @@ -607,9 +627,13 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_TYPE], 'query' ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + undefined + ); assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE); assert.deepStrictEqual(executeSpan.parentSpanId, undefined); }); @@ -664,7 +688,7 @@ describe('graphql', () => { assert.deepStrictEqual( parseSpan.attributes[AttributeNames.SOURCE], '\n' + - ' mutation {\n' + + ' mutation AddBook {\n' + ' addBook(\n' + ' name: "Fifth Book"\n' + ' authorIds: "0,2"\n' + @@ -689,7 +713,7 @@ describe('graphql', () => { assert.deepStrictEqual( executeSpan.attributes[AttributeNames.SOURCE], '\n' + - ' mutation {\n' + + ' mutation AddBook {\n' + ' addBook(\n' + ' name: "Fifth Book"\n' + ' authorIds: "0,2"\n' + @@ -699,9 +723,13 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_TYPE], 'mutation' ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + 'AddBook' + ); assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE); assert.deepStrictEqual(executeSpan.parentSpanId, undefined); }); @@ -785,9 +813,13 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_TYPE], 'query' ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + 'Query1' + ); assert.deepStrictEqual( executeSpan.attributes[`${AttributeNames.VARIABLES}id`], 2 @@ -848,7 +880,7 @@ describe('graphql', () => { assert.deepStrictEqual( parseSpan.attributes[AttributeNames.SOURCE], '\n' + - ' mutation {\n' + + ' mutation AddBook {\n' + ' addBook(\n' + ' name: "*"\n' + ' authorIds: "*"\n' + @@ -873,7 +905,7 @@ describe('graphql', () => { assert.deepStrictEqual( executeSpan.attributes[AttributeNames.SOURCE], '\n' + - ' mutation {\n' + + ' mutation AddBook {\n' + ' addBook(\n' + ' name: "*"\n' + ' authorIds: "*"\n' + @@ -883,9 +915,13 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_TYPE], 'mutation' ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + 'AddBook' + ); assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE); assert.deepStrictEqual(executeSpan.parentSpanId, undefined); }); @@ -1021,7 +1057,7 @@ describe('graphql', () => { it('should attach response hook data to the resulting spans', () => { const querySpan = spans.find( - span => span.attributes['graphql.operation.name'] == 'query' + span => span.attributes[AttributeNames.OPERATION_TYPE] == 'query' ); const instrumentationResult = querySpan?.attributes[dataAttributeName]; assert.deepStrictEqual( @@ -1125,7 +1161,7 @@ describe('graphql', () => { ' }\n' ); assert.deepStrictEqual( - executeSpan.attributes[AttributeNames.OPERATION], + executeSpan.attributes[AttributeNames.OPERATION_NAME], 'Operation "foo" not supported' ); assert.deepStrictEqual(executeSpan.name, SpanNames.EXECUTE);