From 641acb98f8b9fa325fc279cf4f3ee8c06fef2008 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Wed, 18 Jan 2023 15:58:50 -0800 Subject: [PATCH] Expand on non-nullability of `_Service.sdl` field (#7301) Fed v2 updated the subgraph spec to make `_Service.sdl` non-nullable. We should explain this thoroughly in our docs, test for both cases, and update a relevant recent changelog entry. Follow-up to #7274. Addresses https://github.com/apollographql/apollo-server/pull/7274#discussion_r1061064648 --- .changeset/quiet-pears-float.md | 7 +- docs/source/api/plugin/inline-trace.mdx | 4 +- .../src/apolloServerTests.ts | 163 ++++++++++-------- 3 files changed, 97 insertions(+), 77 deletions(-) diff --git a/.changeset/quiet-pears-float.md b/.changeset/quiet-pears-float.md index b7feaa1aa42..a04b4889b9a 100644 --- a/.changeset/quiet-pears-float.md +++ b/.changeset/quiet-pears-float.md @@ -2,4 +2,9 @@ '@apollo/server': patch --- -Update schemaIsSubgraph to also support non nullable \_Service.sdl +The subgraph spec has evolved in Federation v2 such that the type of +`_Service.sdl` (formerly nullable) is now non-nullable. Apollo Server now +detects both cases correctly in order to determine whether to: +1. install / enable the `ApolloServerPluginInlineTrace` plugin +2. throw on startup if `ApolloServerPluginSchemaReporting` should not be installed +3. warn when `ApolloServerPluginUsageReporting` is installed and configured with the `__onlyIfSchemaIsNotSubgraph` option diff --git a/docs/source/api/plugin/inline-trace.mdx b/docs/source/api/plugin/inline-trace.mdx index 953b9fd7a7f..0077000c282 100644 --- a/docs/source/api/plugin/inline-trace.mdx +++ b/docs/source/api/plugin/inline-trace.mdx @@ -11,7 +11,7 @@ This article documents the options for the `ApolloServerPluginInlineTrace` plugi This plugin enables your GraphQL server to include encoded performance and usage traces inside responses. This is primarily designed for use with [Apollo Federation](/federation/metrics/). Federated subgraphs use this plugin and include a trace in the `ftv1` GraphQL response extension if requested to do so by the Apollo gateway. The gateway requests this trace by passing the HTTP header `apollo-federation-include-trace: ftv1`. -Apollo Server installs this plugin by default in all federated subgraphs, with its default configuration. Apollo Server inspects whether or not the schema it is serving includes a field `_Service.sdl: String` to determine if it is a federated subgraph. You typically do not have to install this plugin yourself; you only need to do so if you want to provide non-default configuration. +Apollo Server installs this plugin by default in all federated subgraphs, with its default configuration. Apollo Server inspects whether or not the schema it is serving includes a field `_Service.sdl: String!` (as well as its former, nullable version from Federation v1: `_Service.sdl: String`) to determine if it is a federated subgraph. You typically do not have to install this plugin yourself; you only need to do so if you want to provide non-default configuration. If you want to configure the `ApolloServerPluginInlineTrace` plugin, import it and pass it to your `ApolloServer` constructor's `plugins` array: @@ -34,7 +34,7 @@ const server = new ApolloServer({ -If you don't want to use the inline trace plugin even though your schema defines `_Service.sdl: String`, you can explicitly disable it with the `ApolloServerPluginInlineTraceDisabled` plugin: +If you don't want to use the inline trace plugin even though your schema defines `_Service.sdl: String!`, you can explicitly disable it with the `ApolloServerPluginInlineTraceDisabled` plugin: diff --git a/packages/integration-testsuite/src/apolloServerTests.ts b/packages/integration-testsuite/src/apolloServerTests.ts index 325c3f1ab51..78b71739fca 100644 --- a/packages/integration-testsuite/src/apolloServerTests.ts +++ b/packages/integration-testsuite/src/apolloServerTests.ts @@ -2456,12 +2456,18 @@ export function defineIntegrationTestSuiteApolloServerTests( describe('Federated tracing', () => { // Enable federated tracing by pretending to be federated. - const federationTypeDefs = gql` + const federationV1TypeDefs = gql` type _Service { sdl: String } `; + const federationV2TypeDefs = gql` + type _Service { + sdl: String! + } + `; + const baseTypeDefs = gql` type Book { title: String @@ -2479,7 +2485,8 @@ export function defineIntegrationTestSuiteApolloServerTests( } `; - const allTypeDefs = [federationTypeDefs, baseTypeDefs]; + const v1TypeDefs = [federationV1TypeDefs, baseTypeDefs]; + const v2TypeDefs = [federationV2TypeDefs, baseTypeDefs]; const resolvers = { Query: { @@ -2506,7 +2513,7 @@ export function defineIntegrationTestSuiteApolloServerTests( it("doesn't include federated trace without the special header", async () => { const uri = await createServerAndGetUrl({ - typeDefs: allTypeDefs, + typeDefs: v2TypeDefs, resolvers, logger: quietLogger, }); @@ -2535,94 +2542,102 @@ export function defineIntegrationTestSuiteApolloServerTests( expect(result.extensions).toBeUndefined(); }); - it('reports a total duration that is longer than the duration of its resolvers', async () => { - const uri = await createServerAndGetUrl({ - typeDefs: allTypeDefs, - resolvers, - logger: quietLogger, - }); + describe.each([ + ['nullable _Service.sdl field', v1TypeDefs], + ['non-nullable _Service.sdl! field', v2TypeDefs], + ])('with %s', (_, typeDefs) => { + it('reports a total duration that is longer than the duration of its resolvers', async () => { + const uri = await createServerAndGetUrl({ + typeDefs, + resolvers, + logger: quietLogger, + }); - const apolloFetch = createApolloFetchAsIfFromGateway(uri); + const apolloFetch = createApolloFetchAsIfFromGateway(uri); - const result = await apolloFetch({ - query: `{ books { title author } }`, - }); + const result = await apolloFetch({ + query: `{ books { title author } }`, + }); - const ftv1: string = result.extensions.ftv1; + const ftv1: string = result.extensions.ftv1; - expect(ftv1).toBeTruthy(); - const encoded = Buffer.from(ftv1, 'base64'); - const trace = Trace.decode(encoded); + expect(ftv1).toBeTruthy(); + const encoded = Buffer.from(ftv1, 'base64'); + const trace = Trace.decode(encoded); - let earliestStartOffset = Infinity; - let latestEndOffset = -Infinity; + let earliestStartOffset = Infinity; + let latestEndOffset = -Infinity; - function walk(node: Trace.Node) { - if (node.startTime !== 0 && node.endTime !== 0) { - earliestStartOffset = Math.min(earliestStartOffset, node.startTime); - latestEndOffset = Math.max(latestEndOffset, node.endTime); + function walk(node: Trace.Node) { + if (node.startTime !== 0 && node.endTime !== 0) { + earliestStartOffset = Math.min( + earliestStartOffset, + node.startTime, + ); + latestEndOffset = Math.max(latestEndOffset, node.endTime); + } + node.child.forEach((n) => walk(n as Trace.Node)); } - node.child.forEach((n) => walk(n as Trace.Node)); - } - walk(trace.root as Trace.Node); - expect(earliestStartOffset).toBeLessThan(Infinity); - expect(latestEndOffset).toBeGreaterThan(-Infinity); - const resolverDuration = latestEndOffset - earliestStartOffset; - expect(resolverDuration).toBeGreaterThan(0); - expect(trace.durationNs).toBeGreaterThanOrEqual(resolverDuration); + walk(trace.root as Trace.Node); + expect(earliestStartOffset).toBeLessThan(Infinity); + expect(latestEndOffset).toBeGreaterThan(-Infinity); + const resolverDuration = latestEndOffset - earliestStartOffset; + expect(resolverDuration).toBeGreaterThan(0); + expect(trace.durationNs).toBeGreaterThanOrEqual(resolverDuration); - expect(trace.startTime!.seconds).toBeLessThanOrEqual( - trace.endTime!.seconds!, - ); - if (trace.startTime!.seconds === trace.endTime!.seconds) { - expect(trace.startTime!.nanos).toBeLessThanOrEqual( - trace.endTime!.nanos!, + expect(trace.startTime!.seconds).toBeLessThanOrEqual( + trace.endTime!.seconds!, ); - } - }); + if (trace.startTime!.seconds === trace.endTime!.seconds) { + expect(trace.startTime!.nanos).toBeLessThanOrEqual( + trace.endTime!.nanos!, + ); + } + }); - it('includes errors in federated trace', async () => { - const uri = await createServerAndGetUrl({ - typeDefs: allTypeDefs, - resolvers, - formatError(err) { - return { - ...err, - message: `Formatted: ${err.message}`, - }; - }, - plugins: [ - ApolloServerPluginInlineTrace({ - includeErrors: { - transform(err) { - err.message = `Rewritten for Usage Reporting: ${err.message}`; - return err; + it('includes errors in federated trace', async () => { + const uri = await createServerAndGetUrl({ + typeDefs, + resolvers, + formatError(err) { + return { + ...err, + message: `Formatted: ${err.message}`, + }; + }, + plugins: [ + ApolloServerPluginInlineTrace({ + includeErrors: { + transform(err) { + err.message = `Rewritten for Usage Reporting: ${err.message}`; + return err; + }, }, - }, - }), - ], - }); + }), + ], + }); - const apolloFetch = createApolloFetchAsIfFromGateway(uri); + const apolloFetch = createApolloFetchAsIfFromGateway(uri); - const result = await apolloFetch({ - query: `{ error }`, - }); + const result = await apolloFetch({ + query: `{ error }`, + }); - expect(result.data).toStrictEqual({ error: null }); - expect(result.errors).toBeTruthy(); - expect(result.errors.length).toBe(1); - expect(result.errors[0].message).toBe('Formatted: It broke'); + expect(result.data).toStrictEqual({ error: null }); + expect(result.errors).toBeTruthy(); + expect(result.errors.length).toBe(1); + expect(result.errors[0].message).toBe('Formatted: It broke'); - const ftv1: string = result.extensions.ftv1; + const ftv1: string = result.extensions.ftv1; - expect(ftv1).toBeTruthy(); - const encoded = Buffer.from(ftv1, 'base64'); - const trace = Trace.decode(encoded); - expect(trace.root!.child![0].error![0].message).toBe( - 'Rewritten for Usage Reporting: It broke', - ); + expect(ftv1).toBeTruthy(); + const encoded = Buffer.from(ftv1, 'base64'); + const trace = Trace.decode(encoded); + expect(trace.root!.child![0].error![0].message).toBe( + 'Rewritten for Usage Reporting: It broke', + ); + }); }); });