Introduce usage reporting signature and its components #92
Conversation
🦋 Changeset detectedLatest commit: c068530 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
|
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. |
There was a problem hiding this comment.
otherwise looks good though I know that @o0Ignition0o said he did make some changes in the router code? so let's make sure whatever changes those are get integrated.
I spoke with @timbotnik and @zionts about this. We should still plan on pairing this with other changes that affect the signature, of which they have one (not stripping the contents of lists and objects) planned for the next few months hopefully. So no need to do this today. |
|
This looks great! 🎉 I tried to depend on it to see if the router bridge tests still pass, and I m hitting this error: npm i https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.createhash https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.dropunuseddefinitions https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.fetcher https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.hideliterals https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.isnodelike https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.logger https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.operationregistrysignature https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.printwithreducedwhitespace https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.removealiases https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.sortast https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.stripsensitiveliterals https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.usagereportingsignature
npm ERR! code E404
npm ERR! 404 Not Found - GET https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.operationregistrysignature
npm ERR! 404
npm ERR! 404 'https://pkg.csb.dev/apollographql/apollo-utils/commit/b100d75d/@apollo/utils.operationregistrysignature' is not in this registry.
npm ERR! 404 This package name is not valid, because
npm ERR! 404 1. name can only contain URL-friendly characters
npm ERR! 404
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.
npm ERR! A complete log of this run can be found in:
npm ERR! /Users/ignition/.npm/_logs/2022-05-17T10_14_22_476Z-debug-0.logHappy to pair with you so we can try to wire the bridge against this changeset, if the router-bridge tests pass, we should be good! |
trevor-scheer
force-pushed
the
trevor/usage-reporting-signature
branch
from
5512715
to
ee46e01
Compare
There was a problem hiding this comment.
looking good otherwise. also still unclear what the changes @o0Ignition0o made when importing into router are — did you two go over that?
| } | ||
| `), | ||
| ), | ||
| ).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
Why don't you need to install the serializer here?
There was a problem hiding this comment.
It's never a requirement, but good catch (using it here now)
|
@glasser I did go over with @o0Ignition0o, we actually tested out the code in runtime. The changes necessary in router-bridge were actually at the top level function which called the two important additions here ( |
|
Before merging perhaps check that the codesandbox packages can work directly in router and apollo-server? But if that requires a follow-up patch that's fine too. |
|
@o0Ignition0o can we update apollographql/federation-rs#120 with the latest build from codesandbox? Notably: there's only one package now, so you can uninstall the existing ones and run The package name is changed as well, so you'll need to update the imports to be |
|
@trevor-scheer I just did, it looks super neat! |
Introduce a new
@apollo/utils.usagereportingpackage which exports common bits of usage reporting code shared betweenapollo-serverandrouter-bridge.Consequently, this also introduces/updates a few other related packages:
@apollo/utils.removealiases@apollo/utils.stripsensitiveliteralsThese transforms are used by
usagereportingin order to derive the operation signature.For testing purposes, I've also introduced a snapshot serializer for printing GraphQL AST nodes.
@apollo/utils.jest-graphql-ast-serializerThis PR addresses @glasser 's comment apollographql/federation#1841 (comment)
Related: apollographql/federation#1841
cc @o0Ignition0o
TODO:
Consider switching tostripIgnoredCharactersnow. This affects tests (spacing, commas). @glasser maybe you can help me understand the downstream impact w.r.t. Studio that this would have.