diff --git a/package-lock.json b/package-lock.json index e958786947a..608596fc571 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ ], "devDependencies": { "@apollo/client": "3.7.1", - "@apollo/gateway": "https://output.circle-artifacts.com/output/job/3ea84c49-ca22-4dba-be5d-31004cf8d66a/artifacts/0/storage/@apollo/gateway/gateway-2.1.4.tgz", + "@apollo/gateway": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/gateway", "@apollo/server-plugin-landing-page-graphql-playground": "4.0.0", "@apollo/subgraph": "2.1.4", "@apollo/utils.createhash": "1.1.0", @@ -149,12 +149,13 @@ }, "node_modules/@apollo/composition": { "version": "2.1.4", - "resolved": "https://registry.npmjs.org/@apollo/composition/-/composition-2.1.4.tgz", - "integrity": "sha512-M72m29u4Zx0EHaLO989YGH+shnRE3ADKRtOSe3xx2eESHpN5GuugihQF4bdGKBFpseKBaJwl0x060eq8MVjSeg==", + "resolved": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/composition", + "integrity": "sha512-oKh6y8HDT2q7PjXzyee+UalcUQe0INzZMeItt1+lxU0Njl564Ze/ydhvbuL4LIejY3dUkVlvG6LtlSPXnVuzFg==", "dev": true, + "license": "SEE LICENSE IN ./LICENSE", "dependencies": { - "@apollo/federation-internals": "^2.1.4", - "@apollo/query-graphs": "^2.1.4" + "@apollo/federation-internals": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", + "@apollo/query-graphs": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-graphs" }, "engines": { "node": ">=12.13.0" @@ -165,9 +166,10 @@ }, "node_modules/@apollo/federation-internals": { "version": "2.1.4", - "resolved": "https://registry.npmjs.org/@apollo/federation-internals/-/federation-internals-2.1.4.tgz", - "integrity": "sha512-Fpln5hHEh0Jjy0vaTpIlv1SF+zg2cjunsH2X5QDHwKOXoHsT4o7LF+Duoy7LgZj6W3HNW9Z/srmuOXXNTl2gjw==", + "resolved": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", + "integrity": "sha512-V7VrJ9qpvGEmvJj68HLTW/3u9A1ToXuRrG5P4QPZbrJZkNotTB3ZkcCpBOcnsDh4B7py+6I9pmXtUdxx1MIRHw==", "dev": true, + "license": "SEE LICENSE IN ./LICENSE", "dependencies": { "chalk": "^4.1.0", "js-levenshtein": "^1.1.6" @@ -181,14 +183,14 @@ }, "node_modules/@apollo/gateway": { "version": "2.1.4", - "resolved": "https://output.circle-artifacts.com/output/job/3ea84c49-ca22-4dba-be5d-31004cf8d66a/artifacts/0/storage/@apollo/gateway/gateway-2.1.4.tgz", - "integrity": "sha512-zKtJ4VBCLc8uCXIwrCw/B6OF90INhkDsxh2s1eXEPnnMZp8FgZ6HWRhQLcHCNZpJBR8bCivIsCwXv0aqxJoppA==", + "resolved": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/gateway", + "integrity": "sha512-58/SWBfouBHKBmpQMVH+1I4RItEzmv01M2L0/TFxgiYOiFmXKznkKUrGw30Ain24BD9un4VrJHmB4TaHhlAUYw==", "dev": true, "license": "SEE LICENSE IN ./LICENSE", "dependencies": { - "@apollo/composition": "^2.1.4", - "@apollo/federation-internals": "^2.1.4", - "@apollo/query-planner": "^2.1.4", + "@apollo/composition": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/composition", + "@apollo/federation-internals": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", + "@apollo/query-planner": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-planner", "@apollo/server-gateway-interface": "https://pkg.csb.dev/apollographql/apollo-server/commit/515ae7f7/@apollo/server-gateway-interface", "@apollo/usage-reporting-protobuf": "^4.0.0", "@apollo/utils.createhash": "^1.1.0", @@ -276,11 +278,12 @@ }, "node_modules/@apollo/query-graphs": { "version": "2.1.4", - "resolved": "https://registry.npmjs.org/@apollo/query-graphs/-/query-graphs-2.1.4.tgz", - "integrity": "sha512-oGDdfj+Mhh02JIU0onc0yVxb4f3jqmyHEdTveNYmnmEtFjJWPn9PGq0hpFwTquHyrzUNkx0pqDc4UIFUnZQNGg==", + "resolved": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-graphs", + "integrity": "sha512-l1kLisKHEC2XhqZGp3V712fWUFXEDqPqD4qxrocWOUu+C1p7NQDi2yFewKXqJrl97+Z9eajaiiprxSznOHSPvw==", "dev": true, + "license": "SEE LICENSE IN ./LICENSE", "dependencies": { - "@apollo/federation-internals": "^2.1.4", + "@apollo/federation-internals": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", "deep-equal": "^2.0.5", "ts-graphviz": "^0.16.0" }, @@ -293,12 +296,13 @@ }, "node_modules/@apollo/query-planner": { "version": "2.1.4", - "resolved": "https://registry.npmjs.org/@apollo/query-planner/-/query-planner-2.1.4.tgz", - "integrity": "sha512-OaC52LslJN/uGwJSdJCwJUz1e/0vo+gbHXk1deuRQiM5kr8iV8yklxeMVzy68PG9ioeA3r5SUDIZC+NuCLZeaQ==", + "resolved": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-planner", + "integrity": "sha512-AgtNRoISL7O9sNapVeQTqe4VNKS92UvxP0AzgDWWPd1pVudG8+gCyyZax2N2bYjiRfbLs0RONIaFpcKheBBqVg==", "dev": true, + "license": "SEE LICENSE IN ./LICENSE", "dependencies": { - "@apollo/federation-internals": "^2.1.4", - "@apollo/query-graphs": "^2.1.4", + "@apollo/federation-internals": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", + "@apollo/query-graphs": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-graphs", "chalk": "^4.1.0", "deep-equal": "^2.0.5", "pretty-format": "^28.0.0" @@ -14081,19 +14085,17 @@ } }, "@apollo/composition": { - "version": "2.1.4", - "resolved": "https://registry.npmjs.org/@apollo/composition/-/composition-2.1.4.tgz", - "integrity": "sha512-M72m29u4Zx0EHaLO989YGH+shnRE3ADKRtOSe3xx2eESHpN5GuugihQF4bdGKBFpseKBaJwl0x060eq8MVjSeg==", + "version": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/composition", + "integrity": "sha512-oKh6y8HDT2q7PjXzyee+UalcUQe0INzZMeItt1+lxU0Njl564Ze/ydhvbuL4LIejY3dUkVlvG6LtlSPXnVuzFg==", "dev": true, "requires": { - "@apollo/federation-internals": "^2.1.4", - "@apollo/query-graphs": "^2.1.4" + "@apollo/federation-internals": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", + "@apollo/query-graphs": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-graphs" } }, "@apollo/federation-internals": { - "version": "2.1.4", - "resolved": "https://registry.npmjs.org/@apollo/federation-internals/-/federation-internals-2.1.4.tgz", - "integrity": "sha512-Fpln5hHEh0Jjy0vaTpIlv1SF+zg2cjunsH2X5QDHwKOXoHsT4o7LF+Duoy7LgZj6W3HNW9Z/srmuOXXNTl2gjw==", + "version": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", + "integrity": "sha512-V7VrJ9qpvGEmvJj68HLTW/3u9A1ToXuRrG5P4QPZbrJZkNotTB3ZkcCpBOcnsDh4B7py+6I9pmXtUdxx1MIRHw==", "dev": true, "requires": { "chalk": "^4.1.0", @@ -14101,13 +14103,13 @@ } }, "@apollo/gateway": { - "version": "https://output.circle-artifacts.com/output/job/3ea84c49-ca22-4dba-be5d-31004cf8d66a/artifacts/0/storage/@apollo/gateway/gateway-2.1.4.tgz", - "integrity": "sha512-zKtJ4VBCLc8uCXIwrCw/B6OF90INhkDsxh2s1eXEPnnMZp8FgZ6HWRhQLcHCNZpJBR8bCivIsCwXv0aqxJoppA==", + "version": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/gateway", + "integrity": "sha512-58/SWBfouBHKBmpQMVH+1I4RItEzmv01M2L0/TFxgiYOiFmXKznkKUrGw30Ain24BD9un4VrJHmB4TaHhlAUYw==", "dev": true, "requires": { - "@apollo/composition": "^2.1.4", - "@apollo/federation-internals": "^2.1.4", - "@apollo/query-planner": "^2.1.4", + "@apollo/composition": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/composition", + "@apollo/federation-internals": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", + "@apollo/query-planner": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-planner", "@apollo/server-gateway-interface": "https://pkg.csb.dev/apollographql/apollo-server/commit/515ae7f7/@apollo/server-gateway-interface", "@apollo/usage-reporting-protobuf": "^4.0.0", "@apollo/utils.createhash": "^1.1.0", @@ -14173,24 +14175,22 @@ } }, "@apollo/query-graphs": { - "version": "2.1.4", - "resolved": "https://registry.npmjs.org/@apollo/query-graphs/-/query-graphs-2.1.4.tgz", - "integrity": "sha512-oGDdfj+Mhh02JIU0onc0yVxb4f3jqmyHEdTveNYmnmEtFjJWPn9PGq0hpFwTquHyrzUNkx0pqDc4UIFUnZQNGg==", + "version": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-graphs", + "integrity": "sha512-l1kLisKHEC2XhqZGp3V712fWUFXEDqPqD4qxrocWOUu+C1p7NQDi2yFewKXqJrl97+Z9eajaiiprxSznOHSPvw==", "dev": true, "requires": { - "@apollo/federation-internals": "^2.1.4", + "@apollo/federation-internals": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", "deep-equal": "^2.0.5", "ts-graphviz": "^0.16.0" } }, "@apollo/query-planner": { - "version": "2.1.4", - "resolved": "https://registry.npmjs.org/@apollo/query-planner/-/query-planner-2.1.4.tgz", - "integrity": "sha512-OaC52LslJN/uGwJSdJCwJUz1e/0vo+gbHXk1deuRQiM5kr8iV8yklxeMVzy68PG9ioeA3r5SUDIZC+NuCLZeaQ==", + "version": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-planner", + "integrity": "sha512-AgtNRoISL7O9sNapVeQTqe4VNKS92UvxP0AzgDWWPd1pVudG8+gCyyZax2N2bYjiRfbLs0RONIaFpcKheBBqVg==", "dev": true, "requires": { - "@apollo/federation-internals": "^2.1.4", - "@apollo/query-graphs": "^2.1.4", + "@apollo/federation-internals": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/federation-internals", + "@apollo/query-graphs": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/query-graphs", "chalk": "^4.1.0", "deep-equal": "^2.0.5", "pretty-format": "^28.0.0" diff --git a/package.json b/package.json index c1a2f234dab..74dc9371252 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ }, "devDependencies": { "@apollo/client": "3.7.1", - "@apollo/gateway": "https://output.circle-artifacts.com/output/job/3ea84c49-ca22-4dba-be5d-31004cf8d66a/artifacts/0/storage/@apollo/gateway/gateway-2.1.4.tgz", + "@apollo/gateway": "https://pkg.csb.dev/apollographql/federation/commit/ca4c450d/@apollo/gateway", "@apollo/server-plugin-landing-page-graphql-playground": "4.0.0", "@apollo/subgraph": "2.1.4", "@apollo/utils.createhash": "1.1.0", diff --git a/packages/server/src/__tests__/plugin/usageReporting/stats.test.ts b/packages/server/src/__tests__/plugin/usageReporting/stats.test.ts index 2e0824b1aea..6801d9a1351 100644 --- a/packages/server/src/__tests__/plugin/usageReporting/stats.test.ts +++ b/packages/server/src/__tests__/plugin/usageReporting/stats.test.ts @@ -501,6 +501,7 @@ describe('Add trace to report', () => { trace: baseTrace, asTrace: false, referencedFieldsByType, + nonFtv1Errors: [], }); expect(report.tracesPerQuery['key']?.trace?.length).toBe(0); @@ -517,6 +518,7 @@ describe('Add trace to report', () => { asTrace: true, referencedFieldsByType, maxTraceBytes: 10, + nonFtv1Errors: [], }); expect(report.tracesPerQuery['key']?.trace?.length).toBe(0); @@ -533,6 +535,7 @@ describe('Add trace to report', () => { asTrace: true, referencedFieldsByType, maxTraceBytes: 500 * 1024, + nonFtv1Errors: [], }); expect(report.tracesPerQuery['key']?.trace?.length).toBe(1); @@ -548,6 +551,7 @@ describe('Add trace to report', () => { trace: baseTrace, asTrace: true, referencedFieldsByType, + nonFtv1Errors: [], }); expect(report.tracesPerQuery['key']?.trace?.length).toBe(1); diff --git a/packages/server/src/__tests__/plugin/usageReporting/statsErrors.test.ts b/packages/server/src/__tests__/plugin/usageReporting/statsErrors.test.ts index 9c693535485..bfe9863d108 100644 --- a/packages/server/src/__tests__/plugin/usageReporting/statsErrors.test.ts +++ b/packages/server/src/__tests__/plugin/usageReporting/statsErrors.test.ts @@ -79,8 +79,8 @@ describe('TODO', () => { "requestsWithoutFieldInstrumentation": 1, "rootErrorStats": { "children": {}, - "errorsCount": 2, - "requestsWithErrorsCount": 1, + "errorsCount": 0, + "requestsWithErrorsCount": 0, }, } `); @@ -90,7 +90,6 @@ describe('TODO', () => { }) .reply(200); - // const res = await gatewayServer.executeHTTPGraphQLRequest({ httpGraphQLRequest: { method: 'POST', @@ -101,8 +100,6 @@ describe('TODO', () => { context: async () => ({}), }); - // expect(res).toMatchInlineSnapshot(); - await gatewayServer.stop(); await errorFtv1Subgraph.stop(); await errorNoFtv1Subgraph.stop(); @@ -173,8 +170,8 @@ describe('TODO', () => { "requestsWithErrorsCount": 0, }, }, - "errorsCount": 1, - "requestsWithErrorsCount": 1, + "errorsCount": 0, + "requestsWithErrorsCount": 0, }, } `); @@ -211,7 +208,6 @@ describe('TODO', () => { }) .reply(200); - // const res = await gatewayServer.executeHTTPGraphQLRequest({ httpGraphQLRequest: { method: 'POST', @@ -222,8 +218,6 @@ describe('TODO', () => { context: async () => ({}), }); - // expect(res).toMatchInlineSnapshot(); - await gatewayServer.stop(); await errorFtv1Subgraph.stop(); await errorNoFtv1Subgraph.stop(); diff --git a/packages/server/src/plugin/inlineTrace/index.ts b/packages/server/src/plugin/inlineTrace/index.ts index 2f001ed8ac1..fa2de0215b8 100644 --- a/packages/server/src/plugin/inlineTrace/index.ts +++ b/packages/server/src/plugin/inlineTrace/index.ts @@ -105,7 +105,6 @@ export function ApolloServerPluginInlineTrace( }, async didEncounterErrors({ errors }) { - // FIXME: do I need to filter out errors that have a `serviceName` here as well? treeBuilder.didEncounterErrors(errors); }, diff --git a/packages/server/src/plugin/traceTreeBuilder.ts b/packages/server/src/plugin/traceTreeBuilder.ts index 62baaac03ed..4d796cbc0e0 100644 --- a/packages/server/src/plugin/traceTreeBuilder.ts +++ b/packages/server/src/plugin/traceTreeBuilder.ts @@ -105,6 +105,16 @@ export class TraceTreeBuilder { public didEncounterErrors(errors: readonly GraphQLError[]) { errors.forEach((err) => { + // This is an error from a federated service. We will already be reporting + // it in the nested Trace in the query plan. + // + // XXX This probably shouldn't skip query or validation errors, which are + // not in nested Traces because format() isn't called in this case! Or + // maybe format() should be called in that case? + if (err.extensions?.serviceName) { + return; + } + // In terms of reporting, errors can be re-written by the user by // utilizing the `transformError` parameter. This allows changing // the message or stack to remove potentially sensitive information. diff --git a/packages/server/src/plugin/usageReporting/plugin.ts b/packages/server/src/plugin/usageReporting/plugin.ts index 79e3b1bc9eb..701e38d7491 100644 --- a/packages/server/src/plugin/usageReporting/plugin.ts +++ b/packages/server/src/plugin/usageReporting/plugin.ts @@ -551,7 +551,6 @@ export function ApolloServerPluginUsageReporting( }, async didEncounterSubsequentErrors(_requestContext, errors) { - // FIXME: do I need to filter out errors that have a `serviceName` here as well? treeBuilder.didEncounterErrors(errors); }, @@ -566,18 +565,8 @@ export function ApolloServerPluginUsageReporting( // of the pre-source-resolution errors we are intentionally avoiding. if (!didResolveSource) return; - if (requestContext.metrics.nonFtv1Errors) { - treeBuilder.didEncounterErrors(requestContext.metrics.nonFtv1Errors); - } else if (requestContext.errors) { - treeBuilder.didEncounterErrors(requestContext.errors.filter(error => { - // This is an error from a federated service. We will already be reporting - // it in the nested Trace in the query plan. - // - // XXX This probably shouldn't skip query or validation errors, which are - // not in nested Traces because format() isn't called in this case! Or - // maybe format() should be called in that case? - return !error.extensions?.serviceName; - })); + if (requestContext.errors) { + treeBuilder.didEncounterErrors(requestContext.errors); } // If there isn't any defer/stream coming later, we're done. @@ -723,6 +712,7 @@ export function ApolloServerPluginUsageReporting( sendOperationAsTrace(trace, statsReportKey) && !metrics.nonFtv1Errors?.length, referencedFieldsByType, + nonFtv1Errors: metrics.nonFtv1Errors ?? [], }); // If the buffer gets big (according to our estimate), send. diff --git a/packages/server/src/plugin/usageReporting/stats.ts b/packages/server/src/plugin/usageReporting/stats.ts index 302e67eb510..c715b848aac 100644 --- a/packages/server/src/plugin/usageReporting/stats.ts +++ b/packages/server/src/plugin/usageReporting/stats.ts @@ -12,6 +12,7 @@ import { Trace, } from '@apollo/usage-reporting-protobuf'; import type { ReferencedFieldsByType } from '@apollo/utils.usagereporting'; +import type { GraphQLError } from 'graphql'; import { DurationHistogram } from './durationHistogram.js'; import { iterateOverTrace, ResponseNamePath } from './iterateOverTrace.js'; @@ -65,12 +66,14 @@ export class OurReport implements Required { // Apollo reporting ingress server will never store any traces over 10mb // anyway. They will still be converted to stats as we would do here. maxTraceBytes = 10 * 1024 * 1024, + nonFtv1Errors, }: { statsReportKey: string; trace: Trace; asTrace: boolean; referencedFieldsByType: ReferencedFieldsByType; maxTraceBytes?: number; + nonFtv1Errors: GraphQLError[]; }) { const tracesAndStats = this.getTracesAndStats({ statsReportKey, @@ -80,13 +83,21 @@ export class OurReport implements Required { const encodedTrace = Trace.encode(trace).finish(); if (!isNaN(maxTraceBytes) && encodedTrace.length > maxTraceBytes) { - tracesAndStats.statsWithContext.addTrace(trace, this.sizeEstimator); + tracesAndStats.statsWithContext.addTrace( + trace, + this.sizeEstimator, + nonFtv1Errors, + ); } else { tracesAndStats.trace.push(encodedTrace); this.sizeEstimator.bytes += 2 + encodedTrace.length; } } else { - tracesAndStats.statsWithContext.addTrace(trace, this.sizeEstimator); + tracesAndStats.statsWithContext.addTrace( + trace, + this.sizeEstimator, + nonFtv1Errors, + ); } } @@ -157,10 +168,15 @@ class StatsByContext { } } - addTrace(trace: Trace, sizeEstimator: SizeEstimator) { + addTrace( + trace: Trace, + sizeEstimator: SizeEstimator, + nonFtv1Errors: GraphQLError[], + ) { this.getContextualizedStats(trace, sizeEstimator).addTrace( trace, sizeEstimator, + nonFtv1Errors, ); } @@ -207,7 +223,11 @@ export class OurContextualizedStats implements Required { // We only add to the estimate when adding whole sub-messages. If it really // mattered, we could do a lot more careful things like incrementing it // whenever a numeric field on queryLatencyStats gets incremented over 0. - addTrace(trace: Trace, sizeEstimator: SizeEstimator) { + addTrace( + trace: Trace, + sizeEstimator: SizeEstimator, + nonFtv1Errors: GraphQLError[] = [], + ) { const { fieldExecutionWeight } = trace; if (!fieldExecutionWeight) { this.queryLatencyStats.requestsWithoutFieldInstrumentation++; @@ -258,6 +278,8 @@ export class OurContextualizedStats implements Required { let hasError = false; + const errorPathStats = new Set(); + const traceNodeStats = (node: Trace.INode, path: ResponseNamePath) => { // Generate error stats and error path information if (node.error?.length) { @@ -271,7 +293,7 @@ export class OurContextualizedStats implements Required { ); }); - currPathErrorStats.requestsWithErrorsCount += 1; + errorPathStats.add(currPathErrorStats); currPathErrorStats.errorsCount += node.error.length; } @@ -331,6 +353,30 @@ export class OurContextualizedStats implements Required { }; iterateOverTrace(trace, traceNodeStats, true); + + // iterate over nonFtv1Errors, using some bits from traceNodeStats function + for (const error of nonFtv1Errors) { + hasError = true; + if (error.path) { + let currPathErrorStats = this.queryLatencyStats.rootErrorStats; + error.path.forEach((subPath) => { + if (typeof subPath === 'string') { + currPathErrorStats = currPathErrorStats.getChild( + subPath, + sizeEstimator, + ); + } + }); + + errorPathStats.add(currPathErrorStats); + currPathErrorStats.errorsCount += 1; + } + } + + for (const errorPath of errorPathStats) { + errorPath.requestsWithErrorsCount += 1; + } + if (hasError) { this.queryLatencyStats.requestsWithErrorsCount++; }