From aff84bba1d391ec2061b8d0121ac8dd36fc1980c Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Sun, 6 Nov 2022 12:49:11 +0200 Subject: [PATCH] feat(graphql): add ignoreTrivialResolveSpans config option (#1256) --- .../README.md | 48 ++++++- .../src/instrumentation.ts | 21 +++- .../src/types.ts | 14 +++ .../src/utils.ts | 23 +++- .../test/graphql.test.ts | 117 +++++++++++++++++- 5 files changed, 212 insertions(+), 11 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-graphql/README.md b/plugins/node/opentelemetry-instrumentation-graphql/README.md index e1ab7553ec1..3f235596e6b 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/README.md +++ b/plugins/node/opentelemetry-instrumentation-graphql/README.md @@ -38,10 +38,11 @@ provider.register(); registerInstrumentations({ instrumentations: [ new GraphQLInstrumentation({ - // optional params + // optional params // allowValues: true, // depth: 2, // mergeItems: true, + // ignoreTrivialResolveSpans: true, }), ], }); @@ -55,8 +56,53 @@ registerInstrumentations({ | mergeItems | boolean | false | Whether to merge list items into a single element. example: `users.*.name` instead of `users.0.name`, `users.1.name` | | | depth | number | -1 | The maximum depth of fields/resolvers to instrument. When set to 0 it will not instrument fields and resolvers. When set to -1 it will instrument all fields and resolvers. | | | allowValues | boolean | false | When set to true it will not remove attributes values from schema source. By default all values that can be sensitive are removed and replaced with "*" | | +| ignoreTrivialResolveSpans | boolean | false | Don't create spans for the execution of the default resolver on object properties. | | responseHook | GraphQLInstrumentationExecutionResponseHook | undefined | Hook that allows adding custom span attributes based on the data returned from "execute" GraphQL action. | | +## Verbosity + +The instrumentation by default will create a span for each invocation of a resolver. + +A resolver is run by graphql for each field in the query response, which can be a lot of spans for objects with many properties, or when lists are involved. + +There are few config options which can be used to reduce the verbosity of the instrumentations. + +They are all disabled by default. User can opt in to any combination of them to contol the amount of spans. + +### ignoreTrivialResolveSpans + +When a resolver function is not defined on the schema for a field, graphql will use the default resolver which just looks for a property with that name on the object. If the property is not a function, it's not very interesting to trace. + +### depth + +The depth is the number of nesting levels of the field, and the following is a query with a depth of 3: + +```json +{ + a { + b { + c + } + } +} +``` + +You can limit the instrumentation to stop recording "resolve" spans after a specific depth is reached. + +- `-1` means no limit. +- `0` means don't record any "resolve" spans. +- `2` for the example above will record a span for resolving "a" and "b" but not "c". + +### mergeItems + +When resolving a field to a list, graphql will execute a resolver for every field in the query on every object in the list. + +When setting mergeItems to `true` it will only record a span for the first invocation of a resolver on each field in the list, marking it's path as "foo.*.bar" instead of "foo.0.bar", "foo.1.bar". + +Notice that all span data only reflects the invocation on the first element. That includes timing, events and status. + +Downstream spans in the context of all resolvers will be child of the first span. + ## Examples Can be found [here](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/examples/graphql) diff --git a/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts index 00538a594d8..b79e1c4a8ad 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/src/instrumentation.ts @@ -183,8 +183,9 @@ export class GraphQLInstrumentation extends InstrumentationBase { args[3], args[4], args[5], - args[6] || defaultFieldResolved, - args[7] + args[6], + args[7], + defaultFieldResolved ); } else { const args = arguments[0] as graphqlTypes.ExecutionArgs; @@ -195,8 +196,9 @@ export class GraphQLInstrumentation extends InstrumentationBase { args.contextValue, args.variableValues, args.operationName, - args.fieldResolver || defaultFieldResolved, - args.typeResolver + args.fieldResolver, + args.typeResolver, + defaultFieldResolved ); } @@ -446,7 +448,8 @@ export class GraphQLInstrumentation extends InstrumentationBase { variableValues: Maybe<{ [key: string]: any }>, operationName: Maybe, fieldResolver: Maybe>, - typeResolver: Maybe> + typeResolver: Maybe>, + defaultFieldResolved: graphqlTypes.GraphQLFieldResolver ): OtelExecutionArgs { if (!contextValue) { contextValue = {}; @@ -463,10 +466,16 @@ export class GraphQLInstrumentation extends InstrumentationBase { typeResolver, }; } + + const isUsingDefaultResolver = fieldResolver == null; + // follows graphql implementation here: + // https://github.com/graphql/graphql-js/blob/0b7daed9811731362c71900e12e5ea0d1ecc7f1f/src/execution/execute.ts#L494 + const fieldResolverForExecute = fieldResolver ?? defaultFieldResolved; fieldResolver = wrapFieldResolver( this.tracer, this._getConfig.bind(this), - fieldResolver + fieldResolverForExecute, + isUsingDefaultResolver ); if (schema) { diff --git a/plugins/node/opentelemetry-instrumentation-graphql/src/types.ts b/plugins/node/opentelemetry-instrumentation-graphql/src/types.ts index 56c72a1ad4d..f1636d15a89 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/src/types.ts @@ -31,6 +31,7 @@ export interface GraphQLInstrumentationConfig extends InstrumentationConfig { * @default false */ allowValues?: boolean; + /** * The maximum depth of fields/resolvers to instrument. * When set to 0 it will not instrument fields and resolvers @@ -38,6 +39,19 @@ export interface GraphQLInstrumentationConfig extends InstrumentationConfig { * @default undefined */ depth?: number; + + /** + * Don't create spans for the execution of the default resolver on object properties. + * + * When a resolver function is not defined on the schema for a field, graphql will + * use the default resolver which just looks for a property with that name on the object. + * If the property is not a function, it's not very interesting to trace. + * This option can reduce noise and number of spans created. + * + * @default false + */ + ignoreTrivialResolveSpans?: boolean; + /** * Whether to merge list items into a single element. * diff --git a/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts b/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts index eb05983be9d..9e183fb2a95 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts @@ -36,6 +36,11 @@ export const isPromise = (value: any): value is Promise => { return typeof value?.then === 'function'; }; +// https://github.com/graphql/graphql-js/blob/main/src/jsutils/isObjectLike.ts +const isObjectLike = (value: unknown): value is { [key: string]: unknown } => { + return typeof value == 'object' && value !== null; +}; + // eslint-disable-next-line @typescript-eslint/no-explicit-any function addInputVariableAttribute(span: api.Span, key: string, variable: any) { if (Array.isArray(variable)) { @@ -365,7 +370,8 @@ export function wrapFieldResolver( getConfig: () => Required, fieldResolver: Maybe< graphqlTypes.GraphQLFieldResolver & OtelPatched - > + >, + isDefaultResolver = false ): graphqlTypes.GraphQLFieldResolver & OtelPatched { if ( (wrappedFieldResolver as OtelPatched)[OTEL_PATCHED_SYMBOL] || @@ -386,6 +392,21 @@ export function wrapFieldResolver( } const config = getConfig(); + // follows what graphql is doing to decied if this is a trivial resolver + // for which we don't need to create a resolve span + if ( + config.ignoreTrivialResolveSpans && + isDefaultResolver && + (isObjectLike(source) || typeof source === 'function') + ) { + const property = (source as any)[info.fieldName]; + // a function execution is not trivial and should be recorder. + // property which is not a function is just a value and we don't want a "resolve" span for it + if (typeof property !== 'function') { + return fieldResolver.call(this, source, args, contextValue, info); + } + } + if (!contextValue[OTEL_GRAPHQL_DATA_SYMBOL]) { return fieldResolver.call(this, source, args, contextValue, info); } diff --git a/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts b/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts index 9828894d893..f5c908c3eae 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts @@ -39,10 +39,15 @@ graphQLInstrumentation.enable(); graphQLInstrumentation.disable(); // now graphql can be required - -import { buildSchema } from 'graphql'; +import { + GraphQLSchema, + GraphQLObjectType, + GraphQLString, + buildSchema, + graphqlSync, +} from 'graphql'; import { buildTestSchema } from './schema'; -import { graphql, graphqlSync } from './graphql-adaptor'; +import { graphql } from './graphql-adaptor'; // Construct a schema, using GraphQL schema language const schema = buildTestSchema(); @@ -578,6 +583,112 @@ describe('graphql', () => { }); }); + describe('when ignoreTrivialResolveSpans is set to true', () => { + beforeEach(() => { + create({ + ignoreTrivialResolveSpans: true, + }); + }); + + afterEach(() => { + exporter.reset(); + graphQLInstrumentation.disable(); + }); + + it('should create span for resolver defined on schema', async () => { + const simpleSchemaWithResolver = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'RootQueryType', + fields: { + hello: { + type: GraphQLString, + resolve() { + return 'world'; + }, + }, + }, + }), + }); + + await graphql({ schema: simpleSchemaWithResolver, source: '{ hello }' }); + const resovleSpans = exporter + .getFinishedSpans() + .filter(span => span.name === SpanNames.RESOLVE); + assert.deepStrictEqual(resovleSpans.length, 1); + const resolveSpan = resovleSpans[0]; + assert(resolveSpan.attributes[AttributeNames.FIELD_PATH] === 'hello'); + }); + + it('should create span for resolver function', async () => { + const schema = buildSchema(` + type Query { + hello: String + } + `); + + const rootValue = { + hello: () => 'world', + }; + + await graphql({ schema, source: '{ hello }', rootValue }); + const resovleSpans = exporter + .getFinishedSpans() + .filter(span => span.name === SpanNames.RESOLVE); + assert.deepStrictEqual(resovleSpans.length, 1); + const resolveSpan = resovleSpans[0]; + assert(resolveSpan.attributes[AttributeNames.FIELD_PATH] === 'hello'); + }); + + it('should NOT create span for resolver property', async () => { + const schema = buildSchema(` + type Query { + hello: String + } + `); + + const rootValue = { + hello: 'world', // regular property, not a function + }; + + await graphql({ schema, source: '{ hello }', rootValue }); + const resovleSpans = exporter + .getFinishedSpans() + .filter(span => span.name === SpanNames.RESOLVE); + assert.deepStrictEqual(resovleSpans.length, 0); + }); + + it('should create resolve span for custom field resolver', async () => { + const schema = buildSchema(` + type Query { + hello: String + } + `); + + const rootValue = { + hello: 'world', // regular property, not a function + }; + + // since we use a custom field resolver, we record a span + // even though the field is a property + const fieldResolver = ( + source: any, + args: any, + context: any, + info: any + ) => { + return source[info.fieldName]; + }; + + await graphql({ schema, source: '{ hello }', rootValue, fieldResolver }); + const resovleSpans = exporter + .getFinishedSpans() + .filter(span => span.name === SpanNames.RESOLVE); + assert.deepStrictEqual(resovleSpans.length, 1); + const resolveSpan = resovleSpans[0]; + assert(resolveSpan.attributes[AttributeNames.FIELD_PATH] === 'hello'); + }); + }); + describe('when allowValues is set to true', () => { describe('AND source is query with param', () => { let spans: ReadableSpan[];