Add field usage query analyzer

[cjoudrey]

Follow up from a discussion over Slack with @rmosolgo.

When maintaining a GraphQL API it's important to know what fields are being used, especially deprecated fields.

This adds a simple query analyzer that exposes both used_fields and used_deprecated_fields.

These arrays can then be fed into other systems in order to know if removing a deprecated field is safe and resurfaced to API consumers to let them know they are using a deprecated field.

I'm not sure if this belongs in the repo, but at the very least could serve as an example in docs for someone wanting to build a query analyzer.

@rmosolgo any thoughts?

[rmosolgo]

👍 love it! I have a few thoughts:

• Is it possible to add a query error from the context of this analyzer? Or is there another way to notify the client as a result of using a field? I think you could push an entry into query.context.errors, but I forget if that array holds error instances or hashes.
• I noticed you're pushing one string into used_fields and another into used_deprecated_fields. Can we put the same object in both arrays? I know it's splitting hairs, but this is a "hot path" so I figured I'd ask!
• I wonder if there's benefit to pushing whole objects into the usage arrays, instead of just strings. For example, if you only wanted to log deprecated fields on certain types, then you'd be able to filter the usages. I can imagine introducing some intermediary object (but I can only think of the name FieldUsage 😆 ) that contains references to the field_defn and type_defn. What do you think, any advantage there, or some reason it's no good?

[cjoudrey]

Is it possible to add a query error from the context of this analyzer? Or is there another way to notify the client as a result of using a field? I think you could push an entry into query.context.errors, but I forget if that array holds error instances or hashes.

I'm not sure if you'd want to add a query error. Deprecated fields are meant to be an intermediary state between a field that is available and a field that is removed, no? I believe client libraries should add a warning log when a deprecated field is being queried.

What I meant by "resurfaced to API consumers" was something like a weekly e-mail to API consumers "Your app is using X, Y, Z deprecated fields" or via an admin panel that API consumers have access to.

Can we put the same object in both arrays? I know it's splitting hairs, but this is a "hot path" so I figured I'd ask!

👍 Absolutely!

I wonder if there's benefit to pushing whole objects into the usage arrays, instead of just strings. For example, if you only wanted to log deprecated fields on certain types, then you'd be able to filter the usages.

Good question. I hadn't thought of that as I'm mostly interested in logging all deprecated fields and all used fields and this data is then processed by another system which is why I went with strings.

Alternatively, I suppose we could allow passing a filter lambda when initializing the analyzer. I'm not sure if this should be added right away though. What do you think?

Edit: We could maybe just go with FieldUsageEntry = Struct.new(:type_definition, :field_definition)?

[rmosolgo]

Yes, I guess you're right about errors, that's not really a good way to communicate the deprecation thing.

Happy to leave FieldUsageEntry to a later refactor as YAGNI.

One last thought:

Right now this uses irep_node.definitions which can contain more than one field definiton. For example, I think this one would cause that:

{
  node(id: $myNodeId) {
    ... on Cheese { id }
    ... on Milk   { id }
  }
}

I think this would cause:

Used fields: Milk.id, Cheese.id

That seems right to me: from the query analysis point of view, you don't know which objects will be used at runtime, so you have to mark both as used. And if either one is deprecated, then you want to know, since it is present in the query. Does that sound right to you, is that what you had in mind also?

[rmosolgo]

🤘

[cjoudrey]

🎉

We should probably revisit this once #186 (comment) has landed.

[rmosolgo]

🚢 in 0.18.12