From 81c46421a9e8ba210c8f9321a0909f48ef219a05 Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Mon, 17 Sep 2018 22:45:34 -0700 Subject: [PATCH] Provide ability to specify client info in traces (#1631) * 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. * Use extensions and context in createClientInfo * Remove support for clientAddress The frontend will not support it in the near future * create -> generate and make default generator createClientInfo -> generateClientInfo * Clarify default values --- CHANGELOG.md | 1 - docs/source/api/apollo-server.md | 10 ++++++++ packages/apollo-engine-reporting/src/agent.ts | 13 ++++++++++ .../apollo-engine-reporting/src/extension.ts | 25 ++++++++++++++++++- .../src/requestProcessing.ts | 2 ++ .../apollo-server-core/src/runHttpQuery.ts | 2 ++ packages/graphql-extensions/src/index.ts | 1 + 7 files changed, 52 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 543f98a362b..79233b7df25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,6 @@ ### vNEXT - FIXME(@abernix): Allow context to be passed to all GraphQLExtension methods. -- FIXME(@abernix): client info in traces. - Allow an optional function to resolve the `rootValue`, passing the `DocumentNode` AST to determine the value. [PR #1555](https://github.com/apollographql/apollo-server/pull/1555) - Follow-up on the work in [PR #1516](https://github.com/apollographql/apollo-server/pull/1516) to also fix missing insertion cursor/caret when a custom GraphQL configuration is specified which doesn't specify its own `cursorShape` property. [PR #1607](https://github.com/apollographql/apollo-server/pull/1607) diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md index 3b39fc456f6..90bbea8d71f 100644 --- a/docs/source/api/apollo-server.md +++ b/docs/source/api/apollo-server.md @@ -365,3 +365,13 @@ addMockFunctionsToSchema({ * `maskErrorDetails`: boolean Set to true to remove error details from the traces sent to Apollo's servers. Defaults to false. + +* generateClientInfo?: (o: { context: any, extensions?: Record}) => ClientInfo; + + 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 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/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index c9378925f24..6169d5115ce 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -34,6 +34,11 @@ Traces.encode = function(message, originalWriter) { return writer; }; +export interface ClientInfo { + clientName?: 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 +88,14 @@ 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 + generateClientInfo?: ( + o: { + context: any; + extensions?: 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 30783da6e95..a986de83246 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 @@ -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,11 @@ 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's extensions, when + // the ClientInfo fields are undefined, we send the empty string + (({ extensions }) => (extensions && extensions.clientInfo) || {}); } public requestDidStart(o: { @@ -60,6 +71,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(); @@ -154,6 +167,16 @@ 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.generateClientInfo({ + context: o.context, + extensions: o.extensions, + }); + this.trace.clientName = clientName || ''; + this.trace.clientVersion = clientVersion || ''; + return () => { this.trace.durationNs = durationHrTimeToNanos( process.hrtime(this.startHrTime), diff --git a/packages/apollo-server-core/src/requestProcessing.ts b/packages/apollo-server-core/src/requestProcessing.ts index 07a24eb1a15..c0aebd544d7 100644 --- a/packages/apollo-server-core/src/requestProcessing.ts +++ b/packages/apollo-server-core/src/requestProcessing.ts @@ -55,6 +55,7 @@ export interface GraphQLRequestOptions { debug?: boolean; extensions?: Array<() => GraphQLExtension>; + queryExtensions?: Record; tracing?: boolean; persistedQueries?: PersistedQueryOptions; cacheControl?: CacheControlExtensionOptions; @@ -185,6 +186,7 @@ export class GraphQLRequestProcessor { queryString: request.query, operationName: request.operationName, variables: request.variables, + extensions: request.extensions, persistedQueryHit, persistedQueryRegister, }); diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts index 6383bc75403..b21e7765d4a 100644 --- a/packages/apollo-server-core/src/runHttpQuery.ts +++ b/packages/apollo-server-core/src/runHttpQuery.ts @@ -302,6 +302,8 @@ export async function runHttpQuery( formatResponse: optionsObject.formatResponse, debug: optionsObject.debug, + + queryExtensions: extensions, }); // GET operations should only be queries (not mutations). We want to throw diff --git a/packages/graphql-extensions/src/index.ts b/packages/graphql-extensions/src/index.ts index ea7d70462b9..906fa3c01fb 100644 --- a/packages/graphql-extensions/src/index.ts +++ b/packages/graphql-extensions/src/index.ts @@ -78,6 +78,7 @@ export class GraphQLExtensionStack { variables?: { [key: string]: any }; persistedQueryHit?: boolean; persistedQueryRegister?: boolean; + extensions?: Record; }): EndHandler { return this.handleDidStart( ext => ext.requestDidStart && ext.requestDidStart(o),