Skip to content

Commit

Permalink
refactor: Graph Manager (Engine) reporting "extensions" become "plugi…
Browse files Browse the repository at this point in the history
…ns".

Similar to 6009d8a (#3991) and 68cbc93 (#3997), which migrated the
tracing and cache-control extensions to the (newer) request pipeline plugin
API, this commit introduces:

 - Internally, a `plugin` named export which is utilized by the `agent`'s
   `newExtension` method to provide a plugin which is instrumented to
   transmit metrics to Apollo Graph Manager.  This plugin is meant to
   replicate the behavior of the `EngineReportingExtension` class which,
   as of this commit, still lives besides it.
 - Externally, a `federatedPlugin` exported on the main module of the
   `apollo-engine-reporting` package.  This plugin is meant to replicate the
   behavior of the `EngineFederatedTracingExtension` class (also exported on
   the main module) which, again as of this commit, still lives besides it!

Again, the delta of the commits seemed more confusing by allowing a natural
`diff` to be made of it, I've left the extensions in place so they can be
compared - presumably side-by-side in an editor - on the same commit.  An
(immediate) subsequent commit will remove the extension.
  • Loading branch information
abernix committed Apr 16, 2020
1 parent 38c3b20 commit de7ba72
Show file tree
Hide file tree
Showing 9 changed files with 297 additions and 82 deletions.
1 change: 1 addition & 0 deletions packages/apollo-engine-reporting/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"apollo-server-errors": "file:../apollo-server-errors",
"apollo-server-types": "file:../apollo-server-types",
"async-retry": "^1.2.1",
"apollo-server-plugin-base": "file:../apollo-server-plugin-base",
"graphql-extensions": "file:../graphql-extensions"
},
"peerDependencies": {
Expand Down
56 changes: 20 additions & 36 deletions packages/apollo-engine-reporting/src/__tests__/extension.test.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,11 @@
import { makeExecutableSchema, addMockFunctionsToSchema } from 'graphql-tools';
import {
GraphQLExtensionStack,
enableGraphQLExtensions,
} from 'graphql-extensions';
import { graphql, GraphQLError } from 'graphql';
import { Request } from 'node-fetch';
import {
EngineReportingExtension,
makeTraceDetails,
makeHTTPRequestHeaders,
} from '../extension';
import { makeTraceDetails, makeHTTPRequestHeaders, plugin } from '../extension';
import { Headers } from 'apollo-server-env';
import { InMemoryLRUCache } from 'apollo-server-caching';
import { AddTraceArgs } from '../agent';
import { Trace } from 'apollo-engine-reporting-protobuf';
import pluginTestHarness from 'apollo-server-core/dist/utils/pluginTestHarness';

it('trace construction', async () => {
const typeDefs = `
Expand Down Expand Up @@ -53,41 +45,33 @@ it('trace construction', async () => {

const schema = makeExecutableSchema({ typeDefs });
addMockFunctionsToSchema({ schema });
enableGraphQLExtensions(schema);

const traces: Array<AddTraceArgs> = [];
async function addTrace(args: AddTraceArgs) {
traces.push(args);
}

const reportingExtension = new EngineReportingExtension(
{},
addTrace,
'schema-hash',
);
const stack = new GraphQLExtensionStack([reportingExtension]);
const requestDidEnd = stack.requestDidStart({
request: new Request('http://localhost:123/foo'),
queryString: query,
requestContext: {
request: {
query,
operationName: 'q',
extensions: {
clientName: 'testing suite',
},
const pluginInstance = plugin({ /* no options!*/ }, addTrace);

pluginTestHarness({
pluginInstance,
schema,
graphqlRequest: {
query,
operationName: 'q',
extensions: {
clientName: 'testing suite',
},
context: {},
cache: new InMemoryLRUCache(),
http: new Request('http://localhost:123/foo'),
},
executor: async ({ request: { query: source }}) => {
return await graphql({
schema,
source,
});
},
context: {},
});
await graphql({
schema,
source: query,
contextValue: { _extensionStack: stack },
});
requestDidEnd();

// XXX actually write some tests
});

Expand Down
11 changes: 4 additions & 7 deletions packages/apollo-engine-reporting/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ import {
import { fetch, RequestAgent, Response } from 'apollo-server-env';
import retry from 'async-retry';

import { EngineReportingExtension } from './extension';
import { plugin } from './extension';
import { GraphQLRequestContext, Logger, SchemaHash } from 'apollo-server-types';
import { InMemoryLRUCache } from 'apollo-server-caching';
import { defaultEngineReportingSignature } from 'apollo-graphql';
import { ApolloServerPlugin } from "apollo-server-plugin-base";

export interface ClientInfo {
clientName?: string;
Expand Down Expand Up @@ -278,12 +279,8 @@ export class EngineReportingAgent<TContext = any> {
handleLegacyOptions(this.options);
}

public newExtension(schemaHash: SchemaHash): EngineReportingExtension<TContext> {
return new EngineReportingExtension<TContext>(
this.options,
this.addTrace.bind(this),
schemaHash,
);
public newExtension(): ApolloServerPlugin<TContext> {
return plugin(this.options, this.addTrace.bind(this));
}

public async addTrace({
Expand Down
174 changes: 174 additions & 0 deletions packages/apollo-engine-reporting/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import {
SendValuesBaseOptions,
} from './agent';
import { EngineReportingTreeBuilder } from './treeBuilder';
import { ApolloServerPlugin } from "apollo-server-plugin-base";

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

const clientNameHeaderKey = 'apollographql-client-name';
const clientReferenceIdHeaderKey = 'apollographql-client-reference-id';
Expand Down Expand Up @@ -208,6 +211,177 @@ export class EngineReportingExtension<TContext = any>
}
}

// This plugin is instantiated once at server start-up. Each request that the
// server processes will invoke the `requestDidStart` method which will produce
// a trace (in protobuf Trace format) for that single request. When the request
// is done, it passes the Trace back to its associated EngineReportingAgent via
// the addTrace callback. This class isn't for direct use; its constructor is a
// private API for communicating with EngineReportingAgent.
export const plugin = <TContext>(
options: EngineReportingOptions<TContext> = Object.create(null),
addTrace: (args: AddTraceArgs) => Promise<void>,
// schemaHash: string,
): ApolloServerPlugin<TContext> => {
const logger: Logger = options.logger || console;
const generateClientInfo: GenerateClientInfo<TContext> =
options.generateClientInfo || defaultGenerateClientInfo;


return {
requestDidStart(requestContext) {
let queryString: string | undefined;
const treeBuilder: EngineReportingTreeBuilder =
new EngineReportingTreeBuilder({
rewriteError: options.rewriteError,
logger: requestContext.logger || logger,
});

const metrics: NonNullable<typeof requestContext['metrics']> =
((requestContext as Mutable<typeof requestContext>)
.metrics = requestContext.metrics || Object.create(null));

treeBuilder.startTiming();
metrics.startHrTime = treeBuilder.startHrTime;

if (requestContext.request.http) {
treeBuilder.trace.http = new Trace.HTTP({
method:
Trace.HTTP.Method[
requestContext.request.http
.method as keyof typeof Trace.HTTP.Method
] || Trace.HTTP.Method.UNKNOWN,
// Host and path are not used anywhere on the backend, so let's not bother
// trying to parse request.url to get them, which is a potential
// source of bugs because integrations have different behavior here.
// On Node's HTTP module, request.url only includes the path
// (see https://nodejs.org/api/http.html#http_message_url)
// The same is true on Lambda (where we pass event.path)
// But on environments like Cloudflare we do get a complete URL.
host: null,
path: null,
});
}

let preflightDone: boolean = false;
function ensurePreflight() {
if (preflightDone) return;
preflightDone = true;

if (options.sendHeaders) {
if (requestContext.request.http && treeBuilder.trace.http) {
makeHTTPRequestHeaders(
treeBuilder.trace.http,
requestContext.request.http.headers,
options.sendHeaders,
);
}
}

if (metrics.persistedQueryHit) {
treeBuilder.trace.persistedQueryHit = true;
}
if (metrics.persistedQueryRegister) {
treeBuilder.trace.persistedQueryRegister = true;
}

// Generally, we'll get queryString here and not parsedQuery; we only get
// parsedQuery if you're using an OperationStore. In normal cases we'll
// get our documentAST in the execution callback after it is parsed.
queryString = requestContext.source;

if (requestContext.request.variables) {
treeBuilder.trace.details = makeTraceDetails(
requestContext.request.variables,
options.sendVariableValues,
queryString,
);
}

const clientInfo = generateClientInfo(requestContext);
if (clientInfo) {
// While clientAddress could be a part of the protobuf, we'll ignore it for
// now, since the backend does not group by it and Engine frontend will not
// support it in the short term
const { clientName, clientVersion, clientReferenceId } = clientInfo;
// the backend makes the choice of mapping clientName => clientReferenceId if
// no custom reference id is provided
treeBuilder.trace.clientVersion = clientVersion || '';
treeBuilder.trace.clientReferenceId = clientReferenceId || '';
treeBuilder.trace.clientName = clientName || '';
}
}

let endDone: boolean = false;
function didEnd() {
if (endDone) return;
endDone = true;
treeBuilder.stopTiming();

treeBuilder.trace.fullQueryCacheHit = !!metrics.responseCacheHit;
treeBuilder.trace.forbiddenOperation = !!metrics.forbiddenOperation;
treeBuilder.trace.registeredOperation = !!metrics.registeredOperation;

// If the user did not explicitly specify an operation name (which we
// would have saved in `executionDidStart`), but the request pipeline made
// it far enough to figure out what the operation name must be and store
// it on requestContext.operationName, use that name. (Note that this
// depends on the assumption that the RequestContext passed to
// requestDidStart, which does not yet have operationName, will be mutated
// to add operationName later.)
const operationName = requestContext.operationName || '';

// If this was a federated operation and we're the gateway, add the query plan
// to the trace.
if (metrics.queryPlanTrace) {
treeBuilder.trace.queryPlan = metrics.queryPlanTrace;
}

addTrace({
operationName,
queryHash: requestContext.queryHash!,
documentAST: requestContext.document,
queryString,
trace: treeBuilder.trace,
schemaHash: requestContext.schemaHash,
});
}

return {
parsingDidStart() {
ensurePreflight();
},

validationDidStart() {
ensurePreflight();
},

didResolveOperation() {
ensurePreflight();
},

executionDidStart() {
ensurePreflight();
return didEnd;
},

willResolveField(...args) {
const [, , , info] = args;
return treeBuilder.willResolveField(info);
// We could save the error into the trace during the end handler, but
// it won't have all the information that graphql-js adds to it later,
// like 'locations'.
},

didEncounterErrors({ errors }) {
ensurePreflight();
treeBuilder.didEncounterErrors(errors);
didEnd();
},
};
}
};
};

// Helpers for producing traces.

function defaultGenerateClientInfo({ request }: GraphQLRequestContext) {
Expand Down
62 changes: 62 additions & 0 deletions packages/apollo-engine-reporting/src/federatedExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,68 @@ import { Trace } from 'apollo-engine-reporting-protobuf';
import { GraphQLRequestContext } from 'apollo-server-types';

import { EngineReportingTreeBuilder } from './treeBuilder';
import { ApolloServerPlugin } from "apollo-server-plugin-base";
import { EngineReportingOptions } from "./agent";

type FederatedReportingOptions<TContext> = Pick<EngineReportingOptions<TContext>, 'rewriteError'>

export const plugin = <TContext>(
options: FederatedReportingOptions<TContext> = Object.create(null),
): ApolloServerPlugin<TContext> => {
return {
requestDidStart(requestContext) {
const treeBuilder: EngineReportingTreeBuilder =
new EngineReportingTreeBuilder({
rewriteError: options.rewriteError,
});

// XXX Provide a mechanism to customize this logic.
const http = requestContext.request.http;
if (
!http ||
!http.headers ||
http.headers.get('apollo-federation-include-trace') !== 'ftv1'
) {
return;
}

treeBuilder.startTiming();

return {
willResolveField(...args) {
const [ , , , info] = args;
return treeBuilder.willResolveField(info);
},

didEncounterErrors({ errors }) {
treeBuilder.didEncounterErrors(errors);
},

willSendResponse({ response }) {
// We record the end time at the latest possible time: right before serializing the trace.
// If we wait any longer, the time we record won't actually be sent anywhere!
treeBuilder.stopTiming();

const encodedUint8Array = Trace.encode(treeBuilder.trace).finish();
const encodedBuffer = Buffer.from(
encodedUint8Array,
encodedUint8Array.byteOffset,
encodedUint8Array.byteLength,
);

const extensions =
response.extensions || (response.extensions = Object.create(null));

if (typeof extensions.ftv1 !== "undefined") {
throw new Error("The `ftv1` `extensions` were already present.");
}

extensions.ftv1 = encodedBuffer.toString('base64');
}
}
},
}
};

export class EngineFederatedTracingExtension<TContext = any>
implements GraphQLExtension<TContext> {
Expand Down
2 changes: 1 addition & 1 deletion packages/apollo-engine-reporting/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export { EngineReportingOptions, EngineReportingAgent } from './agent';
export { EngineFederatedTracingExtension } from './federatedExtension';
export { plugin as federatedPlugin } from './federatedExtension';
2 changes: 1 addition & 1 deletion packages/apollo-engine-reporting/src/treeBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class EngineReportingTreeBuilder {
};
}

public didEncounterErrors(errors: GraphQLError[]) {
public didEncounterErrors(errors: readonly GraphQLError[]) {
errors.forEach(err => {
if (
err instanceof PersistedQueryNotFoundError ||
Expand Down
1 change: 1 addition & 0 deletions packages/apollo-engine-reporting/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@
{ "path": "../graphql-extensions" },
{ "path": "../apollo-server-errors" },
{ "path": "../apollo-server-types" },
{ "path": "../apollo-server-plugin-base" },
]
}
Loading

0 comments on commit de7ba72

Please sign in to comment.