Skip to content

Commit 54ff1e6

Browse files
jsegaranJoshua Segarantrevor-scheerabernixrenovate[bot]
authored
feat(reporting): Add reportTiming option to EngineReportingOptions (#3918)
* Add trace reporting api Co-Authored-By: Trevor Scheer <trevor@apollographql.com> * Set captureTraces to false if we don't report * Fix tests * Apply suggestions from code review Co-authored-by: Jesse Rosenberger <git@jro.cc> * Address comments * Prettier * Revert "Prettier" This reverts commit e00dfe1. The need to revert this is not the fault of the author at all. The thrashing here is truly unfortunate, but that's a result of past patterns and this monorepo cannot tolerate any more burying of history. By reverting this commit, the blame for the lines it tried to change will not be re-directed at the author of `e00dfe1db` when they were introduced by others. Most importantly, this makes it reasonable to accurately review this PR in a timely fashion. * Move code around and clean * Set capture traces in the reporting agent. * Clean up * Add documentation * Remove unnecessary check * Update CHANGELOG.md Co-authored-by: Jesse Rosenberger <git@jro.cc> * Update packages/apollo-engine-reporting/src/__tests__/plugin.test.ts Co-authored-by: Jesse Rosenberger <git@jro.cc> * Apply suggestions from code review Co-authored-by: Jesse Rosenberger <git@jro.cc> * Prettier * Remove shouldReportTrace * Update docs/source/api/apollo-server.md Co-authored-by: Jesse Rosenberger <git@jro.cc> * Update packages/apollo-engine-reporting/src/agent.ts Co-authored-by: Jesse Rosenberger <git@jro.cc> * Update packages/apollo-engine-reporting/src/agent.ts Co-authored-by: Jesse Rosenberger <git@jro.cc> * Address comments * Update changelog * Re-prettier agent.ts * Address comments * Quick comment change * Rename to instrument operation * Update apollo-server.md * Rename to `timeOperation` * chore(deps): update dependency gatsby-theme-apollo-docs to v4.2.6 (#4243) Co-authored-by: Renovate Bot <bot@renovateapp.com> * chore(deps): update dependency @types/aws-lambda to v8.10.56 (#4247) Co-authored-by: Renovate Bot <bot@renovateapp.com> * chore(deps): update dependency @types/ioredis to v4.16.5 (#4248) Co-authored-by: Renovate Bot <bot@renovateapp.com> * chore(deps): update dependency @types/uuid to v7.0.4 (#4250) Co-authored-by: Renovate Bot <bot@renovateapp.com> * chore(deps): update dependency gatsby to v2.23.3 (#4251) Co-authored-by: Renovate Bot <bot@renovateapp.com> * chore(deps): update dependency @types/supertest to v2.0.9 (#4249) Co-authored-by: Renovate Bot <bot@renovateapp.com> * chore(deps): update dependency lerna to v3.22.1 (#4252) Co-authored-by: Renovate Bot <bot@renovateapp.com> * Rename to report timing * Update changelog * Use code fences in API documentation Co-authored-by: Joshua Segaran <joshua.segaran@apollographql.com> Co-authored-by: Trevor Scheer <trevor@apollographql.com> Co-authored-by: Jesse Rosenberger <git@jro.cc> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Renovate Bot <bot@renovateapp.com>
1 parent a4c7e15 commit 54ff1e6

10 files changed

Lines changed: 338 additions & 43 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ The version headers in this history reflect the versions of Apollo Server itself
77

88
### vNEXT
99

10-
- _Nothing yet! Stay tuned._
10+
- `apollo-engine-reporting`: Added a `reportTiming` API to allow trace reporting to be enabled or disabled on a per request basis. The option takes either a boolean or a predicate function that takes a [`GraphQLRequestContextDidResolveOperation`](https://github.com/apollographql/apollo-server/blob/a926b7eedbb87abab2ec70fb03d71743985cb18d/packages/apollo-server-types/src/index.ts#L185-L190) or [`GraphQLRequestContextDidEncounterErrors`](https://github.com/apollographql/apollo-server/blob/a926b7eedbb87abab2ec70fb03d71743985cb18d/packages/apollo-server-types/src/index.ts#L191-L195) and returns a boolean. If the boolean is false the request will not be instrumented for tracing and no trace will be sent to Apollo Graph Manager. The default is `true` so all traces will get instrumented and sent, which is the same as the previous default behavior. [PR #3918](https://github.com/apollographql/apollo-server/pull/3918)
11+
- `apollo-engine-reporting`: Removed `GraphQLServerOptions.reporting`. It isn't known whether a trace will be reported at the beginning of the request because of the above change. We believe this field was only used internally within Apollo Server; let us know if this is a problem and we can suggest alternatives. Additionally, the field requestContext.metrics.captureTraces is now initialized later in the request pipeline. [PR #3918](https://github.com/apollographql/apollo-server/pull/3918)
1112

1213
### v2.14.4
1314

‎docs/source/api/apollo-server.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,18 @@ addMockFunctionsToSchema({
480480
481481
A human-readable name for the variant of a schema (i.e. staging, EU). Setting this value will cause metrics to be segmented in the Apollo Graph Manager UI. Additionally schema validation with a graph variant will only check metrics associated with the same string.
482482
483+
* `reportTiming`: Boolean | async (GraphQLRequestContextDidResolveOperation | GraphQLRequestContextDidEncounterErrors) => Boolean
484+
485+
Specify whether to instrument an operation to send traces and metrics to Apollo.
486+
This may resolve to a boolean or a async function returning a promise resolving to a boolean.
487+
If the option resolves to false for an operation the operation will not be instrumented
488+
and no metrics information will be sent to Apollo.
489+
490+
The function will receive a `GraphQLRequestContextDidResolveOperation` with client and operation
491+
information or a `GraphQLRequestContextDiDEncounterErrors` in the case an operation failed
492+
to resolve properly. This allows the choice of whether to include a given request in trace
493+
and metric reporting to be made on a per-request basis. The default value is true.
494+
483495
* `generateClientInfo`: (GraphQLRequestContext) => ClientInfo **AS 2.2**
484496
485497
Creates a client context(ClientInfo) based on the request pipeline's

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

Lines changed: 173 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { graphql, GraphQLError, printSchema } from 'graphql';
33
import { Request } from 'node-fetch';
44
import { makeTraceDetails, makeHTTPRequestHeaders, plugin } from '../plugin';
55
import { Headers } from 'apollo-server-env';
6-
import { AddTraceArgs, computeExecutableSchemaId } from '../agent';
6+
import { computeExecutableSchemaId } from '../agent';
77
import { Trace } from 'apollo-engine-reporting-protobuf';
88
import pluginTestHarness from 'apollo-server-core/dist/utils/pluginTestHarness';
99

@@ -42,11 +42,23 @@ const query = `
4242
}
4343
`;
4444

45+
const queryReport = `
46+
query report {
47+
author(id: 5) {
48+
name
49+
posts(limit: 2) {
50+
id
51+
}
52+
}
53+
aBoolean
54+
}
55+
`;
56+
4557
describe('schema reporting', () => {
4658
const schema = makeExecutableSchema({ typeDefs });
4759
addMockFunctionsToSchema({ schema });
4860

49-
const addTrace = jest.fn().mockResolvedValue(undefined);
61+
const addTrace = jest.fn(() => Promise.resolve());
5062
const startSchemaReporting = jest.fn();
5163
const executableSchemaIdGenerator = jest.fn(computeExecutableSchemaId);
5264

@@ -191,12 +203,9 @@ it('trace construction', async () => {
191203
const schema = makeExecutableSchema({ typeDefs });
192204
addMockFunctionsToSchema({ schema });
193205

194-
const traces: Array<AddTraceArgs> = [];
195-
async function addTrace(args: AddTraceArgs) {
196-
traces.push(args);
197-
}
198206
const startSchemaReporting = jest.fn();
199207
const executableSchemaIdGenerator = jest.fn();
208+
const addTrace = jest.fn(() => Promise.resolve());
200209

201210
const pluginInstance = plugin(
202211
{
@@ -462,6 +471,164 @@ function makeTestHTTP(): Trace.HTTP {
462471
});
463472
}
464473

474+
describe('tests for the "reportTiming', () => {
475+
const schemaReportingFunctions = {
476+
startSchemaReporting: jest.fn(),
477+
executableSchemaIdGenerator: jest.fn(),
478+
};
479+
const schema = makeExecutableSchema({ typeDefs });
480+
addMockFunctionsToSchema({ schema });
481+
482+
const addTrace = jest.fn(() => Promise.resolve());
483+
beforeEach(() => {
484+
addTrace.mockClear();
485+
});
486+
487+
it('report no traces', async () => {
488+
const pluginInstance = plugin(
489+
{ reportTiming: false },
490+
addTrace,
491+
schemaReportingFunctions,
492+
);
493+
494+
const context = await pluginTestHarness({
495+
pluginInstance,
496+
schema,
497+
graphqlRequest: {
498+
query,
499+
operationName: 'q',
500+
extensions: {
501+
clientName: 'testing suite',
502+
},
503+
http: new Request('http://localhost:123/foo'),
504+
},
505+
executor: async ({ request: { query: source } }) => {
506+
return await graphql({
507+
schema,
508+
source,
509+
});
510+
},
511+
});
512+
expect(context.metrics.captureTraces).toBeFalsy();
513+
});
514+
515+
it('report traces based on operation name', async () => {
516+
const pluginInstance = plugin(
517+
{
518+
reportTiming: async request => {
519+
return request.request.operationName === 'report';
520+
},
521+
},
522+
addTrace,
523+
schemaReportingFunctions,
524+
);
525+
526+
const context1 = await pluginTestHarness({
527+
pluginInstance,
528+
schema,
529+
graphqlRequest: {
530+
query: queryReport,
531+
operationName: 'report',
532+
extensions: {
533+
clientName: 'testing suite',
534+
},
535+
http: new Request('http://localhost:123/foo'),
536+
},
537+
executor: async ({ request: { query: source } }) => {
538+
return await graphql({
539+
schema,
540+
source,
541+
});
542+
},
543+
});
544+
545+
expect(addTrace).toBeCalledTimes(1);
546+
expect(context1.metrics.captureTraces).toBeTruthy();
547+
addTrace.mockClear();
548+
549+
const context2 = await pluginTestHarness({
550+
pluginInstance,
551+
schema,
552+
graphqlRequest: {
553+
query,
554+
operationName: 'q',
555+
extensions: {
556+
clientName: 'testing suite',
557+
},
558+
http: new Request('http://localhost:123/foo'),
559+
},
560+
executor: async ({ request: { query: source } }) => {
561+
return await graphql({
562+
schema,
563+
source,
564+
});
565+
},
566+
});
567+
568+
expect(addTrace).not.toBeCalled();
569+
expect(context2.metrics.captureTraces).toBeFalsy();
570+
});
571+
572+
it('report traces async based on operation name', async () => {
573+
const pluginInstance = plugin(
574+
{
575+
reportTiming: async request => {
576+
return await (async () => {
577+
return request.request.operationName === 'report';
578+
})();
579+
},
580+
},
581+
addTrace,
582+
schemaReportingFunctions
583+
);
584+
585+
const context1 = await pluginTestHarness({
586+
pluginInstance,
587+
schema,
588+
graphqlRequest: {
589+
query: queryReport,
590+
operationName: 'report',
591+
extensions: {
592+
clientName: 'testing suite',
593+
},
594+
http: new Request('http://localhost:123/foo'),
595+
},
596+
executor: async ({ request: { query: source } }) => {
597+
return await graphql({
598+
schema,
599+
source,
600+
});
601+
},
602+
});
603+
604+
expect(addTrace).toBeCalledTimes(1);
605+
expect(context1.metrics.captureTraces).toBeTruthy();
606+
addTrace.mockClear();
607+
608+
const context2 = await pluginTestHarness({
609+
pluginInstance,
610+
schema,
611+
graphqlRequest: {
612+
query,
613+
operationName: 'q',
614+
extensions: {
615+
clientName: 'testing suite',
616+
},
617+
http: new Request('http://localhost:123/foo'),
618+
},
619+
executor: async ({ request: { query: source } }) => {
620+
return await graphql({
621+
schema,
622+
source,
623+
});
624+
},
625+
});
626+
627+
expect(addTrace).not.toBeCalled();
628+
expect(context2.metrics.captureTraces).toBeFalsy();
629+
});
630+
});
631+
465632
/**
466633
* TESTS FOR THE sendHeaders REPORTING OPTION
467634
*/

0 commit comments

Comments
 (0)
⚡