feat: include query plan object in extensions

[samuelAndalon]

for debugging purposes include the QueryPlan object, the prettified version of the query plan is hard to read and using the actual source will allow speeding up debugging.

additionally, query plan will help us to get the exact query that a subgraph is receiving which will allow us to run a warming up routine during deployments

[netlify]

👷 Deploy request for apollo-federation-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	343e3ca

[changeset-bot]

🦋 Changeset detected

Latest commit: 343e3ca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@apollo/gateway	Patch
@apollo/federation-internals	Patch
@apollo/composition	Patch
@apollo/query-planner	Patch
@apollo/query-graphs	Patch
@apollo/subgraph	Patch
apollo-federation-integration-testsuite	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[pcmanus]

I agree that having the plan in it's structured/original form is likely easier/better for a number of use cases, and in fact, this old TODO comment (which should ideally be removed/updated here) suggests this was the plan all along.

However, I also think this needs to be behind a new option, not unconditionally shipped alongside the serialized version, because shipping both is unnecessarily wasteful. And while I understand that shipping plans is only an option that is probably only used for debugging/education in the first place, it remains that some users have some large queries for which the plans are fairly large, and the "object" version is going to be even bigger (potentially substantially because the selection sets representation will be a lot less compact), so this might plausibly impact the responsiveness of current existing use cases, and I don't thing that's fair.

Another concern I might have is that shipping the plan "raw" object as done here will expose the underlying selection sets as GraphQL-js encodes them, which:

1. is very much not compact;
2. might not be very convenient if you try to manipulate the result in another language than JS/TS (having the graphQL string is probably a lot easier in those case as any decent language will have a graphQL parser;
3. is easy to produce in the gateway because it's using graphQL-js, but it might make less sense in the router for instance.
4. shipping the internal structure of the plan externally is imo risky: we haven't so far intended the internal query plan structure to be something stable and to make backward compatibility guarantees on it. I'm uncomfortable making it really hard to change the plan structure all of a sudden.

All that to say that, while I do agree the current prettified version is probably not ideal as it forces some pretty custom parsing for any tooling that want to use it, I also feel that a middle ground format where the plan structure is json but the selection sets of fetches are kept as graphql strings, could maybe be a better "default" longer term. And having a transformation between the internal structure and the external format also make it possible to modify the internal structure without breaking the externally exposed format. I do get that this is a bit more work initially obviously but ...

That said, a suggestion could maybe be to start by supporting a Apollo-Query-Plan-Experimental-Options header as a flexible way to control this. For this specific ask, it could accept a format=internal or format=prettified to choose which format to include when Apollo-Query-Plan-Experimental is also set (format=prettified would be the default for now). The "internal" format could then just be plan object as in the current commit, but:

• we'd clearly set the expectation that it is not a stable format, and may not be supported by all implementations (aka, not by the router).
• it would leave room for a futur potential more stable middle-ground/less-graphQL-js-specific format as discussed above.

[samuelAndalon]

I have added the changes to allow a Apollo-Query-Plan-Experimental-Format header,
the advice of using Apollo-Query-Plan-Experimental-Options and value of format=internal would require to add separators in case more options are added in the future, i would say, its easier to just use separate headers

[jeffjakub]

Thanks for updating the pull request @samuelAndalon! The team will go over it and give you feedback soon.

[samuelAndalon]

https://github.com/apollographql/router/blob/main/apollo-router/src/plugins/expose_query_plan.rs#L79

this is also done in the router -- so, adding some feature parity.

[trevor-scheer]

I have added the changes to allow a Apollo-Query-Plan-Experimental-Format header,
the advice of using Apollo-Query-Plan-Experimental-Options and value of format=internal would require to add separators in case more options are added in the future, i would say, its easier to just use separate headers

I don't think parsing separators adds much complexity, and having just one ...-Options header seems preferable over a bunch of numerous, verbose Apollo-Query-Plan-Experimental-* headers. The former is my preference but I don't feel that strongly about it.

[pcmanus]

I don't think parsing separators adds much complexity, and having just one ...-Options header seems preferable over a bunch of numerous, verbose Apollo-Query-Plan-Experimental-* headers. The former is my preference but I don't feel that strongly about it.

Discussed this offline with Trevor, and agreed to move forward with the PR as is, as how the header look like exactly don't matter all that much probably.