Refine GraphQL metrics

[bclozel]

Now that #16 is done, we should work on refining the metrics and the associated tags.

Right now we're publishing a "graphql.query" timer metrics that's measuring the execution time of the query using a SimpleInstrumentation. We're adding a "query" tag with the actual query, and a "outcome" tag describing the outcome of the query (success or error).

There are quite a few things to refine here:

1. the "query" tag is not right, since it's publishing the raw query. Micrometer tags should be bounded and useful for building graphs. Unfortunately, the number of possible queries sent by clients are unbounded, and they can be very long, making it hard to use as a basis for graphs. Is there a way to derive useful information from the query without those downsides?
2. the "operation" and "errors" informations are not used as a tag. In general, what information could we use as tags?
3. without entering the "tracing" domain, are there other things we could create metrics for?

[mcohen75]

We report metrics across a number of Java and Node.js apps. I'll share some of the things that we've done here that may be useful.

• graphql.request.depth - distribution of the max depth of a GraphQL request.
• graphql.request.complexity - distribution of the computed complexity of the query. This is computed using graphql-cost-analysis in some of our Node.js daemons. Something similar could be done using MaxQueryComplexityInstrumentation from graphql-java.
• graphql.request.parse, graphql.request.validate,graphql.request.execute - Timing for each GraphQL request stage.
• graphql.request.errors - a count of errors tagged with an error code, when present (via the extensions.code field, not part of the spec). It looks like something like this is already done using graphql-java's error classification mechanism here.
• graphql.operation.count - a count of operations tagged with the operation name and operation type.

We also have a couple of teams that record a timer metric for each data fetcher execution.

I think it would be important to avoid using query in the metric name when referring to the overall request. I believe the GraphQL spec uses the term request to describe the overall execution request to disambiguate from query operations.

[mcohen75]

It may also be useful to record metrics that would reveal usage of deprecated schema elements in support of schema evolution.

There's an RFC in the works that would provide a standard way to address elements in the schema:
https://github.com/magicmark/graphql-spec/blob/add_field_coordinates/rfcs/SchemaCoordinates.md

I expect a Counter tagged with the field coordinates of any deprecated field would get the job done.

[bclozel]

Thanks a lot @mcohen75 for your feedback.

We've revisited the arrangement with a step in the right direction; we're not done processing your comments, but we'd like to collect some more feedback from various community members before implementing more advanced metrics.

[mcohen75]

Thanks for following up @bclozel! RE: the name request vs query, there are some changes coming to the GraphQL spec that seek to reduce the ambiguity of the term query in the spec. In many cases, this means using request instead of query.

See here for details.

[rstoyanchev]

Thanks for the pointer @mcohen75. I've refined the use of "query" accordingly in 44a034e except for request input since that's what it's called in the JSON.