AER: include encodedTraces only once to prevent duplicates

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### vNEXT
 
+- `apollo-engine-reporting`: When multiple instances of `apollo-engine-reporting` are loaded (an uncommon edge case), ensure that `encodedTraces` are handled only once rather than once per loaded instance. [PR #2040](https://github.com/apollographql/apollo-server/pull/2040)
 - `apollo-server-micro`: Set the `Content-type` to `text/html` for GraphQL Playground. [PR #2026](https://github.com/apollographql/apollo-server/pull/2026)
 
 ### v2.2.5

packages/apollo-engine-reporting-protobuf/README.md

@@ -1,7 +1,29 @@
-# apollo-engine-reporting-protobuf
+# `apollo-engine-reporting-protobuf`
 
-This contains generated Javascript/TypeScript code for the protobuf definitions
-for the Engine reporting API.
+> **Note:** The Apollo Engine reporting API is subject to change.  We strongly
+> encourage developers to contact Apollo Engine support to discuss their use
+> case prior to building their own reporting agent using this module.
 
-The Engine reporting API is currently subject to change at any time; do not rely
-on this to build your own client.
+This module provides JavaScript/TypeScript
+[Protocol buffer](https://developers.google.com/protocol-buffers/) definitions
+for the Apollo Engine reporting API.  These definitions are generated for
+consumption from the `reports.proto` file which is defined internally within
+Apollo.
+
+## Development
+
+> **Note:** Due to a dependency on Unix tools (e.g. `bash`, `grep`, etc.), the
+> development of this module requires a Unix system.  There is no reason why
+> this can't be avoided, the time just hasn't been taken to make those changes.
+> We'd happily accept a PR which makes the appropriate changes!
+
+Currently, this package generates a majority of its code with
+[`protobufjs`](https://www.npmjs.com/package/protobufjs) based on the
+`reports.proto` file. The output is generated with the `prepare` npm script.
+
+The root of the repository provides the `devDependencies` necessary to build
+these definitions (e.g. `pbjs`, `pbts`, `protobuf`, etc.) and the the `prepare`
+npm script is invoked programmatically via the monorepo tooling (e.g. Lerna)
+thanks to _this_ module's `postinstall` script.   Therefore, when making
+changes to this module, `npx lerna run prepare` should be run from the **root**
+of this monorepo in order to update the definitions in _this_ module.

packages/apollo-engine-reporting-protobuf/package.json

@@ -5,9 +5,9 @@
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
   "scripts": {
-    "prepare": "npm run pbjs && npm run pbts",
-    "pbjs": "bash -c 'mkdir -p dist && pbjs --target static-module --out dist/index.js --wrap commonjs --force-number <(grep -v \"package mdg.engine.proto\" reports.proto)'",
-    "pbts": "pbts -o dist/index.d.ts dist/index.js"
+    "prepare": "npm run pbjs && npm run pbts && cp src/* dist",
+    "pbjs": "bash -c 'mkdir -p dist && pbjs --target static-module --out dist/protobuf.js --wrap commonjs --force-number <(grep -v \"package mdg.engine.proto\" src/reports.proto)'",
+    "pbts": "pbts -o dist/protobuf.d.ts dist/protobuf.js"
   },
   "repository": {
     "type": "git",

packages/apollo-engine-reporting-protobuf/src/index.d.ts

@@ -0,0 +1,2 @@
+import * as protobuf from './protobuf';
+export = protobuf;

packages/apollo-engine-reporting-protobuf/src/index.js

@@ -0,0 +1,24 @@
+const protobuf = require('./protobuf');
+
+// Override the generated protobuf Traces.encode function so that it will look
+// for Traces that are already encoded to Buffer as well as unencoded
+// Traces. This amortizes the protobuf encoding time over each generated Trace
+// instead of bunching it all up at once at sendReport time. In load tests, this
+// change improved p99 end-to-end HTTP response times by a factor of 11 without
+// a casually noticeable effect on p50 times. This also makes it easier for us
+// to implement maxUncompressedReportSize as we know the encoded size of traces
+// as we go.
+const originalTracesEncode = protobuf.Traces.encode;
+protobuf.Traces.encode = function(message, originalWriter) {
+  const writer = originalTracesEncode(message, originalWriter);
+  const encodedTraces = message.encodedTraces;
+  if (encodedTraces != null && encodedTraces.length) {
+    for (let i = 0; i < encodedTraces.length; ++i) {
+      writer.uint32(/* id 1, wireType 2 =*/ 10);
+      writer.bytes(encodedTraces[i]);
+    }
+  }
+  return writer;
+};
+
+module.exports = protobuf;

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

@@ -17,27 +17,6 @@ import {
   GraphQLServiceContext,
 } from 'apollo-server-core/dist/requestPipelineAPI';
 
-// Override the generated protobuf Traces.encode function so that it will look
-// for Traces that are already encoded to Buffer as well as unencoded
-// Traces. This amortizes the protobuf encoding time over each generated Trace
-// instead of bunching it all up at once at sendReport time. In load tests, this
-// change improved p99 end-to-end HTTP response times by a factor of 11 without
-// a casually noticeable effect on p50 times. This also makes it easier for us
-// to implement maxUncompressedReportSize as we know the encoded size of traces
-// as we go.
-const originalTracesEncode = Traces.encode;
-Traces.encode = function(message, originalWriter) {
-  const writer = originalTracesEncode(message, originalWriter);
-  const encodedTraces = (message as any).encodedTraces;
-  if (encodedTraces != null && encodedTraces.length) {
-    for (let i = 0; i < encodedTraces.length; ++i) {
-      writer.uint32(/* id 1, wireType 2 =*/ 10);
-      writer.bytes(encodedTraces[i]);
-    }
-  }
-  return writer;
-};
-
 export interface ClientInfo {
   clientName?: string;
   clientVersion?: string;
@@ -190,8 +169,8 @@ export class EngineReportingAgent<TContext = any> {
       this.report.tracesPerQuery[statsReportKey] = new Traces();
       (this.report.tracesPerQuery[statsReportKey] as any).encodedTraces = [];
     }
-    // See comment on our override of Traces.encode to learn more about this
-    // strategy.
+    // See comment on our override of Traces.encode inside of
+    // apollo-engine-reporting-protobuf to learn more about this strategy.
     (this.report.tracesPerQuery[statsReportKey] as any).encodedTraces.push(
       encodedTrace,
     );