Low-hanging fruit for performance improvements in usage reporting #7101
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for apollo-server-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
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. Latest deployment of this branch, based on commit f1b6f6c:
|
glasser
force-pushed
the
glasser/usage-reporting-improvements
branch
from
f1eb334 to
2c38afe
Compare
- Drop references to unencoded and uncompressed versions of reports as soon as they are no longer needed, to hopefully allow garbage collection to be more effective during the HTTP POSTs. - Simplify compression code to no longer require a workaround for Node v6 (we require Node v14!), and use `util.promisify`. - Add a timeout to the report POST. The default is 30 seconds, which is currently the timeout enforced by Apollo's load balancers; a smaller number may be advisable for high traffic users facing memory constraints. This uses the AbortController API (which should be supported by most Fetcher implementations). We use the `node-abort-controller` polyfill (same polyfill we chose for `@apollo/gateway`) because Node's built-in global AbortController requires an experimental flag in Node v14. Fixes #7100.
glasser
force-pushed
the
glasser/usage-reporting-improvements
branch
from
2c38afe to
f1b6f6c
Compare
trevor-scheer
approved these changes
Oct 31, 2022
Merged
glasser
added a commit
that referenced
this pull request
Oct 31, 2022
We only require Node 12 here but that's still enough for the zlib API change. We don't bother to add `signal` to the `apollo-server-env` RequestInit. The integration test does show that it works with the default fetcher (`node-fetch` via `apollo-server-env`).
glasser
pushed a commit
that referenced
this pull request
Oct 31, 2022
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @apollo/server-integration-testsuite@4.0.5 ### Patch Changes - Updated dependencies \[[`15d8d65e0`](15d8d65), [`e4e7738be`](e4e7738), [`e4e7738be`](e4e7738), [`15d8d65e0`](15d8d65)]: - @apollo/server@4.0.5 ## @apollo/server@4.0.5 ### Patch Changes - [#7104](#7104) [`15d8d65e0`](15d8d65) Thanks [@glasser](https://github.com/glasser)! - New `ApolloServerPluginSchemaReportingDisabled` plugin which can override the `APOLLO_SCHEMA_REPORTING` environment variable. - [#7101](#7101) [`e4e7738be`](e4e7738) Thanks [@glasser](https://github.com/glasser)! - Manage memory more efficiently in the usage reporting plugin by allowing large objects to be garbage collected more quickly. - [#7101](#7101) [`e4e7738be`](e4e7738) Thanks [@glasser](https://github.com/glasser)! - The usage reporting plugin now defaults to a 30 second timeout for each attempt to send reports to Apollo Server instead of no timeout; the timeout can be adjusted with the new `requestTimeoutMs` option to `ApolloServerPluginUsageReporting`. (Apollo's servers already enforced a 30 second timeout, so this is unlikely to break any existing use cases.) - [#7104](#7104) [`15d8d65e0`](15d8d65) Thanks [@glasser](https://github.com/glasser)! - It is now an error to combine a "disabled" plugin such as `ApolloServerPluginUsageReportingDisabled` with its enabled counterpart such as `ApolloServerPluginUsageReporting`. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
glasser
added a commit
that referenced
this pull request
Oct 31, 2022
We only require Node 12 here but that's still enough for the zlib API change. We don't bother to add `signal` to the `apollo-server-env` RequestInit. The integration test does show that it works with the default fetcher (`node-fetch` via `apollo-server-env`).
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
util.promisify.currently the timeout enforced by Apollo's load balancers; a smaller
number may be advisable for high traffic users facing memory
constraints. This uses the AbortController API (which should be
supported by most Fetcher implementations). We use the
node-abort-controllerpolyfill (same polyfill we chose for@apollo/gateway) because Node's built-in global AbortControllerrequires an experimental flag in Node v14.
Fixes #7100.