From 51c26a071ec73d9b74aa15616d772299eca1f4e8 Mon Sep 17 00:00:00 2001 From: Jon C <467023+theJC@users.noreply.github.com> Date: Mon, 23 Oct 2023 12:05:22 -0500 Subject: [PATCH] Add operation name attribute on gateway.plan spans (#2807) --- .changeset/lemon-bees-stare.md | 6 + .../__snapshots__/opentelemetry.test.ts.snap | 12 +- .../__tests__/gateway/opentelemetry.test.ts | 109 +++++++++--------- gateway-js/src/index.ts | 17 ++- gateway-js/src/utilities/opentelemetry.ts | 57 +++++---- 5 files changed, 118 insertions(+), 83 deletions(-) create mode 100644 .changeset/lemon-bees-stare.md diff --git a/.changeset/lemon-bees-stare.md b/.changeset/lemon-bees-stare.md new file mode 100644 index 000000000..38fe3c04f --- /dev/null +++ b/.changeset/lemon-bees-stare.md @@ -0,0 +1,6 @@ +--- +"@apollo/gateway": minor +--- + +Add graphql.operation.name attribute on gateway.plan span + \ No newline at end of file diff --git a/gateway-js/src/__tests__/gateway/__snapshots__/opentelemetry.test.ts.snap b/gateway-js/src/__tests__/gateway/__snapshots__/opentelemetry.test.ts.snap index 92911f4bf..2f99acbc6 100644 --- a/gateway-js/src/__tests__/gateway/__snapshots__/opentelemetry.test.ts.snap +++ b/gateway-js/src/__tests__/gateway/__snapshots__/opentelemetry.test.ts.snap @@ -21,7 +21,9 @@ exports[`opentelemetry receives spans on fetch failure 1`] = ` }, { "name": "gateway.plan", - "attributes": {}, + "attributes": { + "graphql.operation.name": "GetProduct" + }, "children": [], "status": { "code": 0 @@ -83,7 +85,9 @@ exports[`opentelemetry with local data receives spans on plan failure 1`] = ` }, { "name": "gateway.plan", - "attributes": {}, + "attributes": { + "graphql.operation.name": "GetProduct" + }, "children": [], "status": { "code": 2 @@ -118,7 +122,9 @@ exports[`opentelemetry with local data receives spans on success 1`] = ` }, { "name": "gateway.plan", - "attributes": {}, + "attributes": { + "graphql.operation.name": "GetProduct" + }, "children": [], "status": { "code": 0 diff --git a/gateway-js/src/__tests__/gateway/opentelemetry.test.ts b/gateway-js/src/__tests__/gateway/opentelemetry.test.ts index cb15b091d..568c20b64 100644 --- a/gateway-js/src/__tests__/gateway/opentelemetry.test.ts +++ b/gateway-js/src/__tests__/gateway/opentelemetry.test.ts @@ -42,14 +42,11 @@ describe('opentelemetry', () => { }); } - describe('with local data', () => - { - async function gateway( - telemetryConfig?: { - includeDocument?: boolean, - recordExceptions?: boolean | number - } - ) { + describe('with local data', () => { + async function gateway(telemetryConfig?: { + includeDocument?: boolean; + recordExceptions?: boolean | number; + }) { const localDataSources = Object.fromEntries( fixtures.map((f) => [ f.name, @@ -63,20 +60,18 @@ describe('opentelemetry', () => { }, telemetry: telemetryConfig || { includeDocument: true, - recordExceptions: true - } + recordExceptions: true, + }, }); const { executor } = await gateway.load(); return executor; } - const executeValidationFailure = async ( - telemetryConfig?: { - includeDocument?: boolean, - recordExceptions?: boolean | number - } - ) => { + const executeValidationFailure = async (telemetryConfig?: { + includeDocument?: boolean; + recordExceptions?: boolean | number; + }) => { const executor = await gateway(telemetryConfig); const source = `#graphql @@ -87,7 +82,7 @@ describe('opentelemetry', () => { }`; await execute(executor, source, { upc: '1' }, 'InvalidVariables'); - } + }; it('receives spans on success', async () => { const executor = await gateway(); @@ -100,23 +95,24 @@ describe('opentelemetry', () => { } `; - await execute(executor, source, {upc: '1'}, 'GetProduct'); - const spans = inMemorySpans.getFinishedSpans() + await execute(executor, source, { upc: '1' }, 'GetProduct'); + const spans = inMemorySpans.getFinishedSpans(); expect(spans).toMatchSnapshot(); spans.forEach((span) => { - expect(span.events).toStrictEqual([]) - }) + expect(span.events).toStrictEqual([]); + }); }); it('receives spans on validation failure', async () => { await executeValidationFailure(); - const spans = inMemorySpans.getFinishedSpans() + const spans = inMemorySpans.getFinishedSpans(); expect(spans).toMatchSnapshot(); - const validationSpan = spans.find((span) => - span.name === 'gateway.validate'); + const validationSpan = spans.find( + (span) => span.name === 'gateway.validate', + ); - expect(validationSpan?.events.length).toEqual(2) + expect(validationSpan?.events.length).toEqual(2); }); it('receives spans on plan failure', async () => { @@ -130,46 +126,46 @@ describe('opentelemetry', () => { `; try { - await execute(executor, source, {upc: '1'}, 'GetProduct'); - } - catch(err) {} - const spans = inMemorySpans.getFinishedSpans() + await execute(executor, source, { upc: '1' }, 'GetProduct'); + } catch (err) {} + const spans = inMemorySpans.getFinishedSpans(); expect(spans).toMatchSnapshot(); - const planSpan = spans.find((span) => - span.name === 'gateway.plan'); + const planSpan = spans.find((span) => span.name === 'gateway.plan'); - expect(planSpan?.events.length).toEqual(1) + expect(planSpan?.events.length).toEqual(1); }); describe('with recordExceptions set to a number', () => { it('receives at most that number of exception events', async () => { - await executeValidationFailure({recordExceptions: 1}) - const spans = inMemorySpans.getFinishedSpans() - const validationSpan = spans.find((span) => - span.name === 'gateway.validate'); + await executeValidationFailure({ recordExceptions: 1 }); + const spans = inMemorySpans.getFinishedSpans(); + const validationSpan = spans.find( + (span) => span.name === 'gateway.validate', + ); - expect(validationSpan?.events.length).toEqual(1) - }) - }) + expect(validationSpan?.events.length).toEqual(1); + }); + }); describe('with recordExceptions set to false', () => { it('receives no exception events', async () => { - await executeValidationFailure({recordExceptions: false}) - const spans = inMemorySpans.getFinishedSpans() - const validationSpan = spans.find((span) => - span.name === 'gateway.validate'); + await executeValidationFailure({ recordExceptions: false }); + const spans = inMemorySpans.getFinishedSpans(); + const validationSpan = spans.find( + (span) => span.name === 'gateway.validate', + ); - expect(validationSpan?.events.length).toEqual(0) - }) - }) + expect(validationSpan?.events.length).toEqual(0); + }); + }); describe('with includeDocument set to false', () => { it('does not include the source document', async () => { - await executeValidationFailure({recordExceptions: false}) - const spans = inMemorySpans.getFinishedSpans() + await executeValidationFailure({ recordExceptions: false }); + const spans = inMemorySpans.getFinishedSpans(); expect(spans).toMatchSnapshot(); - }) - }) + }); + }); }); it('receives spans on fetch failure', async () => { @@ -180,8 +176,8 @@ describe('opentelemetry', () => { }, telemetry: { includeDocument: true, - recordExceptions: true - } + recordExceptions: true, + }, }); const { executor } = await gateway.load(); @@ -194,12 +190,11 @@ describe('opentelemetry', () => { } `; - await execute(executor, source, {upc: '1'}, 'GetProduct'); - const spans = inMemorySpans.getFinishedSpans() + await execute(executor, source, { upc: '1' }, 'GetProduct'); + const spans = inMemorySpans.getFinishedSpans(); expect(spans).toMatchSnapshot(); - const fetchSpan = spans.find((span) => - span.name === 'gateway.fetch'); + const fetchSpan = spans.find((span) => span.name === 'gateway.fetch'); - expect(fetchSpan?.events.length).toEqual(1) + expect(fetchSpan?.events.length).toEqual(1); }); }); diff --git a/gateway-js/src/index.ts b/gateway-js/src/index.ts index 83814dacb..4254947db 100644 --- a/gateway-js/src/index.ts +++ b/gateway-js/src/index.ts @@ -41,7 +41,14 @@ import { SupergraphManager, } from './config'; import { SpanStatusCode } from '@opentelemetry/api'; -import { OpenTelemetrySpanNames, tracer, requestContextSpanAttributes, operationContextSpanAttributes, recordExceptions } from './utilities/opentelemetry'; +import { + OpenTelemetrySpanNames, + tracer, + requestContextSpanAttributes, + operationContextSpanAttributes, + recordExceptions, + OpenTelemetryAttributeNames +} from './utilities/opentelemetry'; import { addExtensions } from './schema-helper/addExtensions'; import { IntrospectAndCompose, @@ -779,6 +786,14 @@ export class ApolloGateway implements GatewayInterface { if (!queryPlan) { queryPlan = tracer.startActiveSpan( OpenTelemetrySpanNames.PLAN, + requestContext.operationName + ? { + attributes: { + [OpenTelemetryAttributeNames.GRAPHQL_OPERATION_NAME]: + requestContext.operationName, + }, + } + : {}, (span) => { try { const operation = operationFromDocument( diff --git a/gateway-js/src/utilities/opentelemetry.ts b/gateway-js/src/utilities/opentelemetry.ts index dc04e33ff..346c61aca 100644 --- a/gateway-js/src/utilities/opentelemetry.ts +++ b/gateway-js/src/utilities/opentelemetry.ts @@ -5,27 +5,23 @@ import { OperationContext } from '../operationContext'; export type OpenTelemetryConfig = { /** - * Whether or not to include the `graphql.document` attribute in the - * `gateway.request` OpenTelemetry span. When set to `true`, the attribute - * will contain the entire GraphQL document for the current request. + * Whether to include the `graphql.document` attribute in the `gateway.request` OpenTelemetry spans. + * When set to `true`, the attribute will contain the entire GraphQL document for the current request. * - * Defaults to `false`, meaning that the GraphQL document will not be added - * as a span attribute. + * Defaults to `false`, meaning that the GraphQL document will not be added as a span attribute. */ includeDocument?: boolean; /** - * Whether or not to record the GraphQL and internal errors that take place - * while processing a request as exception events in the OpenTelemetry spans - * in which they occur. + * Whether to record the GraphQL and internal errors that take place while processing a request as + * exception events in the OpenTelemetry spans in which they occur. * - * When a number is given as a value, it represents the maximum number of - * exceptions that will be reported in each OpenTelemetry span. + * When a number is given as a value, it represents the maximum number of exceptions that will be + * reported in each OpenTelemetry span. * - * Regardless of the value of this setting, the span status code will be set - * to `ERROR` when a GraphQL or internal error occurs. + * Regardless of the value of this setting, the span status code will be set to `ERROR` when a GraphQL + * or internal error occurs. * - * Defaults to `false`, meaning that no exceptions will be reported in any - * spans. + * Defaults to `false`, meaning that no exceptions will be reported in any spans. */ recordExceptions?: boolean | number; } @@ -39,6 +35,18 @@ export enum OpenTelemetrySpanNames { VALIDATE = 'gateway.validate', } +/* + When adding any more, please refer to: + https://opentelemetry.io/docs/specs/otel/common/attribute-naming/ + https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/instrumentation/graphql/ +*/ +export enum OpenTelemetryAttributeNames { + GRAPHQL_DOCUMENT = 'graphql.document', + GRAPHQL_OPERATION_NAME = 'graphql.operation.name', + GRAPHQL_OPERATION_NAME_DEPRECATED = 'operationName', // deprecated in favor of GRAPHQL_OPERATION_NAME + GRAPHQL_OPERATION_TYPE = 'graphql.operation.type', +} + const { name, version } = require('../../package.json'); export const tracer = opentelemetry.trace.getTracer(`${name}/${version}`); @@ -46,10 +54,10 @@ export interface SpanAttributes extends Attributes { /** * @deprecated in favor of `graphql.operation.name` */ - operationName?: string; - 'graphql.operation.name'?: string; - 'graphql.operation.type'?: string; - 'graphql.document'?: string; + [OpenTelemetryAttributeNames.GRAPHQL_OPERATION_NAME_DEPRECATED]?: string; + [OpenTelemetryAttributeNames.GRAPHQL_OPERATION_NAME]?: string; + [OpenTelemetryAttributeNames.GRAPHQL_OPERATION_TYPE]?: string; + [OpenTelemetryAttributeNames.GRAPHQL_DOCUMENT]?: string; } export function requestContextSpanAttributes( @@ -59,11 +67,15 @@ export function requestContextSpanAttributes( const spanAttributes: SpanAttributes = {}; if (requestContext.operationName) { - spanAttributes["operationName"] = requestContext.operationName; - spanAttributes["graphql.operation.name"] = requestContext.operationName; + spanAttributes[ + OpenTelemetryAttributeNames.GRAPHQL_OPERATION_NAME_DEPRECATED + ] = requestContext.operationName; + spanAttributes[OpenTelemetryAttributeNames.GRAPHQL_OPERATION_NAME] = + requestContext.operationName; } if (config?.includeDocument && requestContext.source) { - spanAttributes["graphql.document"] = requestContext.source; + spanAttributes[OpenTelemetryAttributeNames.GRAPHQL_DOCUMENT] = + requestContext.source; } return spanAttributes; @@ -75,7 +87,8 @@ export function operationContextSpanAttributes( const spanAttributes: SpanAttributes = {}; if (operationContext.operation.operation) { - spanAttributes["graphql.operation.type"] = operationContext.operation.operation; + spanAttributes[OpenTelemetryAttributeNames.GRAPHQL_OPERATION_TYPE] = + operationContext.operation.operation; } return spanAttributes;