Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AER: include encodedTraces only once to prevent duplicates #2040

Merged
merged 14 commits into from
Dec 4, 2018
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### vNEXT

- Apollo Engine Reporting: include encodedTraces only once to prevent duplicates with multiple instances of apollo-engine-reporting [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
Expand Down
11 changes: 11 additions & 0 deletions packages/apollo-engine-reporting-protobuf/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,14 @@ for the Engine reporting API.

The Engine reporting API is currently subject to change at any time; do not rely
on this to build your own client.

## Development

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` command.
Normally `prepare` is performed by the root package.json with a `postinstall`
hook. If the code in this package needs to change, you should run `npx lerna
run prepare` in the root of this monorepo to generate the code changes. Running
the command in the root enables the `protobuf` binaries, `pbjs` and `pbts`, to
resolve correctly.
6 changes: 3 additions & 3 deletions packages/apollo-engine-reporting-protobuf/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions packages/apollo-engine-reporting-protobuf/src/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import * as protobuf from './protobuf';
export = protobuf;
24 changes: 24 additions & 0 deletions packages/apollo-engine-reporting-protobuf/src/index.js
Original file line number Diff line number Diff line change
@@ -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;
25 changes: 2 additions & 23 deletions packages/apollo-engine-reporting/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,6 @@ import {
GraphQLServiceContext,
} from 'apollo-server-core/dist/requestPipelineAPI';

// Override the generated protobuf Traces.encode function so that it will look
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that there is a reference to this comment later where encodedTraces is written which should be updated to point to the other file.

// 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;
Expand Down Expand Up @@ -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,
);
Expand Down