Merge pull request #3998 from apollographql/abernix/migrate-engine-re…

…porting-exts-to-plugin-api

CHANGELOG.md

@@ -9,6 +9,11 @@ The version headers in this history reflect the versions of Apollo Server itself
 
 > The changes noted within this `vNEXT` section have not been released yet.  New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development.  When a release is being prepared, a new header will be (manually) created below and the appropriate changes within that release will be moved into the new section.
 
+- `apollo-engine-reporting`: The underlying integration of this plugin, which instruments and traces the graph's resolver performance and transmits these metrics to [Apollo Graph Manager](https://engine.apollographql.com/), has been changed from the (soon to be deprecated) `graphql-extensions` API to the new [request pipeline `plugins` API](https://www.apollographql.com/docs/apollo-server/integrations/plugins/). [PR #3998](https://github.com/apollographql/apollo-server/pull/3998)
+
+  _This change should be purely an implementation detail for a majority of users_.  There are, however, some special considerations which are worth noting:
+
+    - The federated tracing plugin's `ftv1` response on `extensions` (which is present on the response from an implementing service to the gateway) is now placed on the `extensions` _after_ the `formatResponse` hook.  Anyone leveraging the `extensions`.`ftv1` data from the `formatResponse` hook will find that it is no longer present at that phase.
 - `apollo-tracing`: This package's internal integration with Apollo Server has been switched from using the soon-to-be-deprecated`graphql-extensions` API to using [the request pipeline plugin API](https://www.apollographql.com/docs/apollo-server/integrations/plugins/).  Behavior should remain otherwise the same.  [PR #3991](https://github.com/apollographql/apollo-server/pull/3991)
 
 ### v2.13.0

packages/apollo-engine-reporting/package.json

@@ -18,7 +18,7 @@
     "apollo-server-errors": "file:../apollo-server-errors",
     "apollo-server-types": "file:../apollo-server-types",
     "async-retry": "^1.2.1",
-    "graphql-extensions": "file:../graphql-extensions"
+    "apollo-server-plugin-base": "file:../apollo-server-plugin-base"
   },
   "peerDependencies": {
     "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0"

packages/apollo-engine-reporting/src/__tests__/extension.test.ts → packages/apollo-engine-reporting/src/__tests__/plugin.test.ts

@@ -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 '../plugin';
 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 = `
@@ -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
 });
 

packages/apollo-engine-reporting/src/agent.ts

@@ -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 './plugin';
 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";
 
 let warnedOnDeprecatedApiKey = false;
 
@@ -251,8 +252,8 @@ export interface AddTraceArgs {
   operationName: string;
   queryHash: string;
   schemaHash: SchemaHash;
-  queryString?: string;
-  documentAST?: DocumentNode;
+  source?: string;
+  document?: DocumentNode;
 }
 
 const serviceHeaderDefaults = {
@@ -328,20 +329,16 @@ 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 newPlugin(): ApolloServerPlugin<TContext> {
+    return plugin(this.options, this.addTrace.bind(this));
   }
 
   public async addTrace({
     trace,
     queryHash,
-    documentAST,
+    document,
     operationName,
-    queryString,
+    source,
     schemaHash,
   }: AddTraceArgs): Promise<void> {
     // Ignore traces that come in after stop().
@@ -368,8 +365,8 @@ export class EngineReportingAgent<TContext = any> {
 
     const signature = await this.getTraceSignature({
       queryHash,
-      documentAST,
-      queryString,
+      document,
+      source,
       operationName,
     });
 
@@ -534,17 +531,17 @@ export class EngineReportingAgent<TContext = any> {
   private async getTraceSignature({
     queryHash,
     operationName,
-    documentAST,
-    queryString,
+    document,
+    source,
   }: {
     queryHash: string;
     operationName: string;
-    documentAST?: DocumentNode;
-    queryString?: string;
+    document?: DocumentNode;
+    source?: string;
   }): Promise<string> {
-    if (!documentAST && !queryString) {
+    if (!document && !source) {
       // This shouldn't happen: one of those options must be passed to runQuery.
-      throw new Error('No queryString or documentAST?');
+      throw new Error('No document or source?');
     }
 
     const cacheKey = signatureCacheKey(queryHash, operationName);
@@ -559,20 +556,20 @@ export class EngineReportingAgent<TContext = any> {
       return cachedSignature;
     }
 
-    if (!documentAST) {
+    if (!document) {
       // We didn't get an AST, possibly because of a parse failure. Let's just
       // use the full query string.
       //
       // XXX This does mean that even if you use a calculateSignature which
       //     hides literals, you might end up sending literals for queries
       //     that fail parsing or validation. Provide some way to mask them
       //     anyway?
-      return queryString as string;
+      return source as string;
     }
 
     const generatedSignature = (
       this.options.calculateSignature || defaultEngineReportingSignature
-    )(documentAST, operationName);
+    )(document, operationName);
 
     // Intentionally not awaited so the cache can be written to at leisure.
     this.signatureCache.set(cacheKey, generatedSignature);