Fix missing operationName in apollo-engine-reporting. (#2557)

* fix: Set `operationName` via `executionDidStart` and `willResolveField`.

The full-query caching feature implemented in
#2437 originally took the
additional steps of also changing the `apollo-engine-reporting` module to
utilize the new request pipeline available in Apollo Server as of
#1795.

While that would have been nice, there was still some uncertainty about some
of the life-cycle hooks that would be necesssary to make that happen and it
wasn't worth blocking the implementation of full-query caching on those
stalled decisions.

Therefore, the changes to utilize new functionality in the request pipeline,
including what would have been a simplification of the way that
`apollo-engine-reporting` would have obtained the `operationName` (something
that is available via the API of the request-pipeline hooks), were backed
out and will land separately in a future PR.

The portion regarding `operationName` was inadvertently not backed out and
instead was left leveraging a life-cycle hook which was not available to the
`graphql-extensions` API: `didResolveOperation`.

This means that the code was not setting `operationName` in the way it
needed to and therefore `operationName` was always being left undefined (as
is sometimes permitted!) with this new `apollo-engine-reporting`.

This commit puts the functionality back to that which is required for the
`graphql-extensions` implementation, and the commit before this (8a43341)
acts as a regression test (it should pass, as of this commit, and fail
before it).

* Add a regression test for `operationName` not being defined.

This should fail and then be fixed with an upcoming commit.

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

@@ -5,6 +5,7 @@ import {
   responsePathAsArray,
   ResponsePath,
   DocumentNode,
+  ExecutionArgs,
   GraphQLError,
 } from 'graphql';
 import {
@@ -237,13 +238,21 @@ export class EngineReportingExtension<TContext = any>
     };
   }
 
-  public didResolveOperation(o: {
-    requestContext: GraphQLRequestContext<TContext>;
-  }) {
-    const { requestContext } = o;
-
-    this.operationName = requestContext.operationName;
-    this.documentAST = requestContext.document;
+  public executionDidStart(o: { executionArgs: ExecutionArgs }) {
+    // If the operationName is explicitly provided, save it. If there's just one
+    // named operation, the client doesn't have to provide it, but we still want
+    // to know the operation name so that the server can identify the query by
+    // it without having to parse a signature.
+    //
+    // Fortunately, in the non-error case, we can just pull this out of
+    // the first call to willResolveField's `info` argument.  In an
+    // error case (eg, the operationName isn't found, or there are more
+    // than one operation and no specified operationName) it's OK to continue
+    // to file this trace under the empty operationName.
+    if (o.executionArgs.operationName) {
+      this.operationName = o.executionArgs.operationName;
+    }
+    this.documentAST = o.executionArgs.document;
   }
 
   public willResolveField(
@@ -252,6 +261,11 @@ export class EngineReportingExtension<TContext = any>
     _context: TContext,
     info: GraphQLResolveInfo,
   ): ((error: Error | null, result: any) => void) | void {
+    if (this.operationName === undefined) {
+      this.operationName =
+        (info.operation.name && info.operation.name.value) || '';
+    }
+
     const path = info.path;
     const node = this.newNode(path);
     node.type = info.returnType.toString();

packages/apollo-server-integration-testsuite/src/ApolloServer.ts

@@ -28,7 +28,7 @@ import {
   VERSION,
 } from 'apollo-link-persisted-queries';
 
-import { createApolloFetch, GraphQLRequest } from 'apollo-fetch';
+import { createApolloFetch, ApolloFetch, GraphQLRequest } from 'apollo-fetch';
 import {
   AuthenticationError,
   UserInputError,
@@ -586,6 +586,11 @@ export function testApolloServer<AS extends ApolloServerBase>(
           (engineServer.stop() || Promise.resolve()).then(done);
         });
 
+        // TODO(jesse) This test block can be merged with another that will
+        // appear in a separate describe/it block after a branch merges
+        // that hasn't quite become a PR yet: abernix/finish-pr-1639.  It's
+        // intentionally separate right now because of the guaranteed merge
+        // conflict.
         it('validation > engine > extensions > formatError', async () => {
           const throwError = jest.fn(() => {
             throw new Error('nope');
@@ -686,6 +691,72 @@ export function testApolloServer<AS extends ApolloServerBase>(
           expect(trace.root!.child![0].error![0].message).toMatch(/nope/);
           expect(trace.root!.child![0].error![0].message).not.toMatch(/masked/);
         });
+
+        describe('validation > engine > traces', () => {
+          let apolloFetch: ApolloFetch;
+
+          const setupApolloServerAndFetchPair = async (
+            engineOptions: Partial<EngineReportingOptions<any>> = {},
+          ) => {
+            const { url: uri } = await createApolloServer({
+              typeDefs: gql`
+                type Query {
+                  justAField: String
+                }
+              `,
+              resolvers: {
+                Query: {
+                  justAField: () => 'a string',
+                },
+              },
+              engine: {
+                ...engineServer.engineOptions(),
+                apiKey: 'service:my-app:secret',
+                maxUncompressedReportSize: 1,
+                ...engineOptions,
+              },
+              debug: true,
+            });
+
+            apolloFetch = createApolloFetch({ uri });
+          };
+
+          it('sets the trace key to operationName when it is defined', async () => {
+            await setupApolloServerAndFetchPair();
+
+            const result = await apolloFetch({
+              query: `query AnOperationName {justAField}`,
+            });
+            expect(result.data).toEqual({
+              justAField: 'a string',
+            });
+            expect(result.errors).not.toBeDefined();
+
+            const reports = await engineServer.promiseOfReports;
+            expect(reports.length).toBe(1);
+
+            expect(Object.keys(reports[0].tracesPerQuery)[0]).toMatch(
+              /^# AnOperationName\n/,
+            );
+          });
+
+          it('sets the trace key to "-" when operationName is undefined', async () => {
+            await setupApolloServerAndFetchPair();
+
+            const result = await apolloFetch({
+              query: `{justAField}`,
+            });
+            expect(result.data).toEqual({
+              justAField: 'a string',
+            });
+            expect(result.errors).not.toBeDefined();
+
+            const reports = await engineServer.promiseOfReports;
+            expect(reports.length).toBe(1);
+
+            expect(Object.keys(reports[0].tracesPerQuery)[0]).toMatch(/^# -\n/);
+          });
+        });
       });
 
       it('errors thrown in extensions call formatError and are wrapped', async () => {