From b43cdd32bc72dd3dca508f4dbec95bd0a4fdc7a8 Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Fri, 7 Sep 2018 16:15:32 -0700 Subject: [PATCH 1/5] Provide ability to specify client info in traces Adds the createClientInfo to apollo-engine-reporting, which enables the differentiation of clients based on the request, operation, and variables. This could be extended to include the response. However for the first release. It doesn't quite make sense. --- CHANGELOG.md | 1 + docs/source/api/apollo-server.md | 6 ++++++ packages/apollo-engine-reporting/src/agent.ts | 18 +++++++++++++++++- .../apollo-engine-reporting/src/extension.ts | 15 ++++++++++++++- 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5afd3b5352..b3a7cca6552 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All of the packages in the `apollo-server` repo are released with the same versi ### vNEXT - Update link inside Authentication Docs [PR #1682](https://github.com/apollographql/apollo-server/pull/1682) +- Provide ability to specify client info in traces [#1631](https://github.com/apollographql/apollo-server/pull/1631) ### v2.0.8 diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md index b0db2db34aa..b98d9128e2c 100644 --- a/docs/source/api/apollo-server.md +++ b/docs/source/api/apollo-server.md @@ -350,3 +350,9 @@ addMockFunctionsToSchema({ * `maskErrorDetails`: boolean Set to true to remove error details from the traces sent to Apollo's servers. Defaults to false. + +* createClientInfo?: (o: { request: Request, queryString?: string, parsedQuery?: DocumentNode, variables: Record}) => ClientInfo; + + Creates the client information that is attached to the traces sent to the + Apollo backend. `ClientInfo` contains fields for `clientName`, + `clientVersion`, and `clientAddress`, which can all be optional. diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index 379ab085597..cc6b60c5a46 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -8,7 +8,7 @@ import { Trace, } from 'apollo-engine-reporting-protobuf'; -import { fetch, Response } from 'apollo-server-env'; +import { fetch, Request, Response } from 'apollo-server-env'; import * as retry from 'async-retry'; import { EngineReportingExtension } from './extension'; @@ -34,6 +34,12 @@ Traces.encode = function(message, originalWriter) { return writer; }; +export interface ClientInfo { + clientName?: string; + clientAddress?: string; + clientVersion?: string; +} + export interface EngineReportingOptions { // API key for the service. Get this from // [Engine](https://engine.apollographql.com) by logging in and creating @@ -83,6 +89,16 @@ export interface EngineReportingOptions { sendReportsImmediately?: boolean; // To remove the error message from traces, set this to true. Defaults to false maskErrorDetails?: boolean; + // Creates the client information attached to the traces sent to the Apollo + // backend + createClientInfo?: ( + o: { + request: Request; + queryString?: string; + parsedQuery?: DocumentNode; + variables: Record; + }, + ) => ClientInfo; // 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 624985dab0c..35f5bc8dd88 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -15,7 +15,7 @@ import { } from 'graphql-extensions'; import { Trace, google } from 'apollo-engine-reporting-protobuf'; -import { EngineReportingOptions } from './agent'; +import { EngineReportingOptions, ClientInfo } from './agent'; import { defaultSignature } from './signature'; // EngineReportingExtension is the per-request GraphQLExtension which creates a @@ -154,6 +154,19 @@ export class EngineReportingExtension }); } + const { clientName, clientAddress, clientVersion } = + (this.options.createClientInfo && + this.options.createClientInfo({ + request: o.request, + queryString: o.queryString, + parsedQuery: o.parsedQuery, + variables: o.variables, + })) || + ({} as ClientInfo); + this.trace.clientName = clientName || ''; + this.trace.clientAddress = clientAddress || ''; + this.trace.clientVersion = clientVersion || ''; + return () => { this.trace.durationNs = durationHrTimeToNanos( process.hrtime(this.startHrTime), From fd3f5de4c1cbfb3c827f93bc1343e001150f48f7 Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Mon, 17 Sep 2018 21:27:46 -0700 Subject: [PATCH 2/5] Use extensions and context in createClientInfo --- packages/apollo-engine-reporting/src/agent.ts | 8 +++----- packages/apollo-engine-reporting/src/extension.ts | 8 ++++---- packages/apollo-server-core/src/runHttpQuery.ts | 1 + packages/apollo-server-core/src/runQuery.ts | 2 ++ packages/graphql-extensions/src/index.ts | 1 + 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index cc6b60c5a46..26a9e454898 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -8,7 +8,7 @@ import { Trace, } from 'apollo-engine-reporting-protobuf'; -import { fetch, Request, Response } from 'apollo-server-env'; +import { fetch, Response } from 'apollo-server-env'; import * as retry from 'async-retry'; import { EngineReportingExtension } from './extension'; @@ -93,10 +93,8 @@ export interface EngineReportingOptions { // backend createClientInfo?: ( o: { - request: Request; - queryString?: string; - parsedQuery?: DocumentNode; - variables: Record; + context: any; + extensions?: Record; }, ) => ClientInfo; diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 35f5bc8dd88..39671279e5f 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -60,6 +60,8 @@ export class EngineReportingExtension variables?: Record; persistedQueryHit?: boolean; persistedQueryRegister?: boolean; + context: any; + extensions?: Record; }): EndHandler { this.trace.startTime = dateToTimestamp(new Date()); this.startHrTime = process.hrtime(); @@ -157,10 +159,8 @@ export class EngineReportingExtension const { clientName, clientAddress, clientVersion } = (this.options.createClientInfo && this.options.createClientInfo({ - request: o.request, - queryString: o.queryString, - parsedQuery: o.parsedQuery, - variables: o.variables, + context: o.context, + extensions: o.extensions, })) || ({} as ClientInfo); this.trace.clientName = clientName || ''; diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts index 08964abd6aa..e49d1a7b7ad 100644 --- a/packages/apollo-server-core/src/runHttpQuery.ts +++ b/packages/apollo-server-core/src/runHttpQuery.ts @@ -402,6 +402,7 @@ export async function runHttpQuery( : false, request: request.request, extensions: optionsObject.extensions, + queryExtensions: extensions, persistedQueryHit, persistedQueryRegister, }; diff --git a/packages/apollo-server-core/src/runQuery.ts b/packages/apollo-server-core/src/runQuery.ts index 516750dfd78..4eabf053a36 100644 --- a/packages/apollo-server-core/src/runQuery.ts +++ b/packages/apollo-server-core/src/runQuery.ts @@ -66,6 +66,7 @@ export interface QueryOptions { cacheControl?: boolean | CacheControlExtensionOptions; request: Pick; extensions?: Array<() => GraphQLExtension>; + queryExtensions?: Record; persistedQueryHit?: boolean; persistedQueryRegister?: boolean; } @@ -136,6 +137,7 @@ function doRunQuery(options: QueryOptions): Promise { persistedQueryHit: options.persistedQueryHit, persistedQueryRegister: options.persistedQueryRegister, context, + extensions: options.queryExtensions, }); return Promise.resolve() .then( diff --git a/packages/graphql-extensions/src/index.ts b/packages/graphql-extensions/src/index.ts index b7f358f6da8..b4455109da6 100644 --- a/packages/graphql-extensions/src/index.ts +++ b/packages/graphql-extensions/src/index.ts @@ -81,6 +81,7 @@ export class GraphQLExtensionStack { persistedQueryHit?: boolean; persistedQueryRegister?: boolean; context: TContext; + extensions?: Record; }): EndHandler { return this.handleDidStart( ext => ext.requestDidStart && ext.requestDidStart(o), From adae6f7e605d0bcae3bfae790f7bc0bfb980eb79 Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Mon, 17 Sep 2018 21:34:24 -0700 Subject: [PATCH 3/5] Remove support for clientAddress The frontend will not support it in the near future --- docs/source/api/apollo-server.md | 4 ++-- packages/apollo-engine-reporting/src/agent.ts | 1 - packages/apollo-engine-reporting/src/extension.ts | 6 ++++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md index b98d9128e2c..9d5684cf816 100644 --- a/docs/source/api/apollo-server.md +++ b/docs/source/api/apollo-server.md @@ -354,5 +354,5 @@ addMockFunctionsToSchema({ * createClientInfo?: (o: { request: Request, queryString?: string, parsedQuery?: DocumentNode, variables: Record}) => ClientInfo; Creates the client information that is attached to the traces sent to the - Apollo backend. `ClientInfo` contains fields for `clientName`, - `clientVersion`, and `clientAddress`, which can all be optional. + Apollo backend. `ClientInfo` contains fields for `clientName` and + `clientVersion`, which can both be optional. diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index 26a9e454898..b7a489e63be 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -36,7 +36,6 @@ Traces.encode = function(message, originalWriter) { export interface ClientInfo { clientName?: string; - clientAddress?: string; clientVersion?: string; } diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 39671279e5f..834edf8d88e 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -156,7 +156,10 @@ export class EngineReportingExtension }); } - const { clientName, clientAddress, clientVersion } = + // While clientAddress could be a part of the protobuf, we'll ignore it for + // now, since the backend does not group by it and Engine frontend will not + // support it in the short term + const { clientName, clientVersion } = (this.options.createClientInfo && this.options.createClientInfo({ context: o.context, @@ -164,7 +167,6 @@ export class EngineReportingExtension })) || ({} as ClientInfo); this.trace.clientName = clientName || ''; - this.trace.clientAddress = clientAddress || ''; this.trace.clientVersion = clientVersion || ''; return () => { From a6328b9c73843e1db1f4835a190533cee232da8c Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Mon, 17 Sep 2018 21:58:54 -0700 Subject: [PATCH 4/5] create -> generate and make default generator createClientInfo -> generateClientInfo --- docs/source/api/apollo-server.md | 10 ++++++--- packages/apollo-engine-reporting/src/agent.ts | 2 +- .../apollo-engine-reporting/src/extension.ts | 21 ++++++++++++------- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md index 9d5684cf816..69fc669b1f8 100644 --- a/docs/source/api/apollo-server.md +++ b/docs/source/api/apollo-server.md @@ -351,8 +351,12 @@ addMockFunctionsToSchema({ Set to true to remove error details from the traces sent to Apollo's servers. Defaults to false. -* createClientInfo?: (o: { request: Request, queryString?: string, parsedQuery?: DocumentNode, variables: Record}) => ClientInfo; +* generateClientInfo?: (o: { context: any, extensions?: Record}) => ClientInfo; Creates the client information that is attached to the traces sent to the - Apollo backend. `ClientInfo` contains fields for `clientName` and - `clientVersion`, which can both be optional. + Apollo backend. The context field is the execution context passed to the + resolvers and the extensions field is the field provided in the request's + query, operationName, and variables. `ClientInfo` contains fields for + `clientName` and `clientVersion`, which can both be optional. The default + value copies the respective fields from the `extensions`'s `clientInfo` + field. diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index b7a489e63be..ea5b9b17a3d 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -90,7 +90,7 @@ export interface EngineReportingOptions { maskErrorDetails?: boolean; // Creates the client information attached to the traces sent to the Apollo // backend - createClientInfo?: ( + generateClientInfo?: ( o: { context: any; extensions?: Record; diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 834edf8d88e..88df097240d 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -38,6 +38,12 @@ export class EngineReportingExtension operationName: string, trace: Trace, ) => void; + private generateClientInfo: ( + o: { + context: any; + extensions?: Record; + }, + ) => ClientInfo; public constructor( options: EngineReportingOptions, @@ -51,6 +57,10 @@ export class EngineReportingExtension const root = new Trace.Node(); this.trace.root = root; this.nodes.set(responsePathAsString(undefined), root); + this.generateClientInfo = + options.generateClientInfo || + // Default to using the clientInfo field of the request + (({ extensions }) => (extensions && extensions.clientInfo) || {}); } public requestDidStart(o: { @@ -159,13 +169,10 @@ export class EngineReportingExtension // While clientAddress could be a part of the protobuf, we'll ignore it for // now, since the backend does not group by it and Engine frontend will not // support it in the short term - const { clientName, clientVersion } = - (this.options.createClientInfo && - this.options.createClientInfo({ - context: o.context, - extensions: o.extensions, - })) || - ({} as ClientInfo); + const { clientName, clientVersion } = this.generateClientInfo({ + context: o.context, + extensions: o.extensions, + }); this.trace.clientName = clientName || ''; this.trace.clientVersion = clientVersion || ''; From 843fd64c702c0e300e1359fc211ec362c105eb69 Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Mon, 17 Sep 2018 22:39:24 -0700 Subject: [PATCH 5/5] Clarify default values --- docs/source/api/apollo-server.md | 10 +++++----- packages/apollo-engine-reporting/src/extension.ts | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md index 69fc669b1f8..40103674e79 100644 --- a/docs/source/api/apollo-server.md +++ b/docs/source/api/apollo-server.md @@ -355,8 +355,8 @@ addMockFunctionsToSchema({ Creates the client information that is attached to the traces sent to the Apollo backend. The context field is the execution context passed to the - resolvers and the extensions field is the field provided in the request's - query, operationName, and variables. `ClientInfo` contains fields for - `clientName` and `clientVersion`, which can both be optional. The default - value copies the respective fields from the `extensions`'s `clientInfo` - field. + resolvers and the extensions field corresponds to the same value in the POST + body or GET parameters. `ClientInfo` contains fields for `clientName` and + `clientVersion`, which are both optional. The default generation copies the + respective fields from `extensions.clientInfo`. If `clientName` or + `clientVersion` is not present, the values are set to the empty string. diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 88df097240d..bf9b992d569 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -59,7 +59,8 @@ export class EngineReportingExtension this.nodes.set(responsePathAsString(undefined), root); this.generateClientInfo = options.generateClientInfo || - // Default to using the clientInfo field of the request + // Default to using the clientInfo field of the request's extensions, when + // the ClientInfo fields are undefined, we send the empty string (({ extensions }) => (extensions && extensions.clientInfo) || {}); }