Ability to run service check against all registered operations

[noaelad]

At my company, we’ve set up our CI environment with service:check and client:check commands, but after a while we realized that the asymmetry between the two is causing us too much pain and we’ve had to disable client:check. The asymmetry problem is as such: client:check validates all operations in the client code, but service:check only validates against operations seen during the validation period. This poses a loophole, since service:check is more permissive than client:check and can sometimes allow breaking changes, which in turn cause the client’s CI suite to go red. A couple of examples include:

1. Code for an un-launched feature exists in the client code. A backend PR erroneously removes a field used by the un-launched feature code. The backend PR passes service:check (because the feature isn't live) and is merged and deployed, subsequently the client’s CI suite is red for all branches (client:check fails).
2. An infrequently-used feature exists in the client code. A backend PR erroneously removes a field used by said feature. The backend PR passes service:check (because the feature hasn’t been used during the validation period) and is merged and deployed, subsequently the client’s CI is red for all branches (client:check fails).

This situation is problematic because it allows backend developers to merge in code that we could have identified as a breaking change earlier, and it causes a lot of noise for all developers working on the frontend client (in our case that's quite a lot of people).

Currently service check only operates against a validation window. However if we could run it instead against a specific list of operations (such as those specified by client:push for a list of supported clients), this would solve the problem.

Let me know if you have any questions!

[jhampton]

Hello @noaelad, thank you for the GH issue here – it's very helpful for discussion with the team at large.

To be clear - are you using variants to track usage across different environments? In your scenario, a client developer would likely be making recent requests to a dev or staging environment that would be caught be service developers making breaking changes.

On the feature front, how would you like to see a given service:check specify which operations to consider from the operations registry?

[jhampton]

Cc @zionts and @JakeDawkins for visibility here.

[noaelad]

We are not using variants, we set our environments up as entirely different graphs for each environment (e.g. GustoProduction is our production graph, GustoStaging is our staging-environment graph, etc) and our devs are not using managed configuration locally i.e. they run the gateway with a service list. We made that choice based on API key separation.

[noaelad]

how would you like to see a given service:check specify which operations to consider from the operations registry?

Being able to specify a list of supported clients, and for each client a list of supported versions, would be ideal. So something like: apollo service:check --supportedClients={gustoWeb: latest, gustoMobile: [1.0,1.1]}

For certain clients (web) we'd be ok with just supporting the latest version, and with other clients (mobile) we'd like to be able to supply a list of supported client versions. But if it helps scope things down, just being able to specify a list of supported client names would already be super useful for us. We could have different versions of some client register its operations under different client names (e.g. have the client names be "mobileClientV1.1", "mobileClientV1.2") to get the same result as specifying client versions.

[jhampton]

@jbaxleyiii I'm capturing this as a product request (A-0003) to be discussed with the Product team.

[noaelad]

Hey @jhampton @jbaxleyiii any news on this? We're blocked on a couple of fronts by a lack of this feature:

1. We can't protect our app from breaking client changes because we've disabled client:check.
2. We're not fully utilizing client:push (because it relies on client:check), which is preventing us from doing safelisting via the operations registry.

[jhampton]

@noaelad We've been working hard to release new features in Studio and a new Explorer, thus the radio silence here.

In reviewing the Operation Registry Plugin (where published client:push operations are surfaced), I believe this could be an extension to the Apollo CLI. (https://github.com/apollographql/apollo-server/blob/3e2a339eb6538f2c26dc84395090e26f8cd45d90/packages/apollo-server-plugin-operation-registry/src/__tests__/agent.test.ts - these are the tests that show the op mgr manifest format).

Because we're planning on some changes related to the newly-launched Explorer feature related to registered operations, I don't believe we'll have a solution to this particular challenge in the short-term. Using the CLI as a base (specifically https://github.com/apollographql/apollo-tooling/tree/master/packages/apollo-language-server), I believe you could add this kind of functionality in the short-term until a general solution is available.

The general flow would be (per-variant):

• Register operations with client:push and continue to use client:check
• Run a normal service:check to catch breaking changes inside the validation period
• Use a custom script to retrieve the registered operations, and check each services' changes against the registered operations, optionally returning a non-zero status if a registered operation is broken

[noaelad]

@jhampton sorry for the late response, this slipped under my radar. The solution you proposed is similar to what we had in mind for a way we can solve this short-term, but we were thinking more along these lines:

• Register operations with client:push and continue to use client:check.
• Use a custom script to retrieve the registered operations, and run each query against the server (or in our case - the gateway) once per validation period. The queries do not have to succeed, just to show up in metrics.
• Run a normal service:check to catch breaking changes inside the validation period.

This way the errors would all still surface from service:check. WDYT?

Thanks for the update and I hope you still consider building more support for this as part of service:check, so that we don't have to run custom scripts to get this working.