From 61528626348f49cb92f8ae9bc840ba67ffc9ad3b Mon Sep 17 00:00:00 2001
From: Evans Hauser <evanshauser@gmail.com>
Date: Tue, 4 Sep 2018 13:43:19 -0700
Subject: [PATCH] [Squash] Address feedback

Note that the Engine proxy strips all error information from the traces
with noErrorTraces set. To get errors to show up in the ui, the proxy
sends error counts inside of the aggregated stats reports. To get
similar behavior inside of the apollo server metrics reporting, we
always send a trace and mask out the PII.
---
 docs/source/api/apollo-server.md                  |  4 ++--
 packages/apollo-engine-reporting/src/agent.ts     |  4 ++--
 packages/apollo-engine-reporting/src/extension.ts | 15 ++++++---------
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md
index a954498c2c2..3c2ddfcffe4 100644
--- a/docs/source/api/apollo-server.md
+++ b/docs/source/api/apollo-server.md
@@ -345,6 +345,6 @@ addMockFunctionsToSchema({
    'sendReport()' on other signals if you'd like. Note that 'sendReport()'
    does not run synchronously so it cannot work usefully in an 'exit' handler.
 
-*  `sendErrorTraces`: boolean
+*  `maskErrorDetails`: boolean
 
-   To remove errors from traces, set this to false. Defaults to true
+   Set to true to remove error details from the traces sent to Apollo's servers. Defaults to false.
diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts
index 3c0671b5bd2..fa655624215 100644
--- a/packages/apollo-engine-reporting/src/agent.ts
+++ b/packages/apollo-engine-reporting/src/agent.ts
@@ -81,8 +81,8 @@ export interface EngineReportingOptions {
   handleSignals?: boolean;
   // Sends the trace report immediately. This options is useful for stateless environments
   sendReportsImmediately?: boolean;
-  // To remove errors from traces, set this to false. Defaults to true
-  sendErrorTraces?: boolean;
+  // To remove the error message from traces, set this to true. Defaults to false
+  maskErrorDetails?: boolean;
 
   // XXX Provide a way to set client_name, client_version, client_address,
   // service, and service_version fields. They are currently not revealed in the
diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts
index 7ed4260768c..74a58efbfe2 100644
--- a/packages/apollo-engine-reporting/src/extension.ts
+++ b/packages/apollo-engine-reporting/src/extension.ts
@@ -44,7 +44,7 @@ export class EngineReportingExtension<TContext = any>
     addTrace: (signature: string, operationName: string, trace: Trace) => void,
   ) {
     this.options = {
-      sendErrorTraces: true,
+      maskErrorDetails: false,
       ...options,
     };
     this.addTrace = addTrace;
@@ -229,19 +229,16 @@ export class EngineReportingExtension<TContext = any>
           }
         }
 
-        // With noErrorTraces, the Engine proxy strips all error information
-        // from the traces and sends error counts inside of the aggregated stats
-        // reports. To get similar behavior inside of the apollo server metrics
-        // reporting, we always send a trace and mask out the PII
-        const errorInfo = this.options.sendErrorTraces
-          ? {
+        // Always send the trace errors, so that the UI acknowledges that there is an error.
+        const errorInfo = this.options.maskErrorDetails
+          ? { message: '<masked>' }
+          : {
               message: error.message,
               location: (error.locations || []).map(
                 ({ line, column }) => new Trace.Location({ line, column }),
               ),
               json: JSON.stringify(error),
-            }
-          : { message: 'Masked by Apollo Engine Reporting' };
+            };
 
         node!.error!.push(new Trace.Error(errorInfo));
       });