Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

studio: extend existing router-bridge with more metadata #424

Closed
3 tasks
garypen opened this issue Feb 9, 2022 · 9 comments · Fixed by apollographql/federation-rs#106 or #898
Closed
3 tasks

studio: extend existing router-bridge with more metadata #424

garypen opened this issue Feb 9, 2022 · 9 comments · Fixed by apollographql/federation-rs#106 or #898
Assignees
Labels
component/studio-reporting Studio and GraphOS Metric Reporting (not analytics, but Studio usage)

Comments

@garypen
Copy link
Contributor

garypen commented Feb 9, 2022

Requirements

This proposes that we expose more information from the existing router-bridge:

  • Signature for reporting purposes
  • Parse/Validation
  • Possibly field usage data

Original text:

A lot of the data required to generate an appropriate signature in the router usage reporting could be accessible across the router bridge. As long as the router-bridge is a requirement in the router, we may as well use this data to fill the gap in functionality until we can totally replace the router-bridge with a rust implementation.

To paraphrase @glasser:

Extending the main router-bridge interface from "returns a query plan" to "returns all the strongly cacheable information that the router needs about the (operation, schema) tuple". Which includes the stats report key, referenced fields list, whether or not validation passed (though that might not be needed if stats report key calculation is in JS), etc. This will mean you can get these calculations consistent with Gateway immediately without having to wait on apollo-rs to give you more tools.

At the very least you'll need to get the "does the validation algorithm pass" boolean from JS because I don't think apollo-rs is anywhere near implementing the 28 validation algorithms defined in the GraphQL spec.

@abernix
Copy link
Member

abernix commented Feb 14, 2022

relates to #48!

@abernix abernix added component/studio-reporting Studio and GraphOS Metric Reporting (not analytics, but Studio usage) and removed component/agent labels Mar 18, 2022
@glasser
Copy link
Member

glasser commented Mar 21, 2022

Howdy team — I'm probably the company's main usage reporting protocol expert and reviewed @garypen 's first round here. I believe @abernix agrees with me that we'll need to get this done for router GA. I'm happy to prioritize reviewing (or even pairing) on this if that's helpful! If you're going to work on this drop me a Slack DM in case there's some more context I can share.

@abernix abernix changed the title extend existing router-bridge interface to help with studio usage reporting studio: extend existing router-bridge to return reporting signature Mar 23, 2022
@abernix
Copy link
Member

abernix commented Mar 23, 2022

@abernix abernix changed the title studio: extend existing router-bridge to return reporting signature studio: extend existing router-bridge with more metadata Mar 23, 2022
@abernix abernix removed the triage label Mar 24, 2022
@glasser
Copy link
Member

glasser commented Mar 25, 2022

Correct. Feel free to copy-and-paste this into whatever repo for now — let me know where it ends up and I'll eventually factor it out to its own tiny npm package in https://github.com/apollographql/apollo-utils so there isn't a long-term duplication.

@glasser
Copy link
Member

glasser commented Mar 25, 2022

(Though note that the full logic includes checks for special error cases and an extra line on top with the operation name; see https://github.com/apollographql/apollo-server/blob/d75c6cf3360a46ebcd944b2113438be8f549ae6f/packages/apollo-server-core/src/plugin/usageReporting/plugin.ts#L646-L672 )

@o0Ignition0o o0Ignition0o self-assigned this Mar 29, 2022
@o0Ignition0o
Copy link
Contributor

I've been able to get the usageReportingSignature in this wip pr but it looks like there's more to it so I'll try to get this information as well!

@o0Ignition0o
Copy link
Contributor

@glasser do you happen to know if introspection queries are subject to usage reporting? I wouldn't expect anything when it comes to referenced fields, but a signature could come in handy ?

@glasser
Copy link
Member

glasser commented Apr 22, 2022

There's not actually such thing as an "introspection query". There are merely introspection fields. One could choose to send operations that both run normal app-level code and randomly also ask questions about the schema.

So we report introspection operations in exactly the same way as any other operation, because there's not actually a distinction.

A different question is how are the introspection fields such as Query.__schema treated by our field-specific features. As far as referencing fields are concerned, it looks like we do include these fields in the referenced fields list, though we should have a test for that! (That said, while I do believe this data makes it to Studio's databases, I don't think they actually show up on the Fields page, since that is driven by "iterate over the schema and associate data with each field we find" and the introspection types are not part of the schema that we're looking at.)

On the other hand, the mechanism we use in Apollo Server/graphql-js to instrument field execution doesn't let us see introspection fields, so they are not included in traces or in executed field lists. It's imaginable that non-graphql-js field instrumentation implementations might do something different here (eg graphql-js/federation-jvm).

@glasser
Copy link
Member

glasser commented Apr 22, 2022

(I added a test on the version-4 branch: apollographql/apollo-server@88f6c2c)

o0Ignition0o added a commit to apollographql/federation-rs that referenced this issue May 2, 2022
Fixes apollographql/router#424

This PR introduces usage reporting to the query planner.
It contains the stats_report_key as well as the Referenced fields.
@BrynCooke BrynCooke linked a pull request May 9, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/studio-reporting Studio and GraphOS Metric Reporting (not analytics, but Studio usage)
Projects
None yet
4 participants