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

refactor: Graph Manager (Engine) reporting "extensions" become "plugins". #3998

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
de7ba72
refactor: Graph Manager (Engine) reporting "extensions" become "plugi…
abernix Mar 30, 2020
78a4cb7
fix: Keep special-cased errors (e.g. APQ not found) as unreported.
abernix Apr 14, 2020
c67a6df
eliminate!: Remove now deprecated `EngineReportingExtension`.
abernix Apr 15, 2020
dce4a24
fix!: Rename AERs `newExtension` to `newPlugin` to match new usage.
abernix Apr 27, 2020
a8ab841
no-op: Add back comment about `ftv1` trace format.
abernix Apr 27, 2020
fc05e8d
Use optional chaining when accessing optional `request.http.headers`.
abernix Apr 28, 2020
48eab7e
Ensure `metrics` is present before plugin initialization.
abernix Apr 28, 2020
8ca5d11
Remove guard around `metrics` which is unnecessary after 48eab7efa.
abernix Apr 28, 2020
0658df5
Move `Trace.HTTP` init inside of `ensurePreflight`.
abernix Apr 28, 2020
fc966a4
Remove unnecessary `queryString` assignment and related comment.
abernix Apr 28, 2020
766c738
Destructure some `requestContext` properties for brevity.
abernix Apr 28, 2020
f4aad30
chore!: Use `document` / `source` rather than `documentAST` / `queryS…
abernix Apr 28, 2020
6b0c83c
Expand on comment about the `ftv1` extension.
abernix Apr 28, 2020
48b8c95
Merge branch 'abernix/add-willResolveField-and-didResolveField' into …
abernix Apr 29, 2020
283e82f
Tweak err. message when `ftv1` is already present in `extensions` res…
abernix May 7, 2020
e0949ec
Remove `TODO` comment I suggested I'd remove!
abernix May 7, 2020
0a0bd14
Merge branch 'abernix/add-willResolveField-and-didResolveField' into …
abernix May 7, 2020
1756b37
Merge branch 'abernix/add-schema-and-schemaHash-to-requestContext' in…
abernix May 7, 2020
e813e5b
refactor(tests): Better helpers for APQ tests in intgr. testsuite.
abernix May 7, 2020
d913589
wip didResolveSource
abernix May 7, 2020
39ff086
other stuff
abernix May 7, 2020
4be279d
Merge branch 'abernix/add-schema-and-schemaHash-to-requestContext' in…
abernix May 8, 2020
ba37e68
changelog: #3998
abernix May 8, 2020
e9edd9a
types(e-r): Improve the typings of `didEnd`.
abernix May 8, 2020
b981b59
chore(e-r): Eliminate `ensurePreflight` by using (new) `didResolveSou…
abernix May 8, 2020
6d4777d
changelog: Add note that I believe some new APQ errors are now traced.
abernix May 8, 2020
bacbb17
Revert "changelog: Add note that I believe some new APQ errors are no…
abernix May 8, 2020
59b4013
fix(e-r): Do not keep traces unless we resolve the "source".
abernix May 8, 2020
16e471e
Merge branch 'abernix/add-schema-and-schemaHash-to-requestContext' in…
abernix May 8, 2020
b0948a8
fix: Preserve client-requested `operationName` on op. name resolution…
abernix May 8, 2020
7c8b8e3
Merge branch 'abernix/add-schema-and-schemaHash-to-requestContext' in…
abernix May 8, 2020
62fc270
Switch to new `willResolveField` object parameter, rather position.
abernix May 8, 2020
c73cd16
nit: Add missing closing paren on comment
abernix May 11, 2020
af2d6d8
Merge branch 'abernix/add-willResolveField-and-didResolveField' into …
abernix May 11, 2020
1144c7a
Merge branch 'release-2.14.0' into abernix/migrate-engine-reporting-e…
abernix May 12, 2020
3ccccad
Merge remote-tracking branch 'origin/release-2.14.0' into abernix/mig…
abernix May 12, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions packages/apollo-engine-reporting/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
WithRequired,
Logger,
SchemaHash,
InvalidGraphQLRequestError,
} from 'apollo-server-types';
import { Request, Headers } from 'apollo-server-env';
import {
Expand All @@ -23,6 +24,10 @@ import {
} from './agent';
import { EngineReportingTreeBuilder } from './treeBuilder';
import { ApolloServerPlugin } from "apollo-server-plugin-base";
import {
PersistedQueryNotFoundError,
PersistedQueryNotSupportedError,
} from 'apollo-server-errors';

type Mutable<T> = { -readonly [P in keyof T]: T[P] };

Expand Down Expand Up @@ -373,6 +378,12 @@ export const plugin = <TContext>(
},

didEncounterErrors({ errors }) {
// We don't report some special-cased errors to Graph Manager.
// See the definition of this function for the reasons.
if (allUnreportableSpecialCasedErrors(errors)) {
abernix marked this conversation as resolved.
Show resolved Hide resolved
return;
}

ensurePreflight();
treeBuilder.didEncounterErrors(errors);
didEnd();
Expand All @@ -382,6 +393,39 @@ export const plugin = <TContext>(
};
};

/**
* Previously, prior to the new plugin API, the Apollo Engine Reporting
* mechanism was implemented using `graphql-extensions`, the API for which
* didn't invoke `requestDidStart` until _after_ APQ had been negotiated.
*
* The new plugin API starts its `requestDidStart` _before_ APQ validation and
* various other assertions which weren't included in the `requestDidStart`
* life-cycle, even if they perhaps should be in terms of error reporting.
*
* The new plugin API is able to properly capture such errors within its
* `didEncounterErrors` lifecycle hook, however, for behavioral consistency
* reasons, we will still special-case those errors and maintain the legacy
* behavior to avoid a breaking change. We can reconsider this in a future
* version of Apollo Engine Reporting (AS3, perhaps!).
*
* @param errors A list of errors to scan for special-cased instances.
*/
function allUnreportableSpecialCasedErrors(
errors: readonly GraphQLError[],
): boolean {
return errors.every(err => {
if (
err instanceof PersistedQueryNotFoundError ||
err instanceof PersistedQueryNotSupportedError ||
err instanceof InvalidGraphQLRequestError
) {
return true;
}

return false;
});
}

// Helpers for producing traces.

function defaultGenerateClientInfo({ request }: GraphQLRequestContext) {
Expand Down
14 changes: 1 addition & 13 deletions packages/apollo-engine-reporting/src/treeBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import { GraphQLError, GraphQLResolveInfo, ResponsePath } from 'graphql';
import { Trace, google } from 'apollo-engine-reporting-protobuf';
import {
PersistedQueryNotFoundError,
PersistedQueryNotSupportedError,
} from 'apollo-server-errors';
import { InvalidGraphQLRequestError, Logger } from 'apollo-server-types';
import { Logger } from 'apollo-server-types';

function internalError(message: string) {
return new Error(`[internal apollo-server error] ${message}`);
Expand Down Expand Up @@ -80,14 +76,6 @@ export class EngineReportingTreeBuilder {

public didEncounterErrors(errors: readonly GraphQLError[]) {
errors.forEach(err => {
if (
err instanceof PersistedQueryNotFoundError ||
err instanceof PersistedQueryNotSupportedError ||
err instanceof InvalidGraphQLRequestError
) {
return;
}

// This is an error from a federated service. We will already be reporting
// it in the nested Trace in the query plan.
//
Expand Down