pkg:instrumentation-graphql ignoreTrivialResolveSpans doesn't work

[charly22]

What version of OpenTelemetry are you using?

     "@opentelemetry/api": "^1.4.1"

What version of Node are you using?

v18.17.1

What did you do?

I enabled the auto instrumentation with ignoreTrivialResolveSpans option enabled

new GraphQLInstrumentation({
  ignoreTrivialResolveSpans: true
})

Expected:

What did you expect to see?

The resolves that don't have any custom implementation don't generate any span

What did you see instead?

All resolves generate a corresponding span

Additional context

Digging into Apollo's implementation, I found that when it loads its internal plugins, it always wraps the field.resolve with a function that executes the default or the user-defined resolver, as you can see here:

https://github.com/apollographql/apollo-server/blob/f6e3ae021417c3b54200f8d3fcf4366dc3518998/packages/server/src/utils/schemaInstrumentation.ts#L57-L101

For that reason, this condition:

opentelemetry-js-contrib/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts

Lines 327 to 329 in deb9aa4

	if (field.resolve) {
	field.resolve = wrapFieldResolver(tracer, getConfig, field.resolve);
	}

evaluates true for all resolvers (including the default ones) and always ends up instrumenting all field resolvers

[charly22]

Hey @blumamir

Apologies for poking you directly from here, since you're the feature implementor, would you be able to help me?

I think this issue isn't receiving attention because I was unable to tag it properly.

I'm working on a solution but I have nothing concrete yet. At least I have some sort of clue of what's happening. I can provide more details if want. Thanks

[pichlermarc]

@charly22 hmm I see how this is can lead to some very verbose traces. Looking into that there seems to be not much that we can do to work around this. By wrapping the resolve function in this way that Apollo does the resolve function actually becomes non-trivial by the definition we apply here. Looks like the only way to deal with this would be to add a specific apollo instrumentation.

[charly22]

Thanks @pichlermarc for confirming my assumptions. I've been trying to come up with a solution and I found no other than dead ends.

Currently we're using a workaround that consists of filtering all the spans corresponding to scalar field types, which is a bit inconvenient as you might guess but it's the only feasible way we found.

I guess the effort of developing a specific Apollo instrumentation is not trivial, isn't it?
Even though, in which way do you suggest detecting that the resolver is the default one? I don't see that Apollos leaves any record when the resolver is replaced by their default.

Thank you again for looking into this.

[pichlermarc]

Thanks @pichlermarc for confirming my assumptions. I've been trying to come up with a solution and I found no other than dead ends.

Currently we're using a workaround that consists of filtering all the spans corresponding to scalar field types, which is a bit inconvenient as you might guess but it's the only feasible way we found.

I guess the effort of developing a specific Apollo instrumentation is not trivial, isn't it?

Yes, the effort of implementing a specific Apollo instrumentation would not be trivial and would likely require maintaining it indefinitely.

Even though, in which way do you suggest detecting that the resolver is the default one? I don't see that Apollos leaves any record when the resolver is replaced by their default.

Often times there are multiple angles to approach this from; I'm not familiar with Apollo, but in some cases, it's also helpful/necessary to make changes to the upstream repo to get it to work (i.e. make Apollo leave some information that it modified it). Maybe @blumamir knows of an angle to approach this.

[Flarna]

Maybe remove all the resolver spans?
If they are trivial they are of no interest.
If they are not trivial they tend do e.g. some HTTP call, DB request,... which likely creates spans via the corresponding HTTP/DB instrumentation.

[charly22]

The problem is that Apollo's plugins system overrides them to hook additions behavior, but given that there are many built in plugins enabled by default that can't be turned off (metrics, cache), all resolvers end up being defined. Kind of they abused their own plugins system to attach core modules.

I tried to register in the field whether the resolver function is user defined or not but the field type comes from graphql lib.

[seemk]

This is indeed pretty messy, I think ignoring resolvers by default is a decent way to move forward here. The overhead of GraphQL instrumentation currently makes it pretty much unusable in production.

[marco2216]

For our app, I applied these two patches to fix it for now:

@apollo/server

diff --git a/dist/cjs/utils/schemaInstrumentation.js b/dist/cjs/utils/schemaInstrumentation.js
index 3401ddcb3275222733678a9ec6b4c6d3b64ea3c8..c86846d6985d27ccd27c2ced3808f47e66e48f91 100644
--- a/dist/cjs/utils/schemaInstrumentation.js
+++ b/dist/cjs/utils/schemaInstrumentation.js
@@ -31,6 +31,7 @@ function pluginsEnabledForSchemaResolvers(schema) {
 exports.pluginsEnabledForSchemaResolvers = pluginsEnabledForSchemaResolvers;
 function wrapField(field) {
     const originalFieldResolve = field.resolve;
+    field.isTrivialResolver = !originalFieldResolve;
     field.resolve = (source, args, contextValue, info) => {
         const willResolveField = contextValue?.[exports.symbolExecutionDispatcherWillResolveField];
         const userFieldResolver = contextValue?.[exports.symbolUserFieldResolver];

@opentelemetry/instrumentation-grapqhl

diff --git a/build/src/utils.js b/build/src/utils.js
index 85eb3433d332f4e5d07bb206a029d7e45e45988c..f5059456697fd39ccfe9505eaea6ba96552573a0 100644
--- a/build/src/utils.js
+++ b/build/src/utils.js
@@ -225,7 +225,7 @@ function wrapFields(type, tracer, getConfig) {
             return;
         }
         if (field.resolve) {
-            field.resolve = wrapFieldResolver(tracer, getConfig, field.resolve);
+            field.resolve = wrapFieldResolver(tracer, getConfig, field.resolve, field.isTrivialResolver);
         }
         if (field.type) {
             let unwrappedType = field.type;