From 2f81a6513dc174fad5cb6b2584fd206c05e8ad8d Mon Sep 17 00:00:00 2001 From: Michael Watson Date: Fri, 27 Mar 2020 15:49:10 -0700 Subject: [PATCH 01/13] Deprecate ENGINE_API_KEY - Change ENGINE_API_KEY reference to APOLLO_KEY and add a deprecation message. - Export keys to be referenced through out packages --- packages/apollo-engine-reporting/src/agent.ts | 20 +++++++++++++++++-- .../src/ApolloServer.ts | 3 ++- .../package.json | 1 + .../src/ApolloServer.ts | 5 +++-- .../apollo-server-core/src/ApolloServer.ts | 3 ++- .../apollo-server-lambda/src/ApolloServer.ts | 3 ++- 6 files changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index d3b0be2d1c4..67ee17adc74 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -16,6 +16,11 @@ import { GraphQLRequestContext } from 'apollo-server-types'; import { InMemoryLRUCache } from 'apollo-server-caching'; import { defaultEngineReportingSignature } from 'apollo-graphql'; +// TDOD: Reviewer, should we consider moving these exports to a different package +// Maybe apollo-server-env? +export const legacyKeyEnvVar = 'ENGINE_API_KEY'; +export const keyEnvVar = 'APOLLO_KEY' + export interface ClientInfo { clientName?: string; clientVersion?: string; @@ -226,13 +231,24 @@ export class EngineReportingAgent { public constructor(options: EngineReportingOptions = {}) { this.options = options; - this.apiKey = options.apiKey || process.env.ENGINE_API_KEY || ''; + const legacyApiKeyFromEnv = process.env.ENGINE_API_KEY; + const apiKeyFromEnv = process.env.APOLLO_KEY; + + if(legacyApiKeyFromEnv && apiKeyFromEnv) { + throw new Error(`Cannot set both ${legacyKeyEnvVar} and ${keyEnvVar}. Please only set ${keyEnvVar}`); + } + + this.apiKey = options.apiKey || apiKeyFromEnv || legacyApiKeyFromEnv || ''; if (!this.apiKey) { throw new Error( - 'To use EngineReportingAgent, you must specify an API key via the apiKey option or the ENGINE_API_KEY environment variable.', + `To use EngineReportingAgent, you must specify an API key via the apiKey option or the ${keyEnvVar} environment variable.`, ); } + if(legacyApiKeyFromEnv) { + console.warn(`[Deprecation warning] Setting the key via ${legacyKeyEnvVar} is deprecated and will not be supported in future versions.`) + } + // Since calculating the signature for Engine reporting is potentially an // expensive operation, we'll cache the signatures we generate and re-use // them based on repeated traces for the same `queryHash`. diff --git a/packages/apollo-server-azure-functions/src/ApolloServer.ts b/packages/apollo-server-azure-functions/src/ApolloServer.ts index f5e5d68ce39..f8dfb75ad26 100644 --- a/packages/apollo-server-azure-functions/src/ApolloServer.ts +++ b/packages/apollo-server-azure-functions/src/ApolloServer.ts @@ -8,6 +8,7 @@ import { } from '@apollographql/graphql-playground-html'; import { graphqlAzureFunction } from './azureFunctionApollo'; +import { legacyKeyEnvVar, keyEnvVar } from 'apollo-engine-reporting/src/agent'; export interface CreateHandlerOptions { cors?: { @@ -25,7 +26,7 @@ export class ApolloServer extends ApolloServerBase { // another place, since the documentation becomes much more complicated when // the constructor is not longer shared between all integration constructor(options: Config) { - if (process.env.ENGINE_API_KEY || options.engine) { + if (process.env[keyEnvVar] || process.env[legacyKeyEnvVar] || options.engine) { options.engine = { sendReportsImmediately: true, ...(typeof options.engine !== 'boolean' ? options.engine : {}), diff --git a/packages/apollo-server-cloud-functions/package.json b/packages/apollo-server-cloud-functions/package.json index e07a8d0c305..4e8c8f12733 100644 --- a/packages/apollo-server-cloud-functions/package.json +++ b/packages/apollo-server-cloud-functions/package.json @@ -29,6 +29,7 @@ "apollo-server-core": "file:../apollo-server-core", "apollo-server-env": "file:../apollo-server-env", "apollo-server-types": "file:../apollo-server-types", + "apollo-engine-reporting":"file:../apollo-engine-reporting", "graphql-tools": "^4.0.0" }, "devDependencies": { diff --git a/packages/apollo-server-cloud-functions/src/ApolloServer.ts b/packages/apollo-server-cloud-functions/src/ApolloServer.ts index 7971e1c7598..5b7538d0dfe 100644 --- a/packages/apollo-server-cloud-functions/src/ApolloServer.ts +++ b/packages/apollo-server-cloud-functions/src/ApolloServer.ts @@ -6,6 +6,7 @@ import { import { Request, Response } from 'express'; import { graphqlCloudFunction } from './googleCloudApollo'; +import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting/src/agent'; export interface CreateHandlerOptions { cors?: { @@ -23,7 +24,7 @@ export class ApolloServer extends ApolloServerBase { // another place, since the documentation becomes much more complicated when // the constructor is not longer shared between all integration constructor(options: Config) { - if (process.env.ENGINE_API_KEY || options.engine) { + if (process.env[keyEnvVar] || process.env[legacyKeyEnvVar] || options.engine) { options.engine = { sendReportsImmediately: true, ...(typeof options.engine !== 'boolean' ? options.engine : {}), @@ -114,7 +115,7 @@ export class ApolloServer extends ApolloServerBase { ); } } - + res.set(corsHeaders); if (req.method === 'OPTIONS') { diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index 6c21d422fc1..d9a209f7236 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -68,6 +68,7 @@ import { import { Headers } from 'apollo-server-env'; import { buildServiceDefinition } from '@apollographql/apollo-tools'; +import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting/src/agent'; const NoIntrospection = (context: ValidationContext) => ({ Field(node: FieldDefinitionNode) { @@ -83,7 +84,7 @@ const NoIntrospection = (context: ValidationContext) => ({ }); function getEngineApiKey(engine: Config['engine']): string | undefined { - const keyFromEnv = process.env.ENGINE_API_KEY || ''; + const keyFromEnv = process.env[keyEnvVar] || process.env[legacyKeyEnvVar] || ''; if (engine === false) { return; } else if (typeof engine === 'object' && engine.apiKey) { diff --git a/packages/apollo-server-lambda/src/ApolloServer.ts b/packages/apollo-server-lambda/src/ApolloServer.ts index 181de51b061..68dc70d7436 100644 --- a/packages/apollo-server-lambda/src/ApolloServer.ts +++ b/packages/apollo-server-lambda/src/ApolloServer.ts @@ -11,6 +11,7 @@ import { import { graphqlLambda } from './lambdaApollo'; import { Headers } from 'apollo-server-env'; +import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting/src/agent'; export interface CreateHandlerOptions { cors?: { @@ -29,7 +30,7 @@ export class ApolloServer extends ApolloServerBase { // another place, since the documentation becomes much more complicated when // the constructor is not longer shared between all integration constructor(options: Config) { - if (process.env.ENGINE_API_KEY || options.engine) { + if (process.env[keyEnvVar] || process.env[legacyKeyEnvVar] || options.engine) { options.engine = { sendReportsImmediately: true, ...(typeof options.engine !== 'boolean' ? options.engine : {}), From 9728130e87f4dc74c0a53ab077c5e83eefa9daea Mon Sep 17 00:00:00 2001 From: Michael Watson Date: Fri, 27 Mar 2020 16:22:37 -0700 Subject: [PATCH 02/13] Fix keyEnvVar references --- package-lock.json | 1 + packages/apollo-engine-reporting/src/index.ts | 2 +- packages/apollo-server-azure-functions/src/ApolloServer.ts | 2 +- packages/apollo-server-cloud-functions/src/ApolloServer.ts | 2 +- packages/apollo-server-core/src/ApolloServer.ts | 2 +- packages/apollo-server-lambda/src/ApolloServer.ts | 2 +- 6 files changed, 6 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0a34ff9a906..44b9df5c45e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4414,6 +4414,7 @@ "version": "file:packages/apollo-server-cloud-functions", "requires": { "@apollographql/graphql-playground-html": "1.6.24", + "apollo-engine-reporting": "file:packages/apollo-engine-reporting", "apollo-server-core": "file:packages/apollo-server-core", "apollo-server-env": "file:packages/apollo-server-env", "apollo-server-types": "file:packages/apollo-server-types", diff --git a/packages/apollo-engine-reporting/src/index.ts b/packages/apollo-engine-reporting/src/index.ts index 5770edf02c3..2d4b2f3dc27 100644 --- a/packages/apollo-engine-reporting/src/index.ts +++ b/packages/apollo-engine-reporting/src/index.ts @@ -1,2 +1,2 @@ -export { EngineReportingOptions, EngineReportingAgent } from './agent'; +export { EngineReportingOptions, EngineReportingAgent, keyEnvVar, legacyKeyEnvVar } from './agent'; export { EngineFederatedTracingExtension } from './federatedExtension'; diff --git a/packages/apollo-server-azure-functions/src/ApolloServer.ts b/packages/apollo-server-azure-functions/src/ApolloServer.ts index f8dfb75ad26..455c38ab693 100644 --- a/packages/apollo-server-azure-functions/src/ApolloServer.ts +++ b/packages/apollo-server-azure-functions/src/ApolloServer.ts @@ -8,7 +8,7 @@ import { } from '@apollographql/graphql-playground-html'; import { graphqlAzureFunction } from './azureFunctionApollo'; -import { legacyKeyEnvVar, keyEnvVar } from 'apollo-engine-reporting/src/agent'; +import { legacyKeyEnvVar, keyEnvVar } from 'apollo-engine-reporting'; export interface CreateHandlerOptions { cors?: { diff --git a/packages/apollo-server-cloud-functions/src/ApolloServer.ts b/packages/apollo-server-cloud-functions/src/ApolloServer.ts index 5b7538d0dfe..48c4412c2a6 100644 --- a/packages/apollo-server-cloud-functions/src/ApolloServer.ts +++ b/packages/apollo-server-cloud-functions/src/ApolloServer.ts @@ -6,7 +6,7 @@ import { import { Request, Response } from 'express'; import { graphqlCloudFunction } from './googleCloudApollo'; -import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting/src/agent'; +import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting'; export interface CreateHandlerOptions { cors?: { diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index d9a209f7236..8d94a1cef7e 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -68,7 +68,7 @@ import { import { Headers } from 'apollo-server-env'; import { buildServiceDefinition } from '@apollographql/apollo-tools'; -import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting/src/agent'; +import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting'; const NoIntrospection = (context: ValidationContext) => ({ Field(node: FieldDefinitionNode) { diff --git a/packages/apollo-server-lambda/src/ApolloServer.ts b/packages/apollo-server-lambda/src/ApolloServer.ts index 68dc70d7436..715d24ae2f2 100644 --- a/packages/apollo-server-lambda/src/ApolloServer.ts +++ b/packages/apollo-server-lambda/src/ApolloServer.ts @@ -11,7 +11,7 @@ import { import { graphqlLambda } from './lambdaApollo'; import { Headers } from 'apollo-server-env'; -import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting/src/agent'; +import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting'; export interface CreateHandlerOptions { cors?: { From 952edfbf86249cde4a1ceb946b6c39725da3c5e7 Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Tue, 31 Mar 2020 01:47:12 -0700 Subject: [PATCH 03/13] Add tests --- packages/apollo-engine-reporting/src/agent.ts | 39 ++++++++++--------- packages/apollo-engine-reporting/src/index.ts | 2 +- .../apollo-server-core/src/ApolloServer.ts | 18 ++------- .../src/__tests__/ApolloServerBase.test.ts | 39 +++++++++++++++++++ 4 files changed, 63 insertions(+), 35 deletions(-) diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index 67ee17adc74..7c38d58150d 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -16,11 +16,6 @@ import { GraphQLRequestContext } from 'apollo-server-types'; import { InMemoryLRUCache } from 'apollo-server-caching'; import { defaultEngineReportingSignature } from 'apollo-graphql'; -// TDOD: Reviewer, should we consider moving these exports to a different package -// Maybe apollo-server-env? -export const legacyKeyEnvVar = 'ENGINE_API_KEY'; -export const keyEnvVar = 'APOLLO_KEY' - export interface ClientInfo { clientName?: string; clientVersion?: string; @@ -50,6 +45,22 @@ export type GenerateClientInfo = ( requestContext: GraphQLRequestContext, ) => ClientInfo; +export function getEngineApiKey({engine, shouldWarnOnDeprecatedUsage = true}: { engine: EngineReportingOptions | boolean | undefined, shouldWarnOnDeprecatedUsage?: boolean }) { + if (typeof engine === 'object' && engine.apiKey) { + return engine.apiKey; + } + const legacyApiKeyFromEnv = process.env.ENGINE_API_KEY; + const apiKeyFromEnv = process.env.APOLLO_KEY; + + if(legacyApiKeyFromEnv && apiKeyFromEnv) { + throw new Error(`Cannot set both APOLLO_KEY and ENGINE_API_KEY. Please only set APOLLO_KEY`); + } + if(legacyApiKeyFromEnv && shouldWarnOnDeprecatedUsage) { + console.warn(`[Deprecation warning] Setting the key via ENGINE_API_KEY is deprecated and will not be supported in future versions.`) + } + return apiKeyFromEnv || legacyApiKeyFromEnv || '' +} + export interface EngineReportingOptions { /** * API key for the service. Get this from @@ -213,8 +224,8 @@ const serviceHeaderDefaults = { // EngineReportingExtensions for each request and sends batches of trace reports // to the Engine server. export class EngineReportingAgent { - private options: EngineReportingOptions; - private apiKey: string; + private readonly options: EngineReportingOptions; + private readonly apiKey: string; private reports: { [schemaHash: string]: FullTracesReport } = Object.create( null, ); @@ -231,24 +242,14 @@ export class EngineReportingAgent { public constructor(options: EngineReportingOptions = {}) { this.options = options; - const legacyApiKeyFromEnv = process.env.ENGINE_API_KEY; - const apiKeyFromEnv = process.env.APOLLO_KEY; - - if(legacyApiKeyFromEnv && apiKeyFromEnv) { - throw new Error(`Cannot set both ${legacyKeyEnvVar} and ${keyEnvVar}. Please only set ${keyEnvVar}`); - } - this.apiKey = options.apiKey || apiKeyFromEnv || legacyApiKeyFromEnv || ''; + this.apiKey = getEngineApiKey({engine: this.options}); if (!this.apiKey) { throw new Error( - `To use EngineReportingAgent, you must specify an API key via the apiKey option or the ${keyEnvVar} environment variable.`, + `To use EngineReportingAgent, you must specify an API key via the apiKey option or the APOLLO_KEY environment variable.`, ); } - if(legacyApiKeyFromEnv) { - console.warn(`[Deprecation warning] Setting the key via ${legacyKeyEnvVar} is deprecated and will not be supported in future versions.`) - } - // Since calculating the signature for Engine reporting is potentially an // expensive operation, we'll cache the signatures we generate and re-use // them based on repeated traces for the same `queryHash`. diff --git a/packages/apollo-engine-reporting/src/index.ts b/packages/apollo-engine-reporting/src/index.ts index 2d4b2f3dc27..5770edf02c3 100644 --- a/packages/apollo-engine-reporting/src/index.ts +++ b/packages/apollo-engine-reporting/src/index.ts @@ -1,2 +1,2 @@ -export { EngineReportingOptions, EngineReportingAgent, keyEnvVar, legacyKeyEnvVar } from './agent'; +export { EngineReportingOptions, EngineReportingAgent } from './agent'; export { EngineFederatedTracingExtension } from './federatedExtension'; diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index 8d94a1cef7e..2bbac017ebe 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -68,7 +68,7 @@ import { import { Headers } from 'apollo-server-env'; import { buildServiceDefinition } from '@apollographql/apollo-tools'; -import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting'; +import {getEngineApiKey} from "apollo-engine-reporting/dist/agent"; const NoIntrospection = (context: ValidationContext) => ({ Field(node: FieldDefinitionNode) { @@ -83,18 +83,6 @@ const NoIntrospection = (context: ValidationContext) => ({ }, }); -function getEngineApiKey(engine: Config['engine']): string | undefined { - const keyFromEnv = process.env[keyEnvVar] || process.env[legacyKeyEnvVar] || ''; - if (engine === false) { - return; - } else if (typeof engine === 'object' && engine.apiKey) { - return engine.apiKey; - } else if (keyFromEnv) { - return keyFromEnv; - } - return; -} - function getEngineGraphVariant(engine: Config['engine']): string | undefined { if (engine === false) { return; @@ -106,7 +94,7 @@ function getEngineGraphVariant(engine: Config['engine']): string | undefined { } function getEngineServiceId(engine: Config['engine']): string | undefined { - const engineApiKey = getEngineApiKey(engine); + const engineApiKey = getEngineApiKey({engine, shouldWarnOnDeprecatedUsage: false}); if (engineApiKey) { return engineApiKey.split(':', 2)[1]; } @@ -310,7 +298,7 @@ export class ApolloServerBase { // The truthyness of this value can also be used in other forks of logic // related to Engine, as is the case with EngineReportingAgent just below. this.engineServiceId = getEngineServiceId(engine); - const apiKey = getEngineApiKey(engine); + const apiKey = getEngineApiKey({engine, shouldWarnOnDeprecatedUsage: false}); if (apiKey) { this.engineApiKeyHash = createSHA('sha512') .update(apiKey) diff --git a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts index 27af9c3347f..157d3a81d01 100644 --- a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts +++ b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts @@ -48,3 +48,42 @@ describe('ApolloServerBase construction', () => { ); }); }); + +describe('environment variables', () => { + const OLD_ENV = process.env; + + beforeEach(() => { + jest.resetModules(); + process.env = { ...OLD_ENV }; + delete process.env.ENGINE_API_KEY; + delete process.env.APOLLO_KEY; + }); + + afterEach(() => { + process.env = OLD_ENV; + }); + + it('constructs a reporting agent with the legacy env var and warns', async () => { + // set the variables + process.env.ENGINE_API_KEY = 'just:fake:stuff'; + const spyConsoleWarn = jest.spyOn(console, 'warn').mockImplementation(); + + const server = new ApolloServerBase({ + schema: buildServiceDefinition([{ typeDefs, resolvers }]).schema, + }); + + await server.stop(); + expect(spyConsoleWarn).toHaveBeenCalledTimes(1); + spyConsoleWarn.mockReset(); + }); + + it('throws an error with both the legacy env var and new env var set', async () => { + // set the variables + process.env.ENGINE_API_KEY = 'just:fake:stuff'; + process.env.APOLLO_KEY = 'also:fake:stuff'; + + expect( () => new ApolloServerBase({ + schema: buildServiceDefinition([{ typeDefs, resolvers }]).schema, + })).toThrow(); + }); +}); From 521f59563eab07466fb2dbed4e8ea3737908441d Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Tue, 31 Mar 2020 01:49:30 -0700 Subject: [PATCH 04/13] Add CHANGELOG.md updates --- CHANGELOG.md | 4 ++++ packages/apollo-gateway/CHANGELOG.md | 1 + 2 files changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02b187d6177..2ba16d8416b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ The version headers in this history reflect the versions of Apollo Server itself - [__CHANGELOG for `@apollo/gateway`__](https://github.com/apollographql/apollo-server/blob/master/packages/apollo-gateway/CHANGELOG.md) - [__CHANGELOG for `@apollo/federation`__](https://github.com/apollographql/apollo-server/blob/master/packages/apollo-federation/CHANGELOG.md) +### vNEXT + +- Support APOLLO_KEY for setting the API key for which to report metrics to Apollo Graph Manager and fetch managed configuration for a managed gateway. This change deprecates the ENGINE_API_KEY environment variable in favor of the APOLLO_KEY environment variable and does not allow both to be set. [#3923](https://github.com/apollographql/apollo-server/pull/3923) + ### v2.11.0 - The range of accepted `peerDepedencies` versions for `graphql` has been widened to include `graphql@^15.0.0-rc.2` so as to accommodate the latest release-candidate of the `graphql@15` package, and an intention to support it when it is finally released on the `latest` npm tag. While this change will subdue peer dependency warnings for Apollo Server packages, many dependencies from outside of this repository will continue to raise similar warnings until those packages own `peerDependencies` are updated. It is unlikely that all of those packages will update their ranges prior to the final version of `graphql@15` being released, but if everything is working as expected, the warnings can be safely ignored. [PR #3825](https://github.com/apollographql/apollo-server/pull/3825) diff --git a/packages/apollo-gateway/CHANGELOG.md b/packages/apollo-gateway/CHANGELOG.md index 43d15532089..2bf3f043159 100644 --- a/packages/apollo-gateway/CHANGELOG.md +++ b/packages/apollo-gateway/CHANGELOG.md @@ -3,6 +3,7 @@ ## vNEXT - Fix Typescript generic typing for datasource contexts [#3865](https://github.com/apollographql/apollo-server/pull/3865) This is a fix for the `TContext` typings of the gateway's exposed `GraphQLDataSource` implementations. In their current form, they don't work as intended, or in any manner that's useful for typing the `context` property throughout the class methods. This introduces a type argument `TContext` to the class itself (which defaults to `Record` for existing implementations) and removes the non-operational type arguments on the class methods themselves. +- Support APOLLO_KEY for setting the API key for which to report metrics to Apollo Graph Manager and fetch managed configuration for a managed gateway. This change deprecates the ENGINE_API_KEY environment variable in favor of the APOLLO_KEY environment variable and does not allow both to be set. [#3923](https://github.com/apollographql/apollo-server/pull/3923) ## 0.13.2 From 135b9a29a7ce61b596916bdea1171c38f6069616 Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Tue, 31 Mar 2020 01:53:08 -0700 Subject: [PATCH 05/13] Update serverless packages These do not need to sendReportsImmediately because the reporting agent has support for handling shutdown signals to send off its final reports. --- packages/apollo-server-azure-functions/src/ApolloServer.ts | 7 ------- packages/apollo-server-cloud-functions/src/ApolloServer.ts | 7 ------- packages/apollo-server-lambda/src/ApolloServer.ts | 7 ------- 3 files changed, 21 deletions(-) diff --git a/packages/apollo-server-azure-functions/src/ApolloServer.ts b/packages/apollo-server-azure-functions/src/ApolloServer.ts index 455c38ab693..db7334c3605 100644 --- a/packages/apollo-server-azure-functions/src/ApolloServer.ts +++ b/packages/apollo-server-azure-functions/src/ApolloServer.ts @@ -8,7 +8,6 @@ import { } from '@apollographql/graphql-playground-html'; import { graphqlAzureFunction } from './azureFunctionApollo'; -import { legacyKeyEnvVar, keyEnvVar } from 'apollo-engine-reporting'; export interface CreateHandlerOptions { cors?: { @@ -26,12 +25,6 @@ export class ApolloServer extends ApolloServerBase { // another place, since the documentation becomes much more complicated when // the constructor is not longer shared between all integration constructor(options: Config) { - if (process.env[keyEnvVar] || process.env[legacyKeyEnvVar] || options.engine) { - options.engine = { - sendReportsImmediately: true, - ...(typeof options.engine !== 'boolean' ? options.engine : {}), - }; - } super(options); } diff --git a/packages/apollo-server-cloud-functions/src/ApolloServer.ts b/packages/apollo-server-cloud-functions/src/ApolloServer.ts index 48c4412c2a6..c4424bd51fc 100644 --- a/packages/apollo-server-cloud-functions/src/ApolloServer.ts +++ b/packages/apollo-server-cloud-functions/src/ApolloServer.ts @@ -6,7 +6,6 @@ import { import { Request, Response } from 'express'; import { graphqlCloudFunction } from './googleCloudApollo'; -import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting'; export interface CreateHandlerOptions { cors?: { @@ -24,12 +23,6 @@ export class ApolloServer extends ApolloServerBase { // another place, since the documentation becomes much more complicated when // the constructor is not longer shared between all integration constructor(options: Config) { - if (process.env[keyEnvVar] || process.env[legacyKeyEnvVar] || options.engine) { - options.engine = { - sendReportsImmediately: true, - ...(typeof options.engine !== 'boolean' ? options.engine : {}), - }; - } super(options); } diff --git a/packages/apollo-server-lambda/src/ApolloServer.ts b/packages/apollo-server-lambda/src/ApolloServer.ts index 715d24ae2f2..307f864c884 100644 --- a/packages/apollo-server-lambda/src/ApolloServer.ts +++ b/packages/apollo-server-lambda/src/ApolloServer.ts @@ -11,7 +11,6 @@ import { import { graphqlLambda } from './lambdaApollo'; import { Headers } from 'apollo-server-env'; -import { keyEnvVar, legacyKeyEnvVar } from 'apollo-engine-reporting'; export interface CreateHandlerOptions { cors?: { @@ -30,12 +29,6 @@ export class ApolloServer extends ApolloServerBase { // another place, since the documentation becomes much more complicated when // the constructor is not longer shared between all integration constructor(options: Config) { - if (process.env[keyEnvVar] || process.env[legacyKeyEnvVar] || options.engine) { - options.engine = { - sendReportsImmediately: true, - ...(typeof options.engine !== 'boolean' ? options.engine : {}), - }; - } super(options); } From a7c931656ab5e41fca40435e1411fe8065f2469a Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Mon, 13 Apr 2020 09:55:10 -0700 Subject: [PATCH 06/13] Move warn-once logic to module-level var --- packages/apollo-engine-reporting/src/agent.ts | 9 ++++++--- packages/apollo-server-core/src/ApolloServer.ts | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index e8a3e159288..0618fc9a3e4 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -16,6 +16,8 @@ import { GraphQLRequestContext } from 'apollo-server-types'; import { InMemoryLRUCache } from 'apollo-server-caching'; import { defaultEngineReportingSignature } from 'apollo-graphql'; +let warnedOnDeprecatedApiKey = false; + export interface ClientInfo { clientName?: string; clientVersion?: string; @@ -45,7 +47,7 @@ export type GenerateClientInfo = ( requestContext: GraphQLRequestContext, ) => ClientInfo; -export function getEngineApiKey({engine, shouldWarnOnDeprecatedUsage = true}: { engine: EngineReportingOptions | boolean | undefined, shouldWarnOnDeprecatedUsage?: boolean }) { +export function getEngineApiKey(engine: EngineReportingOptions | boolean | undefined) { if (typeof engine === 'object' && engine.apiKey) { return engine.apiKey; } @@ -55,8 +57,9 @@ export function getEngineApiKey({engine, shouldWarnOnDeprecatedUsage = true}: { if(legacyApiKeyFromEnv && apiKeyFromEnv) { throw new Error(`Cannot set both APOLLO_KEY and ENGINE_API_KEY. Please only set APOLLO_KEY`); } - if(legacyApiKeyFromEnv && shouldWarnOnDeprecatedUsage) { + if(legacyApiKeyFromEnv && !warnedOnDeprecatedApiKey) { console.warn(`[Deprecation warning] Setting the key via ENGINE_API_KEY is deprecated and will not be supported in future versions.`) + warnedOnDeprecatedApiKey = true; } return apiKeyFromEnv || legacyApiKeyFromEnv || '' } @@ -247,7 +250,7 @@ export class EngineReportingAgent { public constructor(options: EngineReportingOptions = {}) { this.options = options; - this.apiKey = getEngineApiKey({engine: this.options}); + this.apiKey = getEngineApiKey(this.options); if (!this.apiKey) { throw new Error( `To use EngineReportingAgent, you must specify an API key via the apiKey option or the APOLLO_KEY environment variable.`, diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index e365cba101f..5e9d67bd2f3 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -100,7 +100,7 @@ function getEngineGraphVariant(engine: Config['engine']): string | undefined { } function getEngineServiceId(engine: Config['engine']): string | undefined { - const engineApiKey = getEngineApiKey({engine, shouldWarnOnDeprecatedUsage: false}); + const engineApiKey = getEngineApiKey(engine); if (engineApiKey) { return engineApiKey.split(':', 2)[1]; } @@ -304,7 +304,7 @@ export class ApolloServerBase { // The truthyness of this value can also be used in other forks of logic // related to Engine, as is the case with EngineReportingAgent just below. this.engineServiceId = getEngineServiceId(engine); - const apiKey = getEngineApiKey({engine, shouldWarnOnDeprecatedUsage: false}); + const apiKey = getEngineApiKey(engine); if (apiKey) { this.engineApiKeyHash = createSHA('sha512') .update(apiKey) From dad149d327aa1db7322d531ffc3a92e97d59e3c8 Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Mon, 13 Apr 2020 09:56:45 -0700 Subject: [PATCH 07/13] Error message clean-up --- packages/apollo-engine-reporting/src/agent.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index 0618fc9a3e4..0bdf6fa2dde 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -55,10 +55,10 @@ export function getEngineApiKey(engine: EngineReportingOptions | boolean | const apiKeyFromEnv = process.env.APOLLO_KEY; if(legacyApiKeyFromEnv && apiKeyFromEnv) { - throw new Error(`Cannot set both APOLLO_KEY and ENGINE_API_KEY. Please only set APOLLO_KEY`); + throw new Error(`Cannot set both APOLLO_KEY and ENGINE_API_KEY. Please only set APOLLO_KEY.`); } if(legacyApiKeyFromEnv && !warnedOnDeprecatedApiKey) { - console.warn(`[Deprecation warning] Setting the key via ENGINE_API_KEY is deprecated and will not be supported in future versions.`) + console.warn(`[Deprecation warning] Setting the key via ENGINE_API_KEY is deprecated and will not be supported in future versions.`); warnedOnDeprecatedApiKey = true; } return apiKeyFromEnv || legacyApiKeyFromEnv || '' From 23fdb54506a0e9f5c0af5a89a71cd382eaaf0ae2 Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Mon, 13 Apr 2020 09:57:52 -0700 Subject: [PATCH 08/13] Apply suggestions from code review Co-Authored-By: Jesse Rosenberger --- CHANGELOG.md | 3 ++- packages/apollo-gateway/CHANGELOG.md | 2 +- .../apollo-server-core/src/__tests__/ApolloServerBase.test.ts | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ba16d8416b..f2e186572e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,8 @@ The version headers in this history reflect the versions of Apollo Server itself ### vNEXT -- Support APOLLO_KEY for setting the API key for which to report metrics to Apollo Graph Manager and fetch managed configuration for a managed gateway. This change deprecates the ENGINE_API_KEY environment variable in favor of the APOLLO_KEY environment variable and does not allow both to be set. [#3923](https://github.com/apollographql/apollo-server/pull/3923) +- Deprecate the `ENGINE_API_KEY` environment variable in favor of a newly introduced `APOLLO_KEY`. Continued use of `ENGINE_API_KEY` will result in deprecation warnings and support for it will be removed in a future major version. + [#3923](https://github.com/apollographql/apollo-server/pull/3923) ### v2.11.0 diff --git a/packages/apollo-gateway/CHANGELOG.md b/packages/apollo-gateway/CHANGELOG.md index 2bf3f043159..5f3fab12bd8 100644 --- a/packages/apollo-gateway/CHANGELOG.md +++ b/packages/apollo-gateway/CHANGELOG.md @@ -3,7 +3,7 @@ ## vNEXT - Fix Typescript generic typing for datasource contexts [#3865](https://github.com/apollographql/apollo-server/pull/3865) This is a fix for the `TContext` typings of the gateway's exposed `GraphQLDataSource` implementations. In their current form, they don't work as intended, or in any manner that's useful for typing the `context` property throughout the class methods. This introduces a type argument `TContext` to the class itself (which defaults to `Record` for existing implementations) and removes the non-operational type arguments on the class methods themselves. -- Support APOLLO_KEY for setting the API key for which to report metrics to Apollo Graph Manager and fetch managed configuration for a managed gateway. This change deprecates the ENGINE_API_KEY environment variable in favor of the APOLLO_KEY environment variable and does not allow both to be set. [#3923](https://github.com/apollographql/apollo-server/pull/3923) +- Deprecate the `ENGINE_API_KEY` environment variable in favor of a newly introduced `APOLLO_KEY`. Continued use of `ENGINE_API_KEY` will result in deprecation warnings and support for it will be removed in a future major version. [#3923](https://github.com/apollographql/apollo-server/pull/3923) ## 0.13.2 diff --git a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts index 157d3a81d01..8d00af96696 100644 --- a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts +++ b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts @@ -69,8 +69,8 @@ describe('environment variables', () => { const spyConsoleWarn = jest.spyOn(console, 'warn').mockImplementation(); const server = new ApolloServerBase({ - schema: buildServiceDefinition([{ typeDefs, resolvers }]).schema, - }); + schema: buildServiceDefinition([{ typeDefs, resolvers }]).schema, + }); await server.stop(); expect(spyConsoleWarn).toHaveBeenCalledTimes(1); From 83d74902b147405e9a1aca50e801678d163a0fcb Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Mon, 13 Apr 2020 09:59:44 -0700 Subject: [PATCH 09/13] Roll back changes to cloud function wrappers --- packages/apollo-server-azure-functions/package.json | 4 ++-- .../apollo-server-azure-functions/src/ApolloServer.ts | 6 ++++++ packages/apollo-server-cloud-functions/package.json | 5 ++--- .../apollo-server-cloud-functions/src/ApolloServer.ts | 8 +++++++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/apollo-server-azure-functions/package.json b/packages/apollo-server-azure-functions/package.json index eb569b7eed8..1f2b267c1e1 100644 --- a/packages/apollo-server-azure-functions/package.json +++ b/packages/apollo-server-azure-functions/package.json @@ -1,6 +1,6 @@ { "name": "apollo-server-azure-functions", - "version": "2.11.0", + "version": "2.12.0", "description": "Production-ready Node.js GraphQL server for Azure Functions", "keywords": [ "GraphQL", @@ -36,6 +36,6 @@ "apollo-server-integration-testsuite": "file:../apollo-server-integration-testsuite" }, "peerDependencies": { - "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0-rc.2" + "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0" } } diff --git a/packages/apollo-server-azure-functions/src/ApolloServer.ts b/packages/apollo-server-azure-functions/src/ApolloServer.ts index db7334c3605..f5e5d68ce39 100644 --- a/packages/apollo-server-azure-functions/src/ApolloServer.ts +++ b/packages/apollo-server-azure-functions/src/ApolloServer.ts @@ -25,6 +25,12 @@ export class ApolloServer extends ApolloServerBase { // another place, since the documentation becomes much more complicated when // the constructor is not longer shared between all integration constructor(options: Config) { + if (process.env.ENGINE_API_KEY || options.engine) { + options.engine = { + sendReportsImmediately: true, + ...(typeof options.engine !== 'boolean' ? options.engine : {}), + }; + } super(options); } diff --git a/packages/apollo-server-cloud-functions/package.json b/packages/apollo-server-cloud-functions/package.json index 4e8c8f12733..f04f58019d1 100644 --- a/packages/apollo-server-cloud-functions/package.json +++ b/packages/apollo-server-cloud-functions/package.json @@ -1,6 +1,6 @@ { "name": "apollo-server-cloud-functions", - "version": "2.11.0", + "version": "2.12.0", "description": "Production-ready Node.js GraphQL server for Google Cloud Functions", "keywords": [ "GraphQL", @@ -29,13 +29,12 @@ "apollo-server-core": "file:../apollo-server-core", "apollo-server-env": "file:../apollo-server-env", "apollo-server-types": "file:../apollo-server-types", - "apollo-engine-reporting":"file:../apollo-engine-reporting", "graphql-tools": "^4.0.0" }, "devDependencies": { "apollo-server-integration-testsuite": "file:../apollo-server-integration-testsuite" }, "peerDependencies": { - "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0-rc.2" + "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0" } } diff --git a/packages/apollo-server-cloud-functions/src/ApolloServer.ts b/packages/apollo-server-cloud-functions/src/ApolloServer.ts index c4424bd51fc..7971e1c7598 100644 --- a/packages/apollo-server-cloud-functions/src/ApolloServer.ts +++ b/packages/apollo-server-cloud-functions/src/ApolloServer.ts @@ -23,6 +23,12 @@ export class ApolloServer extends ApolloServerBase { // another place, since the documentation becomes much more complicated when // the constructor is not longer shared between all integration constructor(options: Config) { + if (process.env.ENGINE_API_KEY || options.engine) { + options.engine = { + sendReportsImmediately: true, + ...(typeof options.engine !== 'boolean' ? options.engine : {}), + }; + } super(options); } @@ -108,7 +114,7 @@ export class ApolloServer extends ApolloServerBase { ); } } - + res.set(corsHeaders); if (req.method === 'OPTIONS') { From fd035426cb3e1d49a45aef34050e4854e8e52c92 Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Mon, 13 Apr 2020 11:30:26 -0700 Subject: [PATCH 10/13] Update test --- .../apollo-server-core/src/__tests__/ApolloServerBase.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts index 8d00af96696..bc218b38d7e 100644 --- a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts +++ b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts @@ -63,7 +63,7 @@ describe('environment variables', () => { process.env = OLD_ENV; }); - it('constructs a reporting agent with the legacy env var and warns', async () => { + it('constructs a reporting agent with the ENGINE_API_KEY (deprecated) environment variable and warns', async () => { // set the variables process.env.ENGINE_API_KEY = 'just:fake:stuff'; const spyConsoleWarn = jest.spyOn(console, 'warn').mockImplementation(); From 493c4aafacb7c648b54fd14c379bb82546f70049 Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Mon, 13 Apr 2020 11:30:55 -0700 Subject: [PATCH 11/13] Undo changes to apollo-server-lambda --- packages/apollo-server-lambda/package.json | 4 ++-- packages/apollo-server-lambda/src/ApolloServer.ts | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/apollo-server-lambda/package.json b/packages/apollo-server-lambda/package.json index 1d9ac1319ab..a33da3c6388 100644 --- a/packages/apollo-server-lambda/package.json +++ b/packages/apollo-server-lambda/package.json @@ -1,6 +1,6 @@ { "name": "apollo-server-lambda", - "version": "2.11.0", + "version": "2.12.0", "description": "Production-ready Node.js GraphQL server for AWS Lambda", "keywords": [ "GraphQL", @@ -36,6 +36,6 @@ "apollo-server-integration-testsuite": "file:../apollo-server-integration-testsuite" }, "peerDependencies": { - "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0-rc.2" + "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0" } } diff --git a/packages/apollo-server-lambda/src/ApolloServer.ts b/packages/apollo-server-lambda/src/ApolloServer.ts index 307f864c884..181de51b061 100644 --- a/packages/apollo-server-lambda/src/ApolloServer.ts +++ b/packages/apollo-server-lambda/src/ApolloServer.ts @@ -29,6 +29,12 @@ export class ApolloServer extends ApolloServerBase { // another place, since the documentation becomes much more complicated when // the constructor is not longer shared between all integration constructor(options: Config) { + if (process.env.ENGINE_API_KEY || options.engine) { + options.engine = { + sendReportsImmediately: true, + ...(typeof options.engine !== 'boolean' ? options.engine : {}), + }; + } super(options); } From a83a1f5abfe073d76a678865f2246ccaa17b704d Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Mon, 13 Apr 2020 12:48:37 -0700 Subject: [PATCH 12/13] Update both keys set to warn If both APOLLO_KEY and ENGINE_API_KEY are set, this will now warn and prefer APOLLO_KEY, in accordance with changes to apollo-tooling. Imp personally not very opinionated on this front, so happy to take direction from the reviewer. --- packages/apollo-engine-reporting/src/agent.ts | 16 +++++++++------- .../apollo-server-core/src/ApolloServer.ts | 8 ++++---- .../src/__tests__/ApolloServerBase.test.ts | 18 +++++++++++++----- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index 0bdf6fa2dde..81d6e702a07 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -47,18 +47,20 @@ export type GenerateClientInfo = ( requestContext: GraphQLRequestContext, ) => ClientInfo; -export function getEngineApiKey(engine: EngineReportingOptions | boolean | undefined) { - if (typeof engine === 'object' && engine.apiKey) { - return engine.apiKey; +export function getEngineApiKey(engine: EngineReportingOptions | boolean | undefined, skipWarn: boolean = false) { + if (typeof engine === 'object') { + if (engine.apiKey) { + return engine.apiKey; + } } const legacyApiKeyFromEnv = process.env.ENGINE_API_KEY; const apiKeyFromEnv = process.env.APOLLO_KEY; - if(legacyApiKeyFromEnv && apiKeyFromEnv) { - throw new Error(`Cannot set both APOLLO_KEY and ENGINE_API_KEY. Please only set APOLLO_KEY.`); + if(legacyApiKeyFromEnv && apiKeyFromEnv && !skipWarn) { + console.warn(`Both ENGINE_API_KEY (deprecated) and APOLLO_KEY are set; defaulting to APOLLO_KEY.`); } - if(legacyApiKeyFromEnv && !warnedOnDeprecatedApiKey) { - console.warn(`[Deprecation warning] Setting the key via ENGINE_API_KEY is deprecated and will not be supported in future versions.`); + if(legacyApiKeyFromEnv && !warnedOnDeprecatedApiKey && !skipWarn) { + console.warn(`[deprecated] Setting the key via ENGINE_API_KEY is deprecated and will not be supported in future versions.`); warnedOnDeprecatedApiKey = true; } return apiKeyFromEnv || legacyApiKeyFromEnv || '' diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index 5e9d67bd2f3..8445328cb8d 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -90,7 +90,7 @@ function getEngineGraphVariant(engine: Config['engine']): string | undefined { return engine.graphVariant || engine.schemaTag; } else { if (process.env.ENGINE_SCHEMA_TAG) { - console.warn('[Deprecation warning] Usage of ENGINE_SCHEMA_TAG is deprecated. Please use APOLLO_GRAPH_VARIANT instead.'); + console.warn('[deprecated] Usage of ENGINE_SCHEMA_TAG is deprecated. Please use APOLLO_GRAPH_VARIANT instead.'); } if (process.env.ENGINE_SCHEMA_TAG && process.env.APOLLO_GRAPH_VARIANT) { throw new Error('Cannot set both ENGINE_SCHEMA_TAG and APOLLO_GRAPH_VARIANT. Please use APOLLO_GRAPH_VARIANT.') @@ -100,7 +100,7 @@ function getEngineGraphVariant(engine: Config['engine']): string | undefined { } function getEngineServiceId(engine: Config['engine']): string | undefined { - const engineApiKey = getEngineApiKey(engine); + const engineApiKey = getEngineApiKey(engine, true); if (engineApiKey) { return engineApiKey.split(':', 2)[1]; } @@ -304,7 +304,7 @@ export class ApolloServerBase { // The truthyness of this value can also be used in other forks of logic // related to Engine, as is the case with EngineReportingAgent just below. this.engineServiceId = getEngineServiceId(engine); - const apiKey = getEngineApiKey(engine); + const apiKey = getEngineApiKey(engine, true); if (apiKey) { this.engineApiKeyHash = createSHA('sha512') .update(apiKey) @@ -314,7 +314,7 @@ export class ApolloServerBase { if (this.engineServiceId) { const { EngineReportingAgent } = require('apollo-engine-reporting'); this.engineReportingAgent = new EngineReportingAgent( - typeof engine === 'object' ? engine : Object.create(null), + typeof engine === 'object' ? engine : Object.create(null) ); // Don't add the extension here (we want to add it later in generateSchemaDerivedData). } diff --git a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts index bc218b38d7e..c5c898002f5 100644 --- a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts +++ b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts @@ -69,7 +69,8 @@ describe('environment variables', () => { const spyConsoleWarn = jest.spyOn(console, 'warn').mockImplementation(); const server = new ApolloServerBase({ - schema: buildServiceDefinition([{ typeDefs, resolvers }]).schema, + typeDefs, + resolvers }); await server.stop(); @@ -77,13 +78,20 @@ describe('environment variables', () => { spyConsoleWarn.mockReset(); }); - it('throws an error with both the legacy env var and new env var set', async () => { + it('warns with both the legacy env var and new env var set', async () => { // set the variables process.env.ENGINE_API_KEY = 'just:fake:stuff'; process.env.APOLLO_KEY = 'also:fake:stuff'; + const spyConsoleWarn = jest.spyOn(console, 'warn').mockImplementation(); + + const server = new ApolloServerBase({ + typeDefs, + resolvers + }); - expect( () => new ApolloServerBase({ - schema: buildServiceDefinition([{ typeDefs, resolvers }]).schema, - })).toThrow(); + await server.stop(); + // Once for deprecation, once for double-set + expect(spyConsoleWarn).toHaveBeenCalledTimes(2); + spyConsoleWarn.mockReset(); }); }); From cab5b0e5ee34c2f94d48bb50e75d8297ed6fb8c9 Mon Sep 17 00:00:00 2001 From: Adam Zionts Date: Wed, 15 Apr 2020 12:20:43 -0700 Subject: [PATCH 13/13] Use custom logger --- packages/apollo-engine-reporting/src/agent.ts | 11 +++++++---- packages/apollo-server-core/src/ApolloServer.ts | 8 ++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index 249c8cd39ce..d5833b334dd 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -47,7 +47,10 @@ export type GenerateClientInfo = ( requestContext: GraphQLRequestContext, ) => ClientInfo; -export function getEngineApiKey(engine: EngineReportingOptions | boolean | undefined, skipWarn: boolean = false) { +export function getEngineApiKey( + {engine, skipWarn = false, logger= console }: + {engine: EngineReportingOptions | boolean | undefined, skipWarn?: boolean, logger?: Logger } + ) { if (typeof engine === 'object') { if (engine.apiKey) { return engine.apiKey; @@ -57,10 +60,10 @@ export function getEngineApiKey(engine: EngineReportingOptions | boolean | const apiKeyFromEnv = process.env.APOLLO_KEY; if(legacyApiKeyFromEnv && apiKeyFromEnv && !skipWarn) { - console.warn(`Both ENGINE_API_KEY (deprecated) and APOLLO_KEY are set; defaulting to APOLLO_KEY.`); + logger.warn(`Both ENGINE_API_KEY (deprecated) and APOLLO_KEY are set; defaulting to APOLLO_KEY.`); } if(legacyApiKeyFromEnv && !warnedOnDeprecatedApiKey && !skipWarn) { - console.warn(`[deprecated] Setting the key via ENGINE_API_KEY is deprecated and will not be supported in future versions.`); + logger.warn(`[deprecated] Setting the key via ENGINE_API_KEY is deprecated and will not be supported in future versions.`); warnedOnDeprecatedApiKey = true; } return apiKeyFromEnv || legacyApiKeyFromEnv || '' @@ -259,7 +262,7 @@ export class EngineReportingAgent { public constructor(options: EngineReportingOptions = {}) { this.options = options; - this.apiKey = getEngineApiKey(this.options); + this.apiKey = getEngineApiKey({engine: this.options, skipWarn: false, logger: this.logger}); if (options.logger) this.logger = options.logger; if (!this.apiKey) { throw new Error( diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index 02798717e79..ea46b7ffc9b 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -101,8 +101,8 @@ function getEngineGraphVariant(engine: Config['engine']): string | undefined { } } -function getEngineServiceId(engine: Config['engine']): string | undefined { - const engineApiKey = getEngineApiKey(engine, true); +function getEngineServiceId(engine: Config['engine'], logger: Logger): string | undefined { + const engineApiKey = getEngineApiKey({engine, skipWarn: true, logger} ); if (engineApiKey) { return engineApiKey.split(':', 2)[1]; } @@ -332,8 +332,8 @@ export class ApolloServerBase { // service ID from the API key for plugins which only needs service ID. // The truthiness of this value can also be used in other forks of logic // related to Engine, as is the case with EngineReportingAgent just below. - this.engineServiceId = getEngineServiceId(engine); - const apiKey = getEngineApiKey(engine, true); + this.engineServiceId = getEngineServiceId(engine, this.logger); + const apiKey = getEngineApiKey({engine, skipWarn: true, logger: this.logger}); if (apiKey) { this.engineApiKeyHash = createSHA('sha512') .update(apiKey)