diagnostics channel out for performance checking

[RichardWright]

Questions regarding how to use GraphQL

Hi! I'm currently measuring the time taken for graphql to render and posting the results onto a diagnostics channel. Would a pr be accepted to have this code in the main lib?

nodejs functionality in question is here - https://nodejs.org/dist/latest-v14.x/docs/api/diagnostics_channel.html

It's opt in so users wouldn't having timings if no one is listening for them.

[spencerwilson]

Neat! I'd love to see what you have so far, @RichardWright. (Note: I'm not a maintainer, just a GraphQL user interested in observability.)

The sound of it is reminiscent of https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/opentelemetry-instrumentation-graphql, which monkey-patches graphql's parse, validate, and execute functions to measure spans for schema parse + validation as well as operation parse + validation + resolution of each field (source). One of the goals of OpenTelemetry is that userland packages like graphql would come "pre-instrumented" for rich observability. Some packages have that today, but not graphql, hence @opentelemetry/instrumentation-graphql to monkey-patch it in.

A related question to

Would a PR be accepted to have telemetry emitted to diagnostics_channel?

then, is

Would a PR be accepted to have telemetry emitted to @opentelemetry/api?

Not sure how the overhead, maintainability, value, etc. of these two styles of instrumentation compare. That may be a factor that the maintainers of this package weigh costs/benefits of adding this kind of code to the package.

[spencerwilson]

Ah, there's good context on diagnostics_channel in https://github.com/nodejs/node/pull/34895 and https://github.com/nodejs/diagnostics/issues/180:

the reason for making this a core module and not just something on npm is that the intent is largely to use this to gather diagnostics data from within Node.js core itself, which requires that it exist within Node.js. It is designed as a public API so that some other userland libraries may also make use of it to report diagnostic data. The hope is for an API such as this to largely eliminate the need for APM vendors to patch Node.js core, the module loader and much of the external module ecosystem, which is somewhat fragile and has a significant performance impact.

Perhaps their vision, then, is that modules publish data to a diagnostic channel, and then OpenTelemetry instrumentations can work by subscribing to that channel rather than monkey-patching other packages. That approach is being discussed in https://github.com/open-telemetry/opentelemetry-js/issues/1263.

Ok, going to back away now and let you have this issue again. 😬

[saihaj]

you can do this with envelop https://www.envelop.dev/plugins/use-open-telemetry

[RichardWright]

@saihaj That is much higher level and doesn't have subscriber awareness.

[saihaj]

@saihaj That is much higher level and doesn't have subscriber awareness.

adding something like this to graphql-js we would need to a way to export platform specific module. Cause we also want to support deno #2860 and in browser this really wouldn't make any sense to have. cc @IvanGoncharov