From 18913448ec0700564d040034a24d24a6381ef33e Mon Sep 17 00:00:00 2001 From: Tim Hingston Date: Fri, 6 Sep 2024 14:00:35 +1000 Subject: [PATCH 01/19] Add docs for enhanced OTel tracing in Studio --- docs/source/configuration/overview.mdx | 14 ++++ .../telemetry/apollo-telemetry.mdx | 64 +++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/docs/source/configuration/overview.mdx b/docs/source/configuration/overview.mdx index 13e6991a6a..c96db9d4fc 100644 --- a/docs/source/configuration/overview.mdx +++ b/docs/source/configuration/overview.mdx @@ -865,6 +865,20 @@ You won't see an immediate change in checks behavior when you first turn on exte + + +### Enhanced tracing in Studio via OpenTelemetry + + + + + +Beginning in v1.49.0, the router supports sending traces to Studio via the more detailed OTel (OpenTelemetry) protocol. +Support for OTel traces has historically only been available for 3rd party APM tools. With this option, +Studio can now provide a much more granular view of Router internals than the legacy Apollo tracing protocol. + +See [Enhanced tracing in Studio via OTel](./telemetry/apollo-telemetry#enhanced-tracing-in-studio-via-opentelemetry). + ### Safelisting with persisted queries You can enhance your graph's security with GraphOS Router by maintaining a persisted query list (PQL), an operation safelist made by your first-party apps. As opposed to automatic persisted queries (APQ) where operations are automatically cached, operations must be preregistered to the PQL. Once configured, the router checks incoming requests against the PQL. diff --git a/docs/source/configuration/telemetry/apollo-telemetry.mdx b/docs/source/configuration/telemetry/apollo-telemetry.mdx index c5f6c0c609..d4e276118e 100644 --- a/docs/source/configuration/telemetry/apollo-telemetry.mdx +++ b/docs/source/configuration/telemetry/apollo-telemetry.mdx @@ -72,6 +72,70 @@ telemetry: field_level_instrumentation_sampler: always_off ``` + + +### Enhanced tracing in Studio via OpenTelemetry + + + + + +Beginning in v1.49.0, the router supports sending traces to Studio via the more detailed OTel (OpenTelemetry) protocol. +Support for OTel traces has historically only been available for 3rd party APM tools. With this option, +Studio can now provide a much more granular view of Router internals than the legacy Apollo tracing protocol. + +Benefits include: + +- A comprehensive way to visualize the Router execution path in Studio. +- Additional spans that were previously not included in Studio traces, such as query parsing, planning, execution, and more. +- Additional attributes including HTTP request details, REST connector details, and more. + +It is expected that this will become the default in a future version of Router. + +#### Configuration + +This change adds a new configuration option `telemetry.apollo.experimental_otlp_tracing_sampler`. Use this option to send +a percentage of traces to Studio via OTLP instead of the native Apollo Usage Reporting protocol. Supported values: + +- `always_off` (default): send all traces via the legacy Apollo Usage Reporting protocol. +- `always_on`: send all traces via OTLP. +- `0.0 - 1.0` (used for testing): the ratio of traces to send via OTLP (0.5 = 50 / 50). + +Note that this sampler is only applied _after_ the common tracing sampler, for example: + +#### Sample 1% of traces, send all traces via OTLP: + +```yaml +telemetry: + apollo: + # Send all traces via OTLP + experimental_otlp_tracing_sampler: always_on + + exporters: + tracing: + common: + # Sample traces at 1% of all traffic + sampler: 0.01 +``` + +OTel traces sent to Studio will not necessarily be identical to the ones sent to 3rd Party APM tools via OTLP: + +- Only specific OTLP attributes will be included for parity with what is provided in legacy traces today. This ensures that data privacy + is maintained in an equivalent manner. The existing Router configuration options for Apollo telemetry will continue to function + with OTLP traces, such as forwarding of GraphQL errors, headers, and variables. +- Some features of OTLP traces may only be available in Studio and not in 3rd Party APM tools (e.g. resolver-level timing information from + [Federated Tracing](../../federation/metrics/#enabling-federated-tracing)). + + + +This change results in using a new wire protocol for traces, and some users may experience an increase in tracing traffic +to GraphOS Studio due to the additional detail being captured. In exceptional situations it may be necessary to send fewer traces. +This can be achieved via sending fewer traces (`telemetry.exporters.tracing.common.sampler`) or as a last resort, falling back +to the old protocol via `telemetry.apollo.otlp_tracing_sampler` to send fewer OTLP traces or fully disable them. +Any performance regressions due to the new tracing protocol should also be reported to the Apollo support team. + + + ### Experimental local field metrics Apollo Router can send field-level metrics to GraphOS without using FTV1 tracing. This feature is experimental and is not yet displayable in GraphOS Studio. From abe6daeebe0a82f8bf390aa692626dc8f52db90e Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Wed, 23 Oct 2024 17:37:59 +0300 Subject: [PATCH 02/19] docs: use equals in log specifier (#6034) --- .../configuration/telemetry/exporters/logging/overview.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/configuration/telemetry/exporters/logging/overview.mdx b/docs/source/configuration/telemetry/exporters/logging/overview.mdx index 216375d71e..3f763ac437 100644 --- a/docs/source/configuration/telemetry/exporters/logging/overview.mdx +++ b/docs/source/configuration/telemetry/exporters/logging/overview.mdx @@ -59,7 +59,7 @@ APOLLO_ROUTER_LOG=debug For another example, every line below sets the same log levels: ``` -RUST_LOG=hyper=debug,apollo_router::info,h2=trace +RUST_LOG=hyper=debug,apollo_router=info,h2=trace APOLLO_ROUTER_LOG=hyper=debug,info,h2=trace --log=hyper=debug,info,h2=trace ``` From b75689cf8e649cca268771b20688997a5b187d9e Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Thu, 24 Oct 2024 22:48:50 +0300 Subject: [PATCH 03/19] Docs: Update Dynatrace Metrics documentation (#6179) --- .../telemetry/exporters/metrics/dynatrace.mdx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/source/configuration/telemetry/exporters/metrics/dynatrace.mdx b/docs/source/configuration/telemetry/exporters/metrics/dynatrace.mdx index 41942dbb38..dfd00f1c8d 100644 --- a/docs/source/configuration/telemetry/exporters/metrics/dynatrace.mdx +++ b/docs/source/configuration/telemetry/exporters/metrics/dynatrace.mdx @@ -13,9 +13,12 @@ For general tracing configuration, refer to [Router Metrics Configuration](./ove To configure the router: - Enable the [OTLP exporter](./otlp#configuration) -- Set the `protocol` as `http`; Dynatrace [doesn't currently support](https://docs.dynatrace.com/docs/extend-dynatrace/opentelemetry/getting-started/otlp-export) `grpc` -- Provide your Dynatrace endpoint -- Provide your Dynatrace API token in the `Authorization` header; the header should start with [`Api-token` and then your Dynatrace token](https://docs.dynatrace.com/docs/extend-dynatrace/opentelemetry/getting-started/otlp-export#authentication-export-to-activegate) +- Set `temporality: delta` (Using _Delta_ is required as _Cumulative_ temporality is **not** supported by Dynatrace) +- Set the `protocol` as `http` (Dynatrace [doesn't currently support](https://docs.dynatrace.com/docs/extend-dynatrace/opentelemetry/getting-started/otlp-export) gRPC) +- Set the `endpoint` to your [Dynatrace OpenTelemetry metrics endpoint](https://docs.dynatrace.com/docs/dynatrace-api/environment-api/opentelemetry/post-metrics) (e.g., ensuring that it contains `{your-environment-id}` in the hostname and ends in `/api/v2/otlp/v1/metrics`) +- Provide your Dynatrace API token in the `Authorization` header (the header should start with [`Api-Token` and then your Dynatrace token](https://docs.dynatrace.com/docs/extend-dynatrace/opentelemetry/getting-started/otlp-export#authentication-export-to-activegate) + +For example: ```yaml title="router.yaml" telemetry: @@ -23,6 +26,7 @@ telemetry: metrics: otlp: enabled: true + temporality: delta # Endpoint for your region. endpoint: protocol: http @@ -33,7 +37,8 @@ telemetry: -You must specify `protocol: http` or the exporter will fail to connect to Dynatrace. Additionally, if your Dynatrace endpoint does not contain a port, you must append `:443` to the endpoint. For example: `https://subdomain.live.dynatrace.com:443/api/v2/otlp/v1/traces`. +You must specify `protocol: http` or the exporter will fail to connect to Dynatrace. You must use `temporality: delta` or some metrics will fail to be delivered to your deployment. + Additionally, if your Dynatrace endpoint does not contain a port, you **must** explicitly include `:443` as the port after the host address. For example: `https://subdomain.live.dynatrace.com:443/api/v2/otlp/v1/metrics`. From 336c42d401611117da503b1c96c5a541648c11eb Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 29 Oct 2024 11:48:07 +0100 Subject: [PATCH 04/19] defer integration tests (#6007) --- apollo-router/tests/common.rs | 1 - .../{basic/query1 => core/defer}/README.md | 0 .../query1 => core/defer}/configuration.yaml | 0 .../tests/samples/core/defer/plan.json | 106 +++++++++++++++ .../query2 => core/defer}/supergraph.graphql | 0 .../{basic/query2 => core/query1}/README.md | 0 .../query2 => core/query1}/configuration.yaml | 0 .../samples/{basic => core}/query1/plan.json | 0 .../{basic => core}/query1/supergraph.graphql | 0 .../tests/samples/core/query2/README.md | 3 + .../samples/core/query2/configuration.yaml | 4 + .../samples/{basic => core}/query2/plan.json | 0 .../samples/core/query2/supergraph.graphql | 125 ++++++++++++++++++ .../enterprise/entity-cache/defer/README.md | 3 + .../entity-cache/defer/configuration.yaml | 23 ++++ .../enterprise/entity-cache/defer/plan.json | 113 ++++++++++++++++ .../entity-cache/defer/supergraph.graphql | 122 +++++++++++++++++ apollo-router/tests/samples_tests.rs | 115 ++++++++++++++-- 18 files changed, 605 insertions(+), 10 deletions(-) rename apollo-router/tests/samples/{basic/query1 => core/defer}/README.md (100%) rename apollo-router/tests/samples/{basic/query1 => core/defer}/configuration.yaml (100%) create mode 100644 apollo-router/tests/samples/core/defer/plan.json rename apollo-router/tests/samples/{basic/query2 => core/defer}/supergraph.graphql (100%) rename apollo-router/tests/samples/{basic/query2 => core/query1}/README.md (100%) rename apollo-router/tests/samples/{basic/query2 => core/query1}/configuration.yaml (100%) rename apollo-router/tests/samples/{basic => core}/query1/plan.json (100%) rename apollo-router/tests/samples/{basic => core}/query1/supergraph.graphql (100%) create mode 100644 apollo-router/tests/samples/core/query2/README.md create mode 100644 apollo-router/tests/samples/core/query2/configuration.yaml rename apollo-router/tests/samples/{basic => core}/query2/plan.json (100%) create mode 100644 apollo-router/tests/samples/core/query2/supergraph.graphql create mode 100644 apollo-router/tests/samples/enterprise/entity-cache/defer/README.md create mode 100644 apollo-router/tests/samples/enterprise/entity-cache/defer/configuration.yaml create mode 100644 apollo-router/tests/samples/enterprise/entity-cache/defer/plan.json create mode 100644 apollo-router/tests/samples/enterprise/entity-cache/defer/supergraph.graphql diff --git a/apollo-router/tests/common.rs b/apollo-router/tests/common.rs index 826a377e04..671c462519 100644 --- a/apollo-router/tests/common.rs +++ b/apollo-router/tests/common.rs @@ -587,7 +587,6 @@ impl IntegrationTest { let mut request = builder.json(&query).build().unwrap(); telemetry.inject_context(&mut request); - request.headers_mut().remove(ACCEPT); match client.execute(request).await { Ok(response) => (span_id, response), Err(err) => { diff --git a/apollo-router/tests/samples/basic/query1/README.md b/apollo-router/tests/samples/core/defer/README.md similarity index 100% rename from apollo-router/tests/samples/basic/query1/README.md rename to apollo-router/tests/samples/core/defer/README.md diff --git a/apollo-router/tests/samples/basic/query1/configuration.yaml b/apollo-router/tests/samples/core/defer/configuration.yaml similarity index 100% rename from apollo-router/tests/samples/basic/query1/configuration.yaml rename to apollo-router/tests/samples/core/defer/configuration.yaml diff --git a/apollo-router/tests/samples/core/defer/plan.json b/apollo-router/tests/samples/core/defer/plan.json new file mode 100644 index 0000000000..b2499400bd --- /dev/null +++ b/apollo-router/tests/samples/core/defer/plan.json @@ -0,0 +1,106 @@ +{ + "actions": [ + { + "type": "Start", + "schema_path": "./supergraph.graphql", + "configuration_path": "./configuration.yaml", + "subgraphs": { + "accounts": { + "requests": [ + { + "request": { + "body": { + "query": "{me{__typename name id}}" + } + }, + "response": { + "body": { + "data": { + "me": { + "__typename": "User", + "name": "test", + "id": "1" + } + } + } + } + } + ] + }, + "reviews": { + "requests": [ + { + "request": { + "body": { + "query": "query($representations:[_Any!]!){_entities(representations:$representations){...on User{reviews{body}}}}", + "variables": { + "representations": [ + { + "__typename": "User", + "id": "1" + } + ] + } + } + }, + "response": { + "body": { + "data": { + "_entities": [ + { + "reviews": [ + { + "body": "Test" + } + ] + } + ] + } + } + } + } + ] + } + } + }, + { + "type": "Request", + "headers": { + "Accept": "multipart/mixed;deferSpec=20220824" + }, + "request": { + "query": "{ me { name ... @defer { reviews { body } } } }" + }, + "expected_response": [ + { + "data": { + "me": { + "name": "test" + } + }, + "hasNext": true + }, + { + "hasNext": false, + "incremental": [ + { + "data": { + "reviews": [ + { + "body": "Test" + } + ] + }, + "path": [ + "me" + ] + } + ] + } + ] + }, + { + "type": "Stop" + } + ] +} \ No newline at end of file diff --git a/apollo-router/tests/samples/basic/query2/supergraph.graphql b/apollo-router/tests/samples/core/defer/supergraph.graphql similarity index 100% rename from apollo-router/tests/samples/basic/query2/supergraph.graphql rename to apollo-router/tests/samples/core/defer/supergraph.graphql diff --git a/apollo-router/tests/samples/basic/query2/README.md b/apollo-router/tests/samples/core/query1/README.md similarity index 100% rename from apollo-router/tests/samples/basic/query2/README.md rename to apollo-router/tests/samples/core/query1/README.md diff --git a/apollo-router/tests/samples/basic/query2/configuration.yaml b/apollo-router/tests/samples/core/query1/configuration.yaml similarity index 100% rename from apollo-router/tests/samples/basic/query2/configuration.yaml rename to apollo-router/tests/samples/core/query1/configuration.yaml diff --git a/apollo-router/tests/samples/basic/query1/plan.json b/apollo-router/tests/samples/core/query1/plan.json similarity index 100% rename from apollo-router/tests/samples/basic/query1/plan.json rename to apollo-router/tests/samples/core/query1/plan.json diff --git a/apollo-router/tests/samples/basic/query1/supergraph.graphql b/apollo-router/tests/samples/core/query1/supergraph.graphql similarity index 100% rename from apollo-router/tests/samples/basic/query1/supergraph.graphql rename to apollo-router/tests/samples/core/query1/supergraph.graphql diff --git a/apollo-router/tests/samples/core/query2/README.md b/apollo-router/tests/samples/core/query2/README.md new file mode 100644 index 0000000000..9386489fb0 --- /dev/null +++ b/apollo-router/tests/samples/core/query2/README.md @@ -0,0 +1,3 @@ +This is an example test + +This file adds some context that will be displayed on test failure \ No newline at end of file diff --git a/apollo-router/tests/samples/core/query2/configuration.yaml b/apollo-router/tests/samples/core/query2/configuration.yaml new file mode 100644 index 0000000000..f7ed04641e --- /dev/null +++ b/apollo-router/tests/samples/core/query2/configuration.yaml @@ -0,0 +1,4 @@ +override_subgraph_url: + products: http://localhost:4005 +include_subgraph_errors: + all: true diff --git a/apollo-router/tests/samples/basic/query2/plan.json b/apollo-router/tests/samples/core/query2/plan.json similarity index 100% rename from apollo-router/tests/samples/basic/query2/plan.json rename to apollo-router/tests/samples/core/query2/plan.json diff --git a/apollo-router/tests/samples/core/query2/supergraph.graphql b/apollo-router/tests/samples/core/query2/supergraph.graphql new file mode 100644 index 0000000000..1bd9f596ee --- /dev/null +++ b/apollo-router/tests/samples/core/query2/supergraph.graphql @@ -0,0 +1,125 @@ +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/inaccessible/v0.2", for: SECURITY) + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) { + query: Query + mutation: Mutation +} + +directive @inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + +directive @tag( + name: String! +) repeatable on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION | SCHEMA + +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field( + graph: join__Graph + requires: join__FieldSet + provides: join__FieldSet + type: String + external: Boolean + override: String + usedOverridden: Boolean +) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements( + graph: join__Graph! + interface: String! +) repeatable on OBJECT | INTERFACE + +directive @join__type( + graph: join__Graph! + key: join__FieldSet + extension: Boolean! = false + resolvable: Boolean! = true + isInterfaceObject: Boolean! = false +) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember( + graph: join__Graph! + member: String! +) repeatable on UNION + +directive @link( + url: String + as: String + for: link__Purpose + import: [link__Import] +) repeatable on SCHEMA + +scalar join__FieldSet + +enum join__Graph { + ACCOUNTS + @join__graph(name: "accounts", url: "https://accounts.demo.starstuff.dev/") + INVENTORY + @join__graph( + name: "inventory" + url: "https://inventory.demo.starstuff.dev/" + ) + PRODUCTS + @join__graph(name: "products", url: "https://products.demo.starstuff.dev/") + REVIEWS + @join__graph(name: "reviews", url: "https://reviews.demo.starstuff.dev/") +} + +scalar link__Import + +enum link__Purpose { + SECURITY + EXECUTION +} + +type Mutation @join__type(graph: PRODUCTS) @join__type(graph: REVIEWS) { + createProduct(upc: ID!, name: String): Product @join__field(graph: PRODUCTS) + createReview(upc: ID!, id: ID!, body: String): Review + @join__field(graph: REVIEWS) +} + +type Product + @join__type(graph: INVENTORY, key: "upc") + @join__type(graph: PRODUCTS, key: "upc") + @join__type(graph: REVIEWS, key: "upc") { + inStock: Boolean + @join__field(graph: INVENTORY) + @tag(name: "private") + @inaccessible + name: String @join__field(graph: PRODUCTS) + weight: Int @join__field(graph: INVENTORY, external: true) @join__field(graph: PRODUCTS) + price: Int @join__field(graph: INVENTORY, external: true) @join__field(graph: PRODUCTS) + reviews: [Review] @join__field(graph: REVIEWS) + reviewsForAuthor(authorID: ID!): [Review] @join__field(graph: REVIEWS) + shippingEstimate: Int @join__field(graph: INVENTORY, requires: "price weight") + upc: String! +} + +type Query + @join__type(graph: ACCOUNTS) + @join__type(graph: INVENTORY) + @join__type(graph: PRODUCTS) + @join__type(graph: REVIEWS) { + me: User @join__field(graph: ACCOUNTS) + topProducts(first: Int = 5): [Product] @join__field(graph: PRODUCTS) +} + +type Review @join__type(graph: REVIEWS, key: "id") { + id: ID! + body: String @join__field(graph: REVIEWS) + author: User @join__field(graph: REVIEWS, provides: "username") + product: Product @join__field(graph: REVIEWS) +} + +type User + @join__type(graph: ACCOUNTS, key: "id") + @join__type(graph: REVIEWS, key: "id") { + id: ID! + name: String @join__field(graph: ACCOUNTS) + username: String + @join__field(graph: ACCOUNTS) + @join__field(graph: REVIEWS, external: true) + reviews: [Review] @join__field(graph: REVIEWS) +} diff --git a/apollo-router/tests/samples/enterprise/entity-cache/defer/README.md b/apollo-router/tests/samples/enterprise/entity-cache/defer/README.md new file mode 100644 index 0000000000..a96d350b73 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/entity-cache/defer/README.md @@ -0,0 +1,3 @@ +# Entity cache with @defer + +This tests `Cache-Control` aggregation when using the `@defer` directive. \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/entity-cache/defer/configuration.yaml b/apollo-router/tests/samples/enterprise/entity-cache/defer/configuration.yaml new file mode 100644 index 0000000000..fb6b95ecd4 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/entity-cache/defer/configuration.yaml @@ -0,0 +1,23 @@ +override_subgraph_url: + products: http://localhost:4005 +include_subgraph_errors: + all: true + +preview_entity_cache: + enabled: true + redis: + urls: + ["redis://localhost:6379",] + subgraph: + all: + enabled: true + subgraphs: + reviews: + ttl: 120s + enabled: true + +telemetry: + exporters: + logging: + stdout: + format: text \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/entity-cache/defer/plan.json b/apollo-router/tests/samples/enterprise/entity-cache/defer/plan.json new file mode 100644 index 0000000000..265e282056 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/entity-cache/defer/plan.json @@ -0,0 +1,113 @@ +{ + "enterprise": true, + "redis": true, + "actions": [ + { + "type": "Start", + "schema_path": "./supergraph.graphql", + "configuration_path": "./configuration.yaml", + "subgraphs": { + "cache-defer-accounts": { + "requests": [ + { + "request": { + "body": { + "query": "query CacheDefer__cache_defer_accounts__0{me{__typename name id}}", + "operationName": "CacheDefer__cache_defer_accounts__0" + } + }, + "response": { + "headers": { + "Cache-Control": "public, max-age=10", + "Content-Type": "application/json" + }, + "body": { + "data": { + "me": { + "__typename": "User", + "name": "test-user", + "id": "1" + } + } + } + } + } + ] + }, + "cache-defer-reviews": { + "requests": [ + { + "request": { + "body": { + "query": "query CacheDefer__cache_defer_reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on User{reviews{body}}}}", + "operationName": "CacheDefer__cache_defer_reviews__1", + "variables": { + "representations": [ + { + "id": "1", + "__typename": "User" + } + ] + } + } + }, + "response": { + "headers": { + "Cache-Control": "public, max-age=100", + "Content-Type": "application/json" + }, + "body": { + "data": { + "reviews": [ + { + "body": "test-review" + } + ] + } + } + } + } + ] + } + } + }, + { + "type": "Request", + "request": { + "query": "query CacheDefer { me { name ... @defer { reviews { body } } } }" + }, + "headers": { + "Accept": "multipart/mixed;deferSpec=20220824" + }, + "expected_response": [ + { + "data": { + "me": { + "name": "test-user" + } + }, + "hasNext": true + }, + { + "hasNext": false, + "incremental": [ + { + "data": { + "reviews": null + }, + "path": [ + "me" + ] + } + ] + } + ], + "expected_headers": { + "Cache-Control": "max-age=10,public" + } + }, + { + "type": "Stop" + } + ] +} \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/entity-cache/defer/supergraph.graphql b/apollo-router/tests/samples/enterprise/entity-cache/defer/supergraph.graphql new file mode 100644 index 0000000000..320a9c2a70 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/entity-cache/defer/supergraph.graphql @@ -0,0 +1,122 @@ +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/tag/v0.3") + @link(url: "https://specs.apollo.dev/inaccessible/v0.2") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) { + query: Query + mutation: Mutation +} + +directive @inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION +directive @tag( + name: String! +) repeatable on FIELD_DEFINITION | INTERFACE | OBJECT | UNION + +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field( + graph: join__Graph + requires: join__FieldSet + provides: join__FieldSet + type: String + external: Boolean + override: String + usedOverridden: Boolean +) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements( + graph: join__Graph! + interface: String! +) repeatable on OBJECT | INTERFACE + +directive @join__type( + graph: join__Graph! + key: join__FieldSet + extension: Boolean! = false + resolvable: Boolean! = true + isInterfaceObject: Boolean! = false +) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember( + graph: join__Graph! + member: String! +) repeatable on UNION + +directive @link( + url: String + as: String + for: link__Purpose + import: [link__Import] +) repeatable on SCHEMA + +scalar join__FieldSet +scalar link__Import + +enum join__Graph { + ACCOUNTS @join__graph(name: "cache-defer-accounts", url: "https://accounts.demo.starstuff.dev") + INVENTORY @join__graph(name: "inventory", url: "https://inventory.demo.starstuff.dev") + PRODUCTS @join__graph(name: "products", url: "https://products.demo.starstuff.dev") + REVIEWS @join__graph(name: "cache-defer-reviews", url: "https://reviews.demo.starstuff.dev") +} + +enum link__Purpose { + SECURITY + EXECUTION +} + +type Mutation + @join__type(graph: PRODUCTS) + @join__type(graph: REVIEWS) + @join__type(graph: ACCOUNTS) { + updateMyAccount: User @join__field(graph: ACCOUNTS) + createProduct(name: String, upc: ID!): Product @join__field(graph: PRODUCTS) + createReview(body: String, id: ID!, upc: ID!): Review + @join__field(graph: REVIEWS) +} + +type Product + @join__type(graph: ACCOUNTS, key: "upc", extension: true) + @join__type(graph: INVENTORY, key: "upc") + @join__type(graph: PRODUCTS, key: "upc") + @join__type(graph: REVIEWS, key: "upc") { + inStock: Boolean + @join__field(graph: INVENTORY) + @tag(name: "private") + @inaccessible + name: String @join__field(graph: PRODUCTS) + weight: Int @join__field(graph: INVENTORY, external: true) @join__field(graph: PRODUCTS) + price: Int @join__field(graph: INVENTORY, external: true) @join__field(graph: PRODUCTS) + reviews: [Review] @join__field(graph: REVIEWS) + reviewsForAuthor(authorID: ID!): [Review] @join__field(graph: REVIEWS) + shippingEstimate: Int @join__field(graph: INVENTORY, requires: "price weight") + upc: String! +} + +type Query + @join__type(graph: ACCOUNTS) + @join__type(graph: INVENTORY) + @join__type(graph: PRODUCTS) + @join__type(graph: REVIEWS) { + me: User @join__field(graph: ACCOUNTS) + topProducts(first: Int = 5): [Product] @join__field(graph: PRODUCTS) +} + +type Review @join__type(graph: REVIEWS, key: "id") { + id: ID! + author: User @join__field(graph: REVIEWS, provides: "username") + body: String + product: Product +} + +type User + @join__type(graph: ACCOUNTS, key: "id") + @join__type(graph: REVIEWS, key: "id") { + id: ID! + name: String @join__field(graph: ACCOUNTS) + username: String + @join__field(graph: ACCOUNTS) + @join__field(graph: REVIEWS, external: true) + reviews: [Review] @join__field(graph: REVIEWS) +} diff --git a/apollo-router/tests/samples_tests.rs b/apollo-router/tests/samples_tests.rs index 22e3b31e18..4686bd644c 100644 --- a/apollo-router/tests/samples_tests.rs +++ b/apollo-router/tests/samples_tests.rs @@ -14,6 +14,9 @@ use std::process::ExitCode; use libtest_mimic::Arguments; use libtest_mimic::Failed; use libtest_mimic::Trial; +use mediatype::MediaTypeList; +use mediatype::ReadParams; +use multer::Multipart; use serde::Deserialize; use serde_json::Value; use tokio::runtime::Runtime; @@ -173,12 +176,14 @@ impl TestExecution { query_path, headers, expected_response, + expected_headers, } => { self.request( request.clone(), query_path.as_deref(), headers, expected_response, + expected_headers, path, out, ) @@ -405,12 +410,14 @@ impl TestExecution { } } + #[allow(clippy::too_many_arguments)] async fn request( &mut self, mut request: Value, query_path: Option<&str>, headers: &HashMap, expected_response: &Value, + expected_headers: &HashMap, path: &Path, out: &mut String, ) -> Result<(), Failed> { @@ -434,19 +441,107 @@ impl TestExecution { } writeln!(out, "query: {}\n", serde_json::to_string(&request).unwrap()).unwrap(); + writeln!(out, "header: {:?}\n", headers).unwrap(); + let (_, response) = router .execute_query_with_headers(&request, headers.clone()) .await; - let body = response.bytes().await.map_err(|e| { - writeln!(out, "could not get graphql response data: {e}").unwrap(); - let f: Failed = out.clone().into(); - f - })?; - let graphql_response: Value = serde_json::from_slice(&body).map_err(|e| { - writeln!(out, "could not deserialize graphql response data: {e}").unwrap(); + writeln!(out, "response headers: {:?}", response.headers()).unwrap(); + + let mut failed = false; + for (key, value) in expected_headers { + if !response.headers().contains_key(key) { + failed = true; + writeln!(out, "expected header {} to be present", key).unwrap(); + } else if response.headers().get(key).unwrap() != value { + failed = true; + writeln!( + out, + "expected header {} to be {}, got {:?}", + key, + value, + response.headers().get(key).unwrap() + ) + .unwrap(); + } + } + if failed { let f: Failed = out.clone().into(); - f - })?; + return Err(f); + } + + let content_type = response + .headers() + .get("content-type") + .unwrap() + .to_str() + .unwrap(); + let mut is_multipart = false; + let mut boundary = None; + for mime in MediaTypeList::new(content_type).flatten() { + if mime.ty == mediatype::names::MULTIPART && mime.subty == mediatype::names::MIXED { + is_multipart = true; + boundary = mime.get_param(mediatype::names::BOUNDARY).map(|v| { + // multer does not strip quotes from the boundary: https://github.com/rwf2/multer/issues/64 + let mut s = v.as_str(); + if s.starts_with('\"') && s.ends_with('\"') { + s = &s[1..s.len() - 1]; + } + + s.to_string() + }); + } + } + + let graphql_response: Value = if !is_multipart { + let body = response.bytes().await.map_err(|e| { + writeln!(out, "could not get graphql response data: {e}").unwrap(); + let f: Failed = out.clone().into(); + f + })?; + serde_json::from_slice(&body).map_err(|e| { + writeln!( + out, + "could not deserialize graphql response data: {e}\nfrom:\n{}", + std::str::from_utf8(&body).unwrap() + ) + .unwrap(); + let f: Failed = out.clone().into(); + f + })? + } else { + let mut chunks = Vec::new(); + + let mut multipart = Multipart::new(response.bytes_stream(), boundary.unwrap()); + + // Iterate over the fields, use `next_field()` to get the next field. + while let Some(mut field) = multipart.next_field().await.map_err(|e| { + writeln!(out, "could not get next field from multipart body: {e}",).unwrap(); + let f: Failed = out.clone().into(); + f + })? { + while let Some(chunk) = field.chunk().await.map_err(|e| { + writeln!(out, "could not get next chunk from multipart body: {e}",).unwrap(); + let f: Failed = out.clone().into(); + f + })? { + writeln!(out, "multipart chunk: {:?}\n", std::str::from_utf8(&chunk)).unwrap(); + + let parsed: Value = serde_json::from_slice(&chunk).map_err(|e| { + writeln!( + out, + "could not deserialize graphql response data: {e}\nfrom:\n{}", + std::str::from_utf8(&chunk).unwrap() + ) + .unwrap(); + let f: Failed = out.clone().into(); + f + })?; + chunks.push(parsed); + } + } + Value::Array(chunks) + }; if expected_response != &graphql_response { if let Some(requests) = self @@ -583,6 +678,8 @@ enum Action { #[serde(default)] headers: HashMap, expected_response: Value, + #[serde(default)] + expected_headers: HashMap, }, EndpointRequest { url: url::Url, From 508ed0f10f2386abc51d000ccf70361a7ad2a8a0 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 29 Oct 2024 11:48:40 +0100 Subject: [PATCH 05/19] test interfaceObject (#6072) --- .../samples/basic/interface-object/README.md | 4 + .../basic/interface-object/configuration.yaml | 7 + .../samples/basic/interface-object/plan.json | 254 ++++++++++++++++++ .../basic/interface-object/supergraph.graphql | 115 ++++++++ 4 files changed, 380 insertions(+) create mode 100644 apollo-router/tests/samples/basic/interface-object/README.md create mode 100644 apollo-router/tests/samples/basic/interface-object/configuration.yaml create mode 100644 apollo-router/tests/samples/basic/interface-object/plan.json create mode 100644 apollo-router/tests/samples/basic/interface-object/supergraph.graphql diff --git a/apollo-router/tests/samples/basic/interface-object/README.md b/apollo-router/tests/samples/basic/interface-object/README.md new file mode 100644 index 0000000000..a2e1d7fbb1 --- /dev/null +++ b/apollo-router/tests/samples/basic/interface-object/README.md @@ -0,0 +1,4 @@ + +# @interfaceObject + +Test the [`@interfaceObject`](https://www.apollographql.com/docs/federation/entities/interfaces/) directive. \ No newline at end of file diff --git a/apollo-router/tests/samples/basic/interface-object/configuration.yaml b/apollo-router/tests/samples/basic/interface-object/configuration.yaml new file mode 100644 index 0000000000..d5c60afffa --- /dev/null +++ b/apollo-router/tests/samples/basic/interface-object/configuration.yaml @@ -0,0 +1,7 @@ +override_subgraph_url: + products: http://localhost:4005 +include_subgraph_errors: + all: true + +plugins: + experimental.expose_query_plan: true \ No newline at end of file diff --git a/apollo-router/tests/samples/basic/interface-object/plan.json b/apollo-router/tests/samples/basic/interface-object/plan.json new file mode 100644 index 0000000000..91a5690a0c --- /dev/null +++ b/apollo-router/tests/samples/basic/interface-object/plan.json @@ -0,0 +1,254 @@ +{ + "actions": [ + { + "type": "Start", + "schema_path": "./supergraph.graphql", + "configuration_path": "./configuration.yaml", + "subgraphs": { + "accounts": { + "requests": [ + { + "request": { + "body": { + "query": "query TestItf__accounts__0{i{__typename id x ...on A{a}...on B{b}}}", + "operationName": "TestItf__accounts__0" + } + }, + "response": { + "body": { + "data": { + "i": [ + { + "__typename": "A", + "id": "1", + "x": 1, + "a": "a" + }, + null, + { + "__typename": "B", + "id": "2", + "x": 2, + "b": "b" + }, + { + "__typename": "A", + "id": "1", + "x": 1, + "a": "a" + }, + { + "__typename": "B", + "id": "3", + "x": 3, + "b": "c" + } + ] + } + } + } + } + ] + }, + "products": { + "requests": [ + { + "request": { + "body": { + "query": "query TestItf__products__1($representations:[_Any!]!){_entities(representations:$representations){...on I{y}}}", + "operationName": "TestItf__products__1", + "variables": { + "representations": [ + { + "__typename": "I", + "id": "1" + }, + { + "__typename": "I", + "id": "2" + }, + { + "__typename": "I", + "id": "3" + } + ] + } + } + }, + "response": { + "body": { + "data": { + "_entities": [ + { + "y": 1 + }, + { + "y": 2 + }, + null + ] + } + } + } + } + ] + }, + "reviews": { + "requests": [] + } + } + }, + { + "type": "Request", + "request": { + "query": "query TestItf { i { __typename x y ... on A { a } ... on B { b } } }" + }, + "expected_response": { + "data": { + "i": [ + { + "__typename": "A", + "x": 1, + "y": 1, + "a": "a" + }, + null, + { + "__typename": "B", + "x": 2, + "y": 2, + "b": "b" + }, + { + "__typename": "A", + "x": 1, + "y": 1, + "a": "a" + }, + { + "__typename": "B", + "x": 3, + "y": null, + "b": "c" + } + ] + } + } + }, + { + "type": "ReloadSubgraphs", + "subgraphs": { + "accounts": { + "requests": [ + { + "request": { + "body": { + "query": "query TestItf2__accounts__0{req{__typename id i{__typename id x}}}", + "operationName": "TestItf2__accounts__0" + } + }, + "response": { + "body": { + "data": { + "req": { + "__typename": "C", + "id": "1", + "i": { + "__typename": "A", + "id": "1", + "x": 1 + } + } + } + } + } + } + ] + }, + "products": { + "requests": [ + { + "request": { + "body": { + "query": "query TestItf2__products__1($representations:[_Any!]!){_entities(representations:$representations){...on I{y}}}", + "operationName": "TestItf2__products__1", + "variables": { + "representations": [ + { + "__typename": "I", + "id": "1" + } + ] + } + } + }, + "response": { + "body": { + "data": { + "_entities": [ + { + "y": 1 + } + ] + } + } + } + } + ] + }, + "reviews": { + "requests": [ + { + "request": { + "body": { + "query": "query TestItf2__reviews__2($representations:[_Any!]!){_entities(representations:$representations){...on C{c}}}", + "operationName": "TestItf2__reviews__2", + "variables": { + "representations": [ + { + "__typename": "C", + "i": { + "x": 1, + "y": 1 + }, + "id": "1" + } + ] + } + } + }, + "response": { + "body": { + "data": { + "_entities": [ + { + "c": "c" + } + ] + } + } + } + } + ] + } + } + }, + { + "type": "Request", + "request": { + "query": "query TestItf2 { req { id c } }" + }, + "expected_response": { + "data": { + "req": { + "id": "1", + "c": "c" + } + } + } + }, + { + "type": "Stop" + } + ] +} \ No newline at end of file diff --git a/apollo-router/tests/samples/basic/interface-object/supergraph.graphql b/apollo-router/tests/samples/basic/interface-object/supergraph.graphql new file mode 100644 index 0000000000..08783bf70f --- /dev/null +++ b/apollo-router/tests/samples/basic/interface-object/supergraph.graphql @@ -0,0 +1,115 @@ +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/inaccessible/v0.2", for: SECURITY) + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) { + query: Query +} + +directive @inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + +directive @tag( + name: String! +) repeatable on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION | SCHEMA + +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field( + graph: join__Graph + requires: join__FieldSet + provides: join__FieldSet + type: String + external: Boolean + override: String + usedOverridden: Boolean +) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements( + graph: join__Graph! + interface: String! +) repeatable on OBJECT | INTERFACE + +directive @join__type( + graph: join__Graph! + key: join__FieldSet + extension: Boolean! = false + resolvable: Boolean! = true + isInterfaceObject: Boolean! = false +) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember( + graph: join__Graph! + member: String! +) repeatable on UNION + +directive @link( + url: String + as: String + for: link__Purpose + import: [link__Import] +) repeatable on SCHEMA + +scalar join__FieldSet +scalar link__Import + +enum join__Graph { + ACCOUNTS + @join__graph(name: "accounts", url: "https://accounts.demo.starstuff.dev/") + INVENTORY + @join__graph( + name: "inventory" + url: "https://inventory.demo.starstuff.dev/" + ) + PRODUCTS + @join__graph(name: "products", url: "https://products.demo.starstuff.dev/") + REVIEWS + @join__graph(name: "reviews", url: "https://reviews.demo.starstuff.dev/") +} + +enum link__Purpose { + SECURITY + EXECUTION +} + +type Query + @join__type(graph: ACCOUNTS) + @join__type(graph: REVIEWS) { + i: [I] @join__field(graph: ACCOUNTS) + req: C @join__field(graph: ACCOUNTS) +} + +interface I + @join__type(graph: ACCOUNTS, key: "id") + @join__type(graph: PRODUCTS, key: "id", isInterfaceObject: true) + @join__type(graph: REVIEWS) { + id: ID! + x: Int @join__field(graph: ACCOUNTS) @join__field(graph: REVIEWS) + y: Int @join__field(graph: PRODUCTS) @join__field(graph: REVIEWS) +} + +type A implements I + @join__type(graph: ACCOUNTS, key: "id") + @join__implements(graph: ACCOUNTS, interface: "I") { + id: ID! + x: Int + y: Int @join__field + a: String +} + +type B implements I + @join__type(graph: ACCOUNTS, key: "id") + @join__implements(graph: ACCOUNTS, interface: "I") { + id: ID! + x: Int + y: Int @join__field + b: String +} + +type C + @join__type(graph: ACCOUNTS, key: "id") + @join__type(graph: REVIEWS, key: "id") { + id: ID! + i: I @join__field(graph: ACCOUNTS) @join__field(graph: REVIEWS, external: true) + c: String @join__field(graph: REVIEWS, requires: "i { x y }") +} \ No newline at end of file From 83f57725483f0b195e43f47add0e0cc8f73246ca Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 29 Oct 2024 11:49:01 +0100 Subject: [PATCH 06/19] update entity cache samples config (#6044) --- .../entity-cache/invalidation-entity-key/configuration.yaml | 6 +++--- .../invalidation-subgraph-name/configuration.yaml | 6 +++--- .../invalidation-subgraph-type/configuration.yaml | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/configuration.yaml b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/configuration.yaml index e283bbdace..47f1e99e06 100644 --- a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/configuration.yaml +++ b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/configuration.yaml @@ -5,12 +5,12 @@ include_subgraph_errors: preview_entity_cache: enabled: true - redis: - urls: - ["redis://localhost:6379",] subgraph: all: enabled: true + redis: + urls: + ["redis://localhost:6379",] subgraphs: invalidation-entity-key-reviews: ttl: 120s diff --git a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-subgraph-name/configuration.yaml b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-subgraph-name/configuration.yaml index 85e106df9f..7efcac9a81 100644 --- a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-subgraph-name/configuration.yaml +++ b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-subgraph-name/configuration.yaml @@ -8,12 +8,12 @@ preview_entity_cache: invalidation: listen: 127.0.0.1:4000 path: /invalidation - redis: - urls: - ["redis://localhost:6379",] subgraph: all: enabled: true + redis: + urls: + ["redis://localhost:6379",] subgraphs: reviews: ttl: 120s diff --git a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-subgraph-type/configuration.yaml b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-subgraph-type/configuration.yaml index 96577bbb28..8378e3b127 100644 --- a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-subgraph-type/configuration.yaml +++ b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-subgraph-type/configuration.yaml @@ -5,9 +5,6 @@ include_subgraph_errors: preview_entity_cache: enabled: true - redis: - urls: - ["redis://localhost:6379",] invalidation: # FIXME: right now we cannot configure it to use the same port used for the GraphQL endpoint if it is chosen at random listen: 127.0.0.1:12345 @@ -15,6 +12,9 @@ preview_entity_cache: subgraph: all: enabled: true + redis: + urls: + ["redis://localhost:6379",] invalidation: enabled: true shared_key: "1234" From 39c6c03ed9f15c3872bcad083e1560ddbb1b6d3e Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 29 Oct 2024 11:59:47 +0100 Subject: [PATCH 07/19] add errors for response validation (#5787) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Andrew McGivery Co-authored-by: Renée Kooi --- .../fix_geal_response_validation_errors.md | 5 + apollo-router/src/json_ext.rs | 14 + apollo-router/src/spec/query.rs | 192 +++++++- apollo-router/src/spec/query/tests.rs | 452 ++++++++++++++++++ ...__set_context_unrelated_fetch_failure.snap | 2 +- docs/source/errors.mdx | 9 + 6 files changed, 658 insertions(+), 16 deletions(-) create mode 100644 .changesets/fix_geal_response_validation_errors.md diff --git a/.changesets/fix_geal_response_validation_errors.md b/.changesets/fix_geal_response_validation_errors.md new file mode 100644 index 0000000000..95fc3d60eb --- /dev/null +++ b/.changesets/fix_geal_response_validation_errors.md @@ -0,0 +1,5 @@ +### add errors for response validation ([Issue #5372](https://github.com/apollographql/router/issues/5372)) + +When formatting responses, the router is validating the data returned by subgraphs and replacing it with null values as appropriate. That validation phase is now adding errors when encountering the wrong type in a field requested by the client. + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/5787 \ No newline at end of file diff --git a/apollo-router/src/json_ext.rs b/apollo-router/src/json_ext.rs index c9e617635f..54d1dd0bfe 100644 --- a/apollo-router/src/json_ext.rs +++ b/apollo-router/src/json_ext.rs @@ -146,6 +146,9 @@ pub(crate) trait ValueExt { #[track_caller] fn is_object_of_type(&self, schema: &Schema, maybe_type: &str) -> bool; + /// value type + fn json_type_name(&self) -> &'static str; + /// Convert this value to an instance of `apollo_compiler::ast::Value` fn to_ast(&self) -> apollo_compiler::ast::Value; @@ -475,6 +478,17 @@ impl ValueExt for Value { }) } + fn json_type_name(&self) -> &'static str { + match self { + Value::Array(_) => "array", + Value::Null => "null", + Value::Bool(_) => "boolean", + Value::Number(_) => "number", + Value::String(_) => "string", + Value::Object(_) => "object", + } + } + fn to_ast(&self) -> apollo_compiler::ast::Value { match self { Value::Null => apollo_compiler::ast::Value::Null, diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index 3ad855870a..c8bcddec6d 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -30,6 +30,7 @@ use crate::json_ext::Object; use crate::json_ext::Path; use crate::json_ext::ResponsePathElement; use crate::json_ext::Value; +use crate::json_ext::ValueExt; use crate::plugins::authorization::UnauthorizedPaths; use crate::query_planner::fetch::OperationKind; use crate::query_planner::fetch::QueryHash; @@ -51,6 +52,7 @@ pub(crate) mod transform; pub(crate) mod traverse; pub(crate) const TYPENAME: &str = "__typename"; +pub(crate) const RESPONSE_VALIDATION: &str = "RESPONSE_VALIDATION_FAILED"; /// A GraphQL query. #[derive(Derivative, Serialize, Deserialize)] @@ -143,8 +145,9 @@ impl Query { let mut parameters = FormatParameters { variables: &variables, schema, - errors: Vec::new(), + nullification_errors: Vec::new(), nullified: Vec::new(), + validation_errors: Vec::new(), }; response.data = Some( @@ -161,12 +164,18 @@ impl Query { }, ); - if !parameters.errors.is_empty() { - if let Ok(value) = serde_json_bytes::to_value(¶meters.errors) { + if !parameters.nullification_errors.is_empty() { + if let Ok(value) = + serde_json_bytes::to_value(¶meters.nullification_errors) + { response.extensions.insert("valueCompletion", value); } } + if !parameters.validation_errors.is_empty() { + response.errors.append(&mut parameters.validation_errors); + } + return parameters.nullified; } None => { @@ -198,8 +207,9 @@ impl Query { let mut parameters = FormatParameters { variables: &all_variables, schema, - errors: Vec::new(), + nullification_errors: Vec::new(), nullified: Vec::new(), + validation_errors: Vec::new(), }; response.data = Some( @@ -215,12 +225,18 @@ impl Query { Err(InvalidValue) => Value::Null, }, ); - if !parameters.errors.is_empty() { - if let Ok(value) = serde_json_bytes::to_value(¶meters.errors) { + if !parameters.nullification_errors.is_empty() { + if let Ok(value) = + serde_json_bytes::to_value(¶meters.nullification_errors) + { response.extensions.insert("valueCompletion", value); } } + if !parameters.validation_errors.is_empty() { + response.errors.append(&mut parameters.validation_errors); + } + return parameters.nullified; } } @@ -342,6 +358,7 @@ impl Query { output: &mut Value, path: &mut Vec>, parent_type: &executable::Type, + field_or_index: FieldOrIndex<'a>, selection_set: &'a [Selection], ) -> Result<(), InvalidValue> { // for every type, if we have an invalid value, we will replace it with null @@ -362,7 +379,8 @@ impl Query { input, output, path, - field_type, + parent_type, + field_or_index, selection_set, ) { Err(_) => Err(InvalidValue), @@ -377,7 +395,7 @@ impl Query { ), _ => todo!(), }; - parameters.errors.push(Error { + parameters.nullification_errors.push(Error { message, path: Some(Path::from_response_slice(path)), ..Error::default() @@ -417,6 +435,7 @@ impl Query { &mut output_array[i], path, field_type, + FieldOrIndex::Index(i), selection_set, ); path.pop(); @@ -430,7 +449,22 @@ impl Query { Ok(()) => Ok(()), } } - _ => Ok(()), + Value::Null => Ok(()), + v => { + parameters.validation_errors.push( + Error::builder() + .message(format!( + "Invalid non-list value of type {} for list type {field_type}", + v.json_type_name() + )) + .path(Path::from_response_slice(path)) + .extension_code(RESPONSE_VALIDATION) + .build(), + ); + + *output = Value::Null; + Ok(()) + } }, executable::Type::Named(name) if name == "Int" => { let opt = if input.is_i64() { @@ -446,6 +480,19 @@ impl Query { if opt.is_some() { *output = input.clone(); } else { + if !input.is_null() { + parameters.validation_errors.push( + Error::builder() + .message(invalid_value_message( + parent_type, + field_type, + field_or_index, + )) + .path(Path::from_response_slice(path)) + .extension_code(RESPONSE_VALIDATION) + .build(), + ); + } *output = Value::Null; } Ok(()) @@ -454,6 +501,19 @@ impl Query { if input.as_f64().is_some() { *output = input.clone(); } else { + if !input.is_null() { + parameters.validation_errors.push( + Error::builder() + .message(invalid_value_message( + parent_type, + field_type, + field_or_index, + )) + .path(Path::from_response_slice(path)) + .extension_code(RESPONSE_VALIDATION) + .build(), + ); + } *output = Value::Null; } Ok(()) @@ -462,6 +522,19 @@ impl Query { if input.as_bool().is_some() { *output = input.clone(); } else { + if !input.is_null() { + parameters.validation_errors.push( + Error::builder() + .message(invalid_value_message( + parent_type, + field_type, + field_or_index, + )) + .path(Path::from_response_slice(path)) + .extension_code(RESPONSE_VALIDATION) + .build(), + ); + } *output = Value::Null; } Ok(()) @@ -470,6 +543,19 @@ impl Query { if input.as_str().is_some() { *output = input.clone(); } else { + if !input.is_null() { + parameters.validation_errors.push( + Error::builder() + .message(invalid_value_message( + parent_type, + field_type, + field_or_index, + )) + .path(Path::from_response_slice(path)) + .extension_code(RESPONSE_VALIDATION) + .build(), + ); + } *output = Value::Null; } Ok(()) @@ -478,6 +564,19 @@ impl Query { if input.is_string() || input.is_i64() || input.is_u64() || input.is_f64() { *output = input.clone(); } else { + if !input.is_null() { + parameters.validation_errors.push( + Error::builder() + .message(invalid_value_message( + parent_type, + field_type, + field_or_index, + )) + .path(Path::from_response_slice(path)) + .extension_code(RESPONSE_VALIDATION) + .build(), + ); + } *output = Value::Null; } Ok(()) @@ -497,11 +596,31 @@ impl Query { *output = input.clone(); Ok(()) } else { + parameters.validation_errors.push( + Error::builder() + .message(format!( + "Expected a valid enum value for type {}", + enum_type.name + )) + .path(Path::from_response_slice(path)) + .extension_code(RESPONSE_VALIDATION) + .build(), + ); *output = Value::Null; Ok(()) } } None => { + parameters.validation_errors.push( + Error::builder() + .message(format!( + "Expected a valid enum value for type {}", + enum_type.name + )) + .path(Path::from_response_slice(path)) + .extension_code(RESPONSE_VALIDATION) + .build(), + ); *output = Value::Null; Ok(()) } @@ -512,6 +631,9 @@ impl Query { match input { Value::Object(ref mut input_object) => { + // FIXME: we should return an error if __typename is not a string + // but this might cause issues for some production deployments where + // __typename might be missing or invalid (cf https://github.com/apollographql/router/commit/4a592f4933b7b9e46f14c7a98404b9e067687f09 ) if let Some(input_type) = input_object.get(TYPENAME).and_then(|val| val.as_str()) { @@ -564,8 +686,20 @@ impl Query { Ok(()) } - _ => { - parameters.nullified.push(Path::from_response_slice(path)); + Value::Null => { + *output = Value::Null; + Ok(()) + } + v => { + parameters.validation_errors.push( + Error::builder() + .message(format!( + "Invalid non-object value of type {} for composite type {type_name}", v.json_type_name() + )) + .path(Path::from_response_slice(path)) + .extension_code(RESPONSE_VALIDATION) + .build(), + ); *output = Value::Null; Ok(()) } @@ -641,6 +775,7 @@ impl Query { output_value, path, current_type, + FieldOrIndex::Field(field_name.as_str()), selection_set, ); path.pop(); @@ -650,7 +785,7 @@ impl Query { output.insert((*field_name).clone(), Value::Null); } if field_type.is_non_null() { - parameters.errors.push(Error { + parameters.nullification_errors.push(Error { message: format!( "Cannot return null for non-nullable field {current_type}.{}", field_name.as_str() @@ -772,6 +907,11 @@ impl Query { continue; } + let root_type = apollo_compiler::ast::Type::Named( + // Unchecked name instantiation is always safe, and we know the name is + // valid here + apollo_compiler::Name::new_unchecked(root_type_name), + ); let field_name = alias.as_ref().unwrap_or(name); let field_name_str = field_name.as_str(); @@ -800,13 +940,14 @@ impl Query { input_value, output_value, path, - &field_type.0, + &root_type, + FieldOrIndex::Field(field_name_str), selection_set, ); path.pop(); res? } else if field_type.is_non_null() { - parameters.errors.push(Error { + parameters.nullification_errors.push(Error { message: format!( "Cannot return null for non-nullable field {}.{field_name_str}", root_type_name @@ -1014,7 +1155,8 @@ impl Query { /// Intermediate structure for arguments passed through the entire formatting struct FormatParameters<'a> { variables: &'a Object, - errors: Vec, + nullification_errors: Vec, + validation_errors: Vec, nullified: Vec, schema: &'a ApiSchema, } @@ -1034,6 +1176,26 @@ pub(crate) struct Variable { default_value: Option, } +enum FieldOrIndex<'a> { + Field(&'a str), + Index(usize), +} + +fn invalid_value_message( + parent_type: &executable::Type, + field_type: &executable::Type, + field_or_index: FieldOrIndex, +) -> String { + match field_or_index { + FieldOrIndex::Field(field_name) => { + format!("Invalid value found for field {parent_type}.{field_name}") + } + FieldOrIndex::Index(i) => { + format!("Invalid value found for array element of type {field_type} at index {i}") + } + } +} + impl Operation { fn empty() -> Self { Self { diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index 1d97962405..f9af994874 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -40,6 +40,7 @@ macro_rules! assert_eq_and_ordered_json { } #[derive(Default)] +#[must_use = "Must call .test() to run the test"] struct FormatTest { schema: Option<&'static str>, query_type_name: Option<&'static str>, @@ -100,6 +101,11 @@ impl FormatTest { self } + fn expected_errors(mut self, v: serde_json_bytes::Value) -> Self { + self.expected_errors = Some(v); + self + } + fn expected_extensions(mut self, v: serde_json_bytes::Value) -> Self { self.expected_extensions = Some(v); self @@ -1182,6 +1188,452 @@ fn reformat_response_array_of_id_duplicate() { .test(); } +#[test] +// If this test fails, this means you got greedy about allocations, +// beware of aliases! +fn reformat_response_expected_types() { + FormatTest::builder() + .schema( + "type Query { + get: Thing + } + type Thing { + i: Int + s: String + f: Float + b: Boolean + e: E + u: U + id: ID + l: [Int] + } + + enum E { + A + B + } + union U = ObjA | ObjB + type ObjA { + a: String + } + type ObjB { + a: String + } + ", + ) + .query( + r#"{ + get { + i + s + f + ... on Thing { + b + e + u { + ... on ObjA { + a + } + } + id + } + l + } + }"#, + ) + .response(json! {{ + "get": { + "i": "hello", + "s": 1.0, + "f": [1], + "b": 0, + "e": "X", + "u": 1, + "id": { + "test": "test", + }, + "l": "A" + }, + }}) + .expected(json! {{ + "get": { + "i": null, + "s": null, + "f": null, + "b": null, + "e": null, + "u": null, + // FIXME(@goto-bus-stop): this should be null, but we do not + // validate ID values today + "id": { + "test": "test", + }, + "l": null + }, + }}) + .expected_errors(json! ([ + { + "message": "Invalid value found for field Thing.i", + "path": ["get", "i"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Thing.s", + "path": ["get", "s"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Thing.f", + "path": ["get", "f"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Thing.b", + "path": ["get", "b"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Expected a valid enum value for type E", + "path": ["get", "e"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid non-object value of type number for composite type U", + "path": ["get", "u"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid non-list value of type string for list type [Int]", + "path": ["get", "l"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + } + ])) + .test(); +} + +#[test] +fn reformat_response_expected_int() { + FormatTest::builder() + .schema( + r#" + type Query { + a: Int + b: Int + c: Int + d: Int + e: Int + f: Int + g: Int + } + "#, + ) + .query(r#"{ a b c d e f g }"#) + .response(json!({ + "a": 1, + "b": 1.0, // Should be accepted as Int 1 + "c": 1.2, // Float should not be truncated + "d": "1234", // Optional to be coerced by spec: we do not do so + "e": true, + "f": [1], + "g": { "value": 1 }, + })) + .expected(json!({ + "a": 1, + // FIXME(@goto-bus-stop): we should accept this, and truncate it + // to Int value `1`, but do not do so today + "b": null, + "c": null, + "d": null, + "e": null, + "f": null, + "g": null, + })) + .expected_errors(json!([ + { + "message": "Invalid value found for field Query.b", + "path": ["b"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.c", + "path": ["c"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.d", + "path": ["d"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.e", + "path": ["e"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.f", + "path": ["f"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.g", + "path": ["g"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + ])) + .test(); +} + +#[test] +fn reformat_response_expected_int_range() { + let schema = "type Query { + me: User + } + + type User { + id: String! + name: String + someNumber: Int + someOtherNumber: Int! + } + "; + + let query = "query { me { id name someNumber } }"; + + FormatTest::builder() + .schema(schema) + .query(query) + .response(json!({ + "me": { + "id": "123", + "name": "Guy Guyson", + "someNumber": 51049694213_i64 + }, + })) + .expected(json!({ + "me": { + "id": "123", + "name": "Guy Guyson", + "someNumber": null, + }, + })) + .expected_errors(json!([ + { + "message": "Invalid value found for field User.someNumber", + "path": ["me", "someNumber"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" }, + } + ])) + .test(); + + let query2 = "query { me { id name someOtherNumber } }"; + + FormatTest::builder() + .schema(schema) + .query(query2) + .response(json!({ + "me": { + "id": "123", + "name": "Guy Guyson", + "someOtherNumber": 51049694213_i64 + }, + })) + .expected(json!({ + "me": null, + })) + .expected_errors(json!([ + { + "message": "Invalid value found for field User.someOtherNumber", + "path": ["me", "someOtherNumber"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" }, + }, + ])) + .test(); +} + +#[test] +fn reformat_response_expected_float() { + FormatTest::builder() + .schema( + r#" + type Query { + a: Float + b: Float + c: Float + d: Float + e: Float + f: Float + } + "#, + ) + .query(r#"{ a b c d e f }"#) + .response(json!({ + // Note: NaNs and Infinitys are not supported by GraphQL Floats, + // and handily not representable in JSON, so we don't need to handle them. + "a": 1, // Int can be interpreted as Float + "b": 1.2, + "c": "2.2", // Optional to be coerced by spec: we do not do so + "d": true, + "e": [1.234], + "f": { "value": 12.34 }, + })) + .expected(json!({ + "a": 1, // Representing int-valued float without the decimals is okay in JSON + "b": 1.2, + "c": null, + "d": null, + "e": null, + "f": null, + })) + .expected_errors(json!([ + { + "message": "Invalid value found for field Query.c", + "path": ["c"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.d", + "path": ["d"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.e", + "path": ["e"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.f", + "path": ["f"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + ])) + .test(); +} + +#[test] +fn reformat_response_expected_string() { + FormatTest::builder() + .schema( + r#" + type Query { + a: String + b: String + c: String + d: String + e: String + f: String + } + "#, + ) + .query(r#"{ a b c d e f }"#) + .response(json!({ + "a": "text", + "b": 1, // Optional to be coerced by spec: we do not do so + "c": false, // Optional to be coerced by spec: we do not do so + "d": 1234.5678, // Optional to be coerced by spec: we do not do so + "e": ["s"], + "f": { "text": "text" }, + })) + .expected(json!({ + "a": "text", + "b": null, + "c": null, + "d": null, + "e": null, + "f": null, + })) + .expected_errors(json!([ + { + "message": "Invalid value found for field Query.b", + "path": ["b"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.c", + "path": ["c"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.d", + "path": ["d"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.e", + "path": ["e"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + { + "message": "Invalid value found for field Query.f", + "path": ["f"], + "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + }, + ])) + .test(); +} + +#[test] +fn reformat_response_expected_id() { + FormatTest::builder() + .schema( + r#" + type Query { + a: ID + b: ID + c: ID + d: ID + e: ID + f: ID + g: ID + } + "#, + ) + .query(r#"{ a b c d e f g }"#) + .response(json!({ + "a": "1234", + "b": "ABCD", + "c": 1234, + "d": 1234.0, // Integer represented as a float should be coerced + "e": false, + "f": 1234.5678, // Float should not be truncated + "g": ["s"], + })) + .expected(json!({ + // Note technically IDs should always be represented as a String in JSON, + // though the value returned from a field can be either Int or String. + // We do not coerce the acceptable types to strings today. + "a": "1234", + "b": "ABCD", + "c": 1234, + // FIXME(@goto-bus-stop): We should coerce this to string "1234" (without .0), + // but we don't do so today + "d": 1234.0, + // FIXME(@goto-bus-stop): We should null out all these values, + // but we don't validate IDs today + "e": false, + "f": 1234.5678, + "g": ["s"], + })) + .expected_errors(json!([ + // FIXME(@goto-bus-stop): we should expect these errors: + // { + // "message": "Invalid value found for field Query.e", + // "path": ["e"], + // "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + // }, + // { + // "message": "Invalid value found for field Query.f", + // "path": ["f"], + // "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + // }, + // { + // "message": "Invalid value found for field Query.g", + // "path": ["g"], + // "extensions": { "code": "RESPONSE_VALIDATION_FAILED" } + // }, + ])) + .test(); +} + #[test] fn solve_query_with_single_typename() { FormatTest::builder() diff --git a/apollo-router/tests/snapshots/set_context__set_context_unrelated_fetch_failure.snap b/apollo-router/tests/snapshots/set_context__set_context_unrelated_fetch_failure.snap index 49dcf6bf9b..4f28e80419 100644 --- a/apollo-router/tests/snapshots/set_context__set_context_unrelated_fetch_failure.snap +++ b/apollo-router/tests/snapshots/set_context__set_context_unrelated_fetch_failure.snap @@ -160,7 +160,7 @@ expression: response ] }, { - "message": "Cannot return null for non-nullable field T!.t", + "message": "Cannot return null for non-nullable field Query.t", "path": [ "t" ] diff --git a/docs/source/errors.mdx b/docs/source/errors.mdx index 5730dd6fef..497471d8d9 100644 --- a/docs/source/errors.mdx +++ b/docs/source/errors.mdx @@ -99,4 +99,13 @@ The query could not be parsed. The response from a subgraph did not match the GraphQL schema. + + + + +A subgraph returned a field with a different type that mandated by the GraphQL schema. + + + + From f2dc704d69f11ccbfdbf2e5198547f17ba9e61b9 Mon Sep 17 00:00:00 2001 From: timbotnik Date: Tue, 29 Oct 2024 22:21:10 +1100 Subject: [PATCH 08/19] Apply suggestions from code review Co-authored-by: Edward Huang --- docs/source/configuration/overview.mdx | 8 ++-- .../telemetry/apollo-telemetry.mdx | 47 ++++++++++--------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/docs/source/configuration/overview.mdx b/docs/source/configuration/overview.mdx index c96db9d4fc..2d0ec9af30 100644 --- a/docs/source/configuration/overview.mdx +++ b/docs/source/configuration/overview.mdx @@ -873,11 +873,11 @@ You won't see an immediate change in checks behavior when you first turn on exte -Beginning in v1.49.0, the router supports sending traces to Studio via the more detailed OTel (OpenTelemetry) protocol. -Support for OTel traces has historically only been available for 3rd party APM tools. With this option, -Studio can now provide a much more granular view of Router internals than the legacy Apollo tracing protocol. +Beginning in v1.49.0, the router supports sending traces to Studio via the OpenTelemetry Protocol (OTLP). +Support for OTLP traces has historically only been available for third-party APM tools. With this option, +Studio can now provide a much more granular and detailed view of router internals than the previous Apollo tracing protocol. -See [Enhanced tracing in Studio via OTel](./telemetry/apollo-telemetry#enhanced-tracing-in-studio-via-opentelemetry). +To learn more, see [Enhanced tracing in Studio via OpenTelemetry](./telemetry/apollo-telemetry#enhanced-tracing-in-studio-via-opentelemetry). ### Safelisting with persisted queries diff --git a/docs/source/configuration/telemetry/apollo-telemetry.mdx b/docs/source/configuration/telemetry/apollo-telemetry.mdx index d4e276118e..20070898ce 100644 --- a/docs/source/configuration/telemetry/apollo-telemetry.mdx +++ b/docs/source/configuration/telemetry/apollo-telemetry.mdx @@ -80,30 +80,34 @@ telemetry: -Beginning in v1.49.0, the router supports sending traces to Studio via the more detailed OTel (OpenTelemetry) protocol. +Beginning in v1.49.0, the router supports sending traces to Studio via the OpenTelemetry protocol (OTLP). +Support for OTLP traces has historically only been available for third-party APM tools. With this option, +Studio can now provide a much more granular and detailed view of router internals than the previous Apollo tracing protocol. Support for OTel traces has historically only been available for 3rd party APM tools. With this option, Studio can now provide a much more granular view of Router internals than the legacy Apollo tracing protocol. -Benefits include: +Benefits of OTLP traces include: -- A comprehensive way to visualize the Router execution path in Studio. -- Additional spans that were previously not included in Studio traces, such as query parsing, planning, execution, and more. -- Additional attributes including HTTP request details, REST connector details, and more. +- Comprehensive visualization of the router execution path in Studio +- New spans in Studio traces, including query parsing, planning, execution, and more +- New attributes, including HTTP request details, REST connector details, and more -It is expected that this will become the default in a future version of Router. #### Configuration -This change adds a new configuration option `telemetry.apollo.experimental_otlp_tracing_sampler`. Use this option to send +You can configure OTel traces with the `telemetry.apollo.experimental_otlp_tracing_sampler` option. Use this option to send a percentage of traces to Studio via OTLP instead of the native Apollo Usage Reporting protocol. Supported values: -- `always_off` (default): send all traces via the legacy Apollo Usage Reporting protocol. -- `always_on`: send all traces via OTLP. -- `0.0 - 1.0` (used for testing): the ratio of traces to send via OTLP (0.5 = 50 / 50). +- `always_off` (default): send all traces via the legacy Apollo Usage Reporting protocol +- `always_on`: send all traces via OTLP +- `0.0 - 1.0` (used for testing): the ratio of traces to send via OTLP (0.4 = 40% OTLP / 60% legacy) -Note that this sampler is only applied _after_ the common tracing sampler, for example: +This sampler is applied after the common tracing sampler. + +#### Example configuration + +An example configuration that samples 1% of traces and sends all traces via OTLP: -#### Sample 1% of traces, send all traces via OTLP: ```yaml telemetry: @@ -118,21 +122,18 @@ telemetry: sampler: 0.01 ``` -OTel traces sent to Studio will not necessarily be identical to the ones sent to 3rd Party APM tools via OTLP: +OTLP traces sent to Studio aren't necessarily identical to ones sent to third-party APM tools via OTLP: -- Only specific OTLP attributes will be included for parity with what is provided in legacy traces today. This ensures that data privacy - is maintained in an equivalent manner. The existing Router configuration options for Apollo telemetry will continue to function - with OTLP traces, such as forwarding of GraphQL errors, headers, and variables. -- Some features of OTLP traces may only be available in Studio and not in 3rd Party APM tools (e.g. resolver-level timing information from - [Federated Tracing](../../federation/metrics/#enabling-federated-tracing)). +- Only specific OTLP attributes are included for parity with legacy traces today. This ensures that data privacy is maintained in an equivalent manner. Existing router configuration options for Apollo telemetry will continue to function with OTLP traces, including forwarding of GraphQL errors, headers, and variables. +- Some features of OTLP traces are available only in Studio and not in third-party APM tools, such as resolver-level timing information from [Federated Tracing](../../federation/metrics/#enabling-federated-tracing) -This change results in using a new wire protocol for traces, and some users may experience an increase in tracing traffic -to GraphOS Studio due to the additional detail being captured. In exceptional situations it may be necessary to send fewer traces. -This can be achieved via sending fewer traces (`telemetry.exporters.tracing.common.sampler`) or as a last resort, falling back -to the old protocol via `telemetry.apollo.otlp_tracing_sampler` to send fewer OTLP traces or fully disable them. -Any performance regressions due to the new tracing protocol should also be reported to the Apollo support team. +You may experience an increase in tracing traffic sent to GraphOS Studio due to the additional detail captured by the new wire protocol. In exceptional situations, you may need to send fewer traces. + +To send fewer traces, configure `telemetry.exporters.tracing.common.sampler` or revert to the old protocol via `telemetry.apollo.otlp_tracing_sampler` to send fewer OTLP traces or to disable them. + +For performance regressions due to the new tracing protocol, you should report them to the [Apollo support team](https://www.apollographql.com/support). From 442a543e8078eec7e0c0015e8111f5a939c6f00a Mon Sep 17 00:00:00 2001 From: timbotnik Date: Tue, 29 Oct 2024 23:37:19 +1100 Subject: [PATCH 09/19] Update docs/source/configuration/telemetry/apollo-telemetry.mdx Co-authored-by: Coenen Benjamin --- docs/source/configuration/telemetry/apollo-telemetry.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/configuration/telemetry/apollo-telemetry.mdx b/docs/source/configuration/telemetry/apollo-telemetry.mdx index 20070898ce..4c2fb8d04f 100644 --- a/docs/source/configuration/telemetry/apollo-telemetry.mdx +++ b/docs/source/configuration/telemetry/apollo-telemetry.mdx @@ -109,7 +109,7 @@ This sampler is applied after the common tracing sampler. An example configuration that samples 1% of traces and sends all traces via OTLP: -```yaml +```yaml title="router.config.yaml" telemetry: apollo: # Send all traces via OTLP From aa8bdd074e1be3449b62d899652583a5a839c93d Mon Sep 17 00:00:00 2001 From: Coenen Benjamin Date: Wed, 30 Oct 2024 10:18:25 +0100 Subject: [PATCH 10/19] Remove summit ad (#6209) --- README.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/README.md b/README.md index 0f87d6d343..df19eb7593 100644 --- a/README.md +++ b/README.md @@ -4,11 +4,6 @@ --- -**Announcement:** -Join 1000+ engineers at GraphQL Summit for talks, workshops, and office hours, Oct 8-10 in NYC. [Get your pass here ->](https://summit.graphql.com/?utm_campaign=github_federation_readme) - ---- - # Apollo Router Core The **Apollo Router Core** is a configurable, high-performance **graph router** written in Rust to run a [federated supergraph](https://www.apollographql.com/docs/federation/) that uses [Apollo Federation 2](https://www.apollographql.com/docs/federation/v2/federation-2/new-in-federation-2). From d31da835092ff3ade224ffc3b2c6d372058c512e Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Wed, 30 Oct 2024 14:35:59 +0100 Subject: [PATCH 11/19] Progressive override: fix query planner cache warmup (#6108) The query planner needs various metadata along with the query to influence planning, and due to historical reasons that data was passed through the request context, not the planner request. The progressive override labels list was communicated like that, but missed the case of query plan cache warmup, where upon schema or configuration updates, the router takes the list of most used queries from the query planner's in memory cache, and plans them again before activating the new schema or configuration. Due to the mistake, the labels were not transmitted during warmup, which resulted in queries correctly using the overridden fields, but after an update, would revert to non overridden fields, and could not recover (unless the plan was evicted from the cache). Co-authored-by: Jesse Rosenberger --- .../fix_geal_progressive_override_test.md | 5 + .circleci/config.yml | 2 + .../query_planner/caching_query_planner.rs | 5 + .../progressive-override/basic/README.md | 3 + .../basic/configuration.yaml | 13 + .../basic/configuration2.yaml | 17 ++ .../progressive-override/basic/plan.json | 226 ++++++++++++++++++ .../progressive-override/basic/rhai/main.rhai | 22 ++ .../basic/supergraph.graphql | 98 ++++++++ .../progressive-override/warmup/README.md | 3 + .../warmup/configuration.yaml | 17 ++ .../warmup/configuration2.yaml | 21 ++ .../progressive-override/warmup/plan.json | 196 +++++++++++++++ .../warmup/rhai/main.rhai | 22 ++ .../warmup/supergraph.graphql | 98 ++++++++ apollo-router/tests/samples_tests.rs | 16 ++ 16 files changed, 764 insertions(+) create mode 100644 .changesets/fix_geal_progressive_override_test.md create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/basic/README.md create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/basic/configuration.yaml create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/basic/configuration2.yaml create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/basic/plan.json create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/basic/rhai/main.rhai create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/basic/supergraph.graphql create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/warmup/README.md create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/warmup/configuration.yaml create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/warmup/configuration2.yaml create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/warmup/plan.json create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/warmup/rhai/main.rhai create mode 100644 apollo-router/tests/samples/enterprise/progressive-override/warmup/supergraph.graphql diff --git a/.changesets/fix_geal_progressive_override_test.md b/.changesets/fix_geal_progressive_override_test.md new file mode 100644 index 0000000000..eea5a214ab --- /dev/null +++ b/.changesets/fix_geal_progressive_override_test.md @@ -0,0 +1,5 @@ +### Progressive override: fix query planner cache warmup ([PR #6108](https://github.com/apollographql/router/pull/6108)) + +This fixes an issue in progressive override where the override labels would not be transmitted to the query planner during cache warmup, resulting in queries correctly using the overridden fields at first, but after an update, would revert to non overridden fields, and could not recover. + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/6108 \ No newline at end of file diff --git a/.circleci/config.yml b/.circleci/config.yml index eea532b73b..91bce18b05 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,5 +1,7 @@ version: 2.1 +# Cache key bump: 1 + # These "CircleCI Orbs" are reusable bits of configuration that can be shared # across projects. See https://circleci.com/orbs/ for more information. orbs: diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs index 1c4e953c05..6ef9a671e8 100644 --- a/apollo-router/src/query_planner/caching_query_planner.rs +++ b/apollo-router/src/query_planner/caching_query_planner.rs @@ -336,6 +336,11 @@ where lock.insert(caching_key.metadata) }); + let _ = context.insert( + LABELS_TO_OVERRIDE_KEY, + caching_key.plan_options.override_conditions.clone(), + ); + let request = QueryPlannerRequest { query, operation_name, diff --git a/apollo-router/tests/samples/enterprise/progressive-override/basic/README.md b/apollo-router/tests/samples/enterprise/progressive-override/basic/README.md new file mode 100644 index 0000000000..3802c3942d --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/basic/README.md @@ -0,0 +1,3 @@ +# Progressive override + +This tests subgraph field migration: https://www.apollographql.com/docs/federation/entities/migrate-fields/ \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/progressive-override/basic/configuration.yaml b/apollo-router/tests/samples/enterprise/progressive-override/basic/configuration.yaml new file mode 100644 index 0000000000..321fd80abd --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/basic/configuration.yaml @@ -0,0 +1,13 @@ +include_subgraph_errors: + all: true + +telemetry: + exporters: + logging: + stdout: + format: text + +experimental_query_planner_mode: legacy + +plugins: + experimental.expose_query_plan: true \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/progressive-override/basic/configuration2.yaml b/apollo-router/tests/samples/enterprise/progressive-override/basic/configuration2.yaml new file mode 100644 index 0000000000..7c445ce5b2 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/basic/configuration2.yaml @@ -0,0 +1,17 @@ +include_subgraph_errors: + all: true + +telemetry: + exporters: + logging: + stdout: + format: text + +experimental_query_planner_mode: legacy + +rhai: + scripts: "tests/samples/enterprise/progressive-override/basic/rhai" + main: "main.rhai" + +plugins: + experimental.expose_query_plan: true \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/progressive-override/basic/plan.json b/apollo-router/tests/samples/enterprise/progressive-override/basic/plan.json new file mode 100644 index 0000000000..6cdf9d7b60 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/basic/plan.json @@ -0,0 +1,226 @@ +{ + "enterprise": true, + "actions": [ + { + "type": "Start", + "schema_path": "./supergraph.graphql", + "configuration_path": "./configuration.yaml", + "subgraphs": { + "Subgraph1": { + "requests": [ + { + "request": { + "body": { + "query": "query progressive1__Subgraph1__0{percent100{__typename id}}", + "operationName": "progressive1__Subgraph1__0" + } + }, + "response": { + "body": { + "data": { + "percent100": { + "__typename": "T", + "id": "1" + } + } + } + } + } + ] + }, + "Subgraph2": { + "requests": [ + { + "request": { + "body": { + "query": "query progressive1__Subgraph2__1($representations:[_Any!]!){_entities(representations:$representations){...on T{foo}}}", + "operationName": "progressive1__Subgraph2__1", + "variables": { + "representations": [ + { + "__typename": "T", + "id": "1" + } + ] + } + } + }, + "response": { + "body": { + "data": { + "_entities": [ + { + "foo": 1 + } + ] + } + } + } + }, + { + "request": { + "body": { + "query": "query progressive2__Subgraph2__0{percent0{foo}}", + "operationName": "progressive2__Subgraph2__0" + } + }, + "response": { + "body": { + "data": { + "percent0": { + "foo": 2 + } + } + } + } + } + ] + } + } + }, + { + "type": "Request", + "request": { + "query": "query progressive1 { percent100 { foo } }" + }, + "headers": { + "apollo-expose-query-plan": "false" + }, + "expected_response": { + "data": { + "percent100": { + "foo": 1 + } + } + } + }, + { + "type": "Request", + "request": { + "query": "query progressive2 { percent0 { foo } }" + }, + "expected_response": { + "data": { + "percent0": { + "foo": 2 + } + } + } + }, + { + "type": "ReloadConfiguration", + "configuration_path": "./configuration2.yaml" + }, + { + "type": "ReloadSubgraphs", + "subgraphs": { + "Subgraph1": { + "requests": [ + { + "request": { + "body": { + "query": "query progressive3__Subgraph1__0{percent100{__typename id}}", + "operationName": "progressive3__Subgraph1__0" + } + }, + "response": { + "body": { + "data": { + "percent100": { + "__typename": "T", + "id": "1" + } + } + } + } + }, + { + "request": { + "body": { + "query": "query progressive4__Subgraph1__0{percent100{bar}}", + "operationName": "progressive4__Subgraph1__0" + } + }, + "response": { + "body": { + "data": { + "percent100": { + "bar": 2 + } + } + } + } + } + ] + }, + "Subgraph2": { + "requests": [ + { + "request": { + "body": { + "query": "query progressive3__Subgraph2__1($representations:[_Any!]!){_entities(representations:$representations){...on T{bar}}}", + "operationName": "progressive3__Subgraph2__1", + "variables": { + "representations": [ + { + "__typename": "T", + "id": "1" + } + ] + } + } + }, + "response": { + "body": { + "data": { + "_entities": [ + { + "bar": 1 + } + ] + } + } + } + } + ] + } + } + }, + { + "type": "Request", + "request": { + "query": "query progressive3 { percent100 { bar } }" + }, + "headers": { + "apollo-expose-query-plan": "false" + }, + "expected_response": { + "data": { + "percent100": { + "bar": 1 + } + } + } + }, + { + "type": "Request", + "request": { + "query": "query progressive4 { percent100 { bar } }" + }, + "headers": { + "apollo-expose-query-plan": "false", + "x-override": "true" + }, + "expected_response": { + "data": { + "percent100": { + "bar": 2 + } + } + } + }, + { + "type": "Stop" + } + ] +} \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/progressive-override/basic/rhai/main.rhai b/apollo-router/tests/samples/enterprise/progressive-override/basic/rhai/main.rhai new file mode 100644 index 0000000000..3ecb55cca9 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/basic/rhai/main.rhai @@ -0,0 +1,22 @@ +fn supergraph_service(service) { + const request_callback = Fn("process_request"); + service.map_request(request_callback); +} + +// Add a timestamp to context which we'll use in the response. +fn process_request(request) { + request.context["request_start"] = Router.APOLLO_START.elapsed; + let labels = request.context["apollo_override::unresolved_labels"]; + print(`unresolved: ${labels}`); + + let override = request.context["apollo_override::labels_to_override"]; + print(`override: ${override}`); + + + if "x-override" in request.headers { + if request.headers["x-override"] == "true" { + request.context["apollo_override::labels_to_override"] += "bar"; + } + } +} + diff --git a/apollo-router/tests/samples/enterprise/progressive-override/basic/supergraph.graphql b/apollo-router/tests/samples/enterprise/progressive-override/basic/supergraph.graphql new file mode 100644 index 0000000000..e5ffb347c0 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/basic/supergraph.graphql @@ -0,0 +1,98 @@ +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.4", for: EXECUTION) { + query: Query +} + +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field( + graph: join__Graph + requires: join__FieldSet + provides: join__FieldSet + type: String + external: Boolean + override: String + usedOverridden: Boolean + overrideLabel: String +) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements( + graph: join__Graph! + interface: String! +) repeatable on OBJECT | INTERFACE + +directive @join__type( + graph: join__Graph! + key: join__FieldSet + extension: Boolean! = false + resolvable: Boolean! = true + isInterfaceObject: Boolean! = false +) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember( + graph: join__Graph! + member: String! +) repeatable on UNION + +directive @link( + url: String + as: String + for: link__Purpose + import: [link__Import] +) repeatable on SCHEMA + +scalar join__FieldSet + +enum join__Graph { + SUBGRAPH1 @join__graph(name: "Subgraph1", url: "https://Subgraph1") + SUBGRAPH2 @join__graph(name: "Subgraph2", url: "https://Subgraph2") +} + +scalar link__Import + +enum link__Purpose { + """ + \`SECURITY\` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + \`EXECUTION\` features provide metadata necessary for operation execution. + """ + EXECUTION +} + +type Query @join__type(graph: SUBGRAPH1) @join__type(graph: SUBGRAPH2) { + percent100: T + @join__field( + graph: SUBGRAPH1 + override: "Subgraph2" + overrideLabel: "percent(100)" + ) + @join__field(graph: SUBGRAPH2, overrideLabel: "percent(100)") + percent0: T + @join__field( + graph: SUBGRAPH1 + override: "Subgraph2" + overrideLabel: "percent(0)" + ) + @join__field(graph: SUBGRAPH2, overrideLabel: "percent(0)") +} + +type T + @join__type(graph: SUBGRAPH1, key: "id") + @join__type(graph: SUBGRAPH2, key: "id") { + id: ID + foo: Int + @join__field(graph: SUBGRAPH1, override: "Subgraph2", overrideLabel: "foo") + @join__field(graph: SUBGRAPH2, overrideLabel: "foo") + bar: Int + @join__field(graph: SUBGRAPH1, override: "Subgraph2", overrideLabel: "bar") + @join__field(graph: SUBGRAPH2, overrideLabel: "bar") + baz: Int + @join__field(graph: SUBGRAPH1, override: "Subgraph2", overrideLabel: "baz") + @join__field(graph: SUBGRAPH2, overrideLabel: "baz") +} diff --git a/apollo-router/tests/samples/enterprise/progressive-override/warmup/README.md b/apollo-router/tests/samples/enterprise/progressive-override/warmup/README.md new file mode 100644 index 0000000000..dfc4a1834c --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/warmup/README.md @@ -0,0 +1,3 @@ +# Progressive override warmup + +This checks progrssive override behaviour across router reloads \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/progressive-override/warmup/configuration.yaml b/apollo-router/tests/samples/enterprise/progressive-override/warmup/configuration.yaml new file mode 100644 index 0000000000..b069d5af18 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/warmup/configuration.yaml @@ -0,0 +1,17 @@ +include_subgraph_errors: + all: true + +supergraph: + query_planning: + warmed_up_queries: 5 + +telemetry: + exporters: + logging: + stdout: + format: text + +experimental_query_planner_mode: legacy + +plugins: + experimental.expose_query_plan: true \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/progressive-override/warmup/configuration2.yaml b/apollo-router/tests/samples/enterprise/progressive-override/warmup/configuration2.yaml new file mode 100644 index 0000000000..413d26aba3 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/warmup/configuration2.yaml @@ -0,0 +1,21 @@ +include_subgraph_errors: + all: true + +supergraph: + query_planning: + warmed_up_queries: 6 + +telemetry: + exporters: + logging: + stdout: + format: text + +experimental_query_planner_mode: legacy + +# rhai: +# scripts: "tests/samples/enterprise/progressive-override/rhai" +# main: "main.rhai" + +plugins: + experimental.expose_query_plan: true \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/progressive-override/warmup/plan.json b/apollo-router/tests/samples/enterprise/progressive-override/warmup/plan.json new file mode 100644 index 0000000000..8f913eb5be --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/warmup/plan.json @@ -0,0 +1,196 @@ +{ + "enterprise": true, + "actions": [ + { + "type": "Start", + "schema_path": "./supergraph.graphql", + "configuration_path": "./configuration.yaml", + "subgraphs": { + "Subgraph1": { + "requests": [ + { + "request": { + "body": { + "query": "query progressive1__Subgraph1__0{percent100{__typename id}}", + "operationName": "progressive1__Subgraph1__0" + } + }, + "response": { + "body": { + "data": { + "percent100": { + "__typename": "T", + "id": "1" + } + } + } + } + } + ] + }, + "Subgraph2": { + "requests": [ + { + "request": { + "body": { + "query": "query progressive1__Subgraph2__1($representations:[_Any!]!){_entities(representations:$representations){...on T{foo}}}", + "operationName": "progressive1__Subgraph2__1", + "variables": { + "representations": [ + { + "__typename": "T", + "id": "1" + } + ] + } + } + }, + "response": { + "body": { + "data": { + "_entities": [ + { + "foo": 1 + } + ] + } + } + } + }, + { + "request": { + "body": { + "query": "query progressive2__Subgraph2__0{percent0{foo}}", + "operationName": "progressive2__Subgraph2__0" + } + }, + "response": { + "body": { + "data": { + "percent0": { + "foo": 2 + } + } + } + } + } + ] + } + } + }, + { + "type": "Request", + "request": { + "query": "query progressive1 { percent100 { foo } }" + }, + "headers": { + "apollo-expose-query-plan": "false" + }, + "expected_response": { + "data": { + "percent100": { + "foo": 1 + } + } + } + }, + { + "type": "ReloadConfiguration", + "configuration_path": "./configuration2.yaml" + }, + { + "type": "ReloadSubgraphs", + "subgraphs": { + "Subgraph1": { + "requests": [ + { + "request": { + "body": { + "query": "query progressive1__Subgraph1__0{percent100{__typename id}}", + "operationName": "progressive1__Subgraph1__0" + } + }, + "response": { + "body": { + "data": { + "percent100": { + "__typename": "T", + "id": "1" + } + } + } + } + } + ] + }, + "Subgraph2": { + "requests": [ + { + "request": { + "body": { + "query": "query progressive1__Subgraph2__1($representations:[_Any!]!){_entities(representations:$representations){...on T{foo}}}", + "operationName": "progressive1__Subgraph2__1", + "variables": { + "representations": [ + { + "__typename": "T", + "id": "1" + } + ] + } + } + }, + "response": { + "body": { + "data": { + "_entities": [ + { + "foo": 1 + } + ] + } + } + } + }, + { + "request": { + "body": { + "query": "query progressive2__Subgraph2__0{percent0{foo}}", + "operationName": "progressive2__Subgraph2__0" + } + }, + "response": { + "body": { + "data": { + "percent0": { + "foo": 2 + } + } + } + } + } + ] + } + } + }, + { + "type": "Request", + "request": { + "query": "query progressive1 { percent100 { foo } }" + }, + "headers": { + "apollo-expose-query-plan": "false" + }, + "expected_response": { + "data": { + "percent100": { + "foo": 1 + } + } + } + }, + { + "type": "Stop" + } + ] +} \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/progressive-override/warmup/rhai/main.rhai b/apollo-router/tests/samples/enterprise/progressive-override/warmup/rhai/main.rhai new file mode 100644 index 0000000000..3ecb55cca9 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/warmup/rhai/main.rhai @@ -0,0 +1,22 @@ +fn supergraph_service(service) { + const request_callback = Fn("process_request"); + service.map_request(request_callback); +} + +// Add a timestamp to context which we'll use in the response. +fn process_request(request) { + request.context["request_start"] = Router.APOLLO_START.elapsed; + let labels = request.context["apollo_override::unresolved_labels"]; + print(`unresolved: ${labels}`); + + let override = request.context["apollo_override::labels_to_override"]; + print(`override: ${override}`); + + + if "x-override" in request.headers { + if request.headers["x-override"] == "true" { + request.context["apollo_override::labels_to_override"] += "bar"; + } + } +} + diff --git a/apollo-router/tests/samples/enterprise/progressive-override/warmup/supergraph.graphql b/apollo-router/tests/samples/enterprise/progressive-override/warmup/supergraph.graphql new file mode 100644 index 0000000000..e5ffb347c0 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/progressive-override/warmup/supergraph.graphql @@ -0,0 +1,98 @@ +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.4", for: EXECUTION) { + query: Query +} + +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field( + graph: join__Graph + requires: join__FieldSet + provides: join__FieldSet + type: String + external: Boolean + override: String + usedOverridden: Boolean + overrideLabel: String +) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements( + graph: join__Graph! + interface: String! +) repeatable on OBJECT | INTERFACE + +directive @join__type( + graph: join__Graph! + key: join__FieldSet + extension: Boolean! = false + resolvable: Boolean! = true + isInterfaceObject: Boolean! = false +) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember( + graph: join__Graph! + member: String! +) repeatable on UNION + +directive @link( + url: String + as: String + for: link__Purpose + import: [link__Import] +) repeatable on SCHEMA + +scalar join__FieldSet + +enum join__Graph { + SUBGRAPH1 @join__graph(name: "Subgraph1", url: "https://Subgraph1") + SUBGRAPH2 @join__graph(name: "Subgraph2", url: "https://Subgraph2") +} + +scalar link__Import + +enum link__Purpose { + """ + \`SECURITY\` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + \`EXECUTION\` features provide metadata necessary for operation execution. + """ + EXECUTION +} + +type Query @join__type(graph: SUBGRAPH1) @join__type(graph: SUBGRAPH2) { + percent100: T + @join__field( + graph: SUBGRAPH1 + override: "Subgraph2" + overrideLabel: "percent(100)" + ) + @join__field(graph: SUBGRAPH2, overrideLabel: "percent(100)") + percent0: T + @join__field( + graph: SUBGRAPH1 + override: "Subgraph2" + overrideLabel: "percent(0)" + ) + @join__field(graph: SUBGRAPH2, overrideLabel: "percent(0)") +} + +type T + @join__type(graph: SUBGRAPH1, key: "id") + @join__type(graph: SUBGRAPH2, key: "id") { + id: ID + foo: Int + @join__field(graph: SUBGRAPH1, override: "Subgraph2", overrideLabel: "foo") + @join__field(graph: SUBGRAPH2, overrideLabel: "foo") + bar: Int + @join__field(graph: SUBGRAPH1, override: "Subgraph2", overrideLabel: "bar") + @join__field(graph: SUBGRAPH2, overrideLabel: "bar") + baz: Int + @join__field(graph: SUBGRAPH1, override: "Subgraph2", overrideLabel: "baz") + @join__field(graph: SUBGRAPH2, overrideLabel: "baz") +} diff --git a/apollo-router/tests/samples_tests.rs b/apollo-router/tests/samples_tests.rs index 22e3b31e18..cf2e9b99f7 100644 --- a/apollo-router/tests/samples_tests.rs +++ b/apollo-router/tests/samples_tests.rs @@ -231,6 +231,22 @@ impl TestExecution { path: &Path, out: &mut String, ) -> Result<(), Failed> { + if let Some(requests) = self + .subgraphs_server + .as_ref() + .unwrap() + .received_requests() + .await + { + writeln!(out, "Will reload config, subgraphs received requests:").unwrap(); + for request in requests { + writeln!(out, "\tmethod: {}", request.method).unwrap(); + writeln!(out, "\tpath: {}", request.url).unwrap(); + writeln!(out, "\t{}\n", std::str::from_utf8(&request.body).unwrap()).unwrap(); + } + } else { + writeln!(out, "subgraphs received no requests").unwrap(); + } let mut subgraphs_server = match self.subgraphs_server.take() { Some(subgraphs_server) => subgraphs_server, None => self.start_subgraphs(out).await.0, From 3d628280a0c92981c731b46768eda40449bf4ee9 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Wed, 30 Oct 2024 14:45:25 +0100 Subject: [PATCH 12/19] prep release: v1.57.1-rc.0 --- Cargo.lock | 8 ++++---- apollo-federation/Cargo.toml | 2 +- apollo-router-benchmarks/Cargo.toml | 2 +- apollo-router-scaffold/Cargo.toml | 2 +- apollo-router-scaffold/templates/base/Cargo.template.toml | 2 +- .../templates/base/xtask/Cargo.template.toml | 2 +- apollo-router/Cargo.toml | 4 ++-- dockerfiles/tracing/docker-compose.datadog.yml | 2 +- dockerfiles/tracing/docker-compose.jaeger.yml | 2 +- dockerfiles/tracing/docker-compose.zipkin.yml | 2 +- helm/chart/router/Chart.yaml | 4 ++-- helm/chart/router/README.md | 6 +++--- scripts/install.sh | 2 +- 13 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 58595d4317..eb2db1dd1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -178,7 +178,7 @@ dependencies = [ [[package]] name = "apollo-federation" -version = "1.57.0" +version = "1.57.1-rc.0" dependencies = [ "apollo-compiler", "derive_more", @@ -229,7 +229,7 @@ dependencies = [ [[package]] name = "apollo-router" -version = "1.57.0" +version = "1.57.1-rc.0" dependencies = [ "access-json", "ahash", @@ -397,7 +397,7 @@ dependencies = [ [[package]] name = "apollo-router-benchmarks" -version = "1.57.0" +version = "1.57.1-rc.0" dependencies = [ "apollo-parser", "apollo-router", @@ -413,7 +413,7 @@ dependencies = [ [[package]] name = "apollo-router-scaffold" -version = "1.57.0" +version = "1.57.1-rc.0" dependencies = [ "anyhow", "cargo-scaffold", diff --git a/apollo-federation/Cargo.toml b/apollo-federation/Cargo.toml index 56def7c08c..a0ae61368f 100644 --- a/apollo-federation/Cargo.toml +++ b/apollo-federation/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "apollo-federation" -version = "1.57.0" +version = "1.57.1-rc.0" authors = ["The Apollo GraphQL Contributors"] edition = "2021" description = "Apollo Federation" diff --git a/apollo-router-benchmarks/Cargo.toml b/apollo-router-benchmarks/Cargo.toml index ad2cd569c7..f12ef5d6ce 100644 --- a/apollo-router-benchmarks/Cargo.toml +++ b/apollo-router-benchmarks/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "apollo-router-benchmarks" -version = "1.57.0" +version = "1.57.1-rc.0" authors = ["Apollo Graph, Inc. "] edition = "2021" license = "Elastic-2.0" diff --git a/apollo-router-scaffold/Cargo.toml b/apollo-router-scaffold/Cargo.toml index a598f6f5ee..ea717e4c6b 100644 --- a/apollo-router-scaffold/Cargo.toml +++ b/apollo-router-scaffold/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "apollo-router-scaffold" -version = "1.57.0" +version = "1.57.1-rc.0" authors = ["Apollo Graph, Inc. "] edition = "2021" license = "Elastic-2.0" diff --git a/apollo-router-scaffold/templates/base/Cargo.template.toml b/apollo-router-scaffold/templates/base/Cargo.template.toml index 3855918e82..daacc01507 100644 --- a/apollo-router-scaffold/templates/base/Cargo.template.toml +++ b/apollo-router-scaffold/templates/base/Cargo.template.toml @@ -22,7 +22,7 @@ apollo-router = { path ="{{integration_test}}apollo-router" } apollo-router = { git="https://github.com/apollographql/router.git", branch="{{branch}}" } {{else}} # Note if you update these dependencies then also update xtask/Cargo.toml -apollo-router = "1.57.0" +apollo-router = "1.57.1-rc.0" {{/if}} {{/if}} async-trait = "0.1.52" diff --git a/apollo-router-scaffold/templates/base/xtask/Cargo.template.toml b/apollo-router-scaffold/templates/base/xtask/Cargo.template.toml index 0c164855ff..70403df5ee 100644 --- a/apollo-router-scaffold/templates/base/xtask/Cargo.template.toml +++ b/apollo-router-scaffold/templates/base/xtask/Cargo.template.toml @@ -13,7 +13,7 @@ apollo-router-scaffold = { path ="{{integration_test}}apollo-router-scaffold" } {{#if branch}} apollo-router-scaffold = { git="https://github.com/apollographql/router.git", branch="{{branch}}" } {{else}} -apollo-router-scaffold = { git = "https://github.com/apollographql/router.git", tag = "v1.57.0" } +apollo-router-scaffold = { git = "https://github.com/apollographql/router.git", tag = "v1.57.1-rc.0" } {{/if}} {{/if}} anyhow = "1.0.58" diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index 020e624ef9..0ab9b7bd0b 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "apollo-router" -version = "1.57.0" +version = "1.57.1-rc.0" authors = ["Apollo Graph, Inc. "] repository = "https://github.com/apollographql/router/" documentation = "https://docs.rs/apollo-router" @@ -62,7 +62,7 @@ features = ["docs_rs"] access-json = "0.1.0" anyhow = "1.0.86" apollo-compiler.workspace = true -apollo-federation = { path = "../apollo-federation", version = "=1.57.0" } +apollo-federation = { path = "../apollo-federation", version = "=1.57.1-rc.0" } arc-swap = "1.6.0" async-channel = "1.9.0" async-compression = { version = "0.4.6", features = [ diff --git a/dockerfiles/tracing/docker-compose.datadog.yml b/dockerfiles/tracing/docker-compose.datadog.yml index c6b46fcd7e..68df74f969 100644 --- a/dockerfiles/tracing/docker-compose.datadog.yml +++ b/dockerfiles/tracing/docker-compose.datadog.yml @@ -3,7 +3,7 @@ services: apollo-router: container_name: apollo-router - image: ghcr.io/apollographql/router:v1.57.0 + image: ghcr.io/apollographql/router:v1.57.1-rc.0 volumes: - ./supergraph.graphql:/etc/config/supergraph.graphql - ./router/datadog.router.yaml:/etc/config/configuration.yaml diff --git a/dockerfiles/tracing/docker-compose.jaeger.yml b/dockerfiles/tracing/docker-compose.jaeger.yml index 71d4e88301..a4974f8290 100644 --- a/dockerfiles/tracing/docker-compose.jaeger.yml +++ b/dockerfiles/tracing/docker-compose.jaeger.yml @@ -4,7 +4,7 @@ services: apollo-router: container_name: apollo-router #build: ./router - image: ghcr.io/apollographql/router:v1.57.0 + image: ghcr.io/apollographql/router:v1.57.1-rc.0 volumes: - ./supergraph.graphql:/etc/config/supergraph.graphql - ./router/jaeger.router.yaml:/etc/config/configuration.yaml diff --git a/dockerfiles/tracing/docker-compose.zipkin.yml b/dockerfiles/tracing/docker-compose.zipkin.yml index 2e6eea86ba..9b40dc6ff5 100644 --- a/dockerfiles/tracing/docker-compose.zipkin.yml +++ b/dockerfiles/tracing/docker-compose.zipkin.yml @@ -4,7 +4,7 @@ services: apollo-router: container_name: apollo-router build: ./router - image: ghcr.io/apollographql/router:v1.57.0 + image: ghcr.io/apollographql/router:v1.57.1-rc.0 volumes: - ./supergraph.graphql:/etc/config/supergraph.graphql - ./router/zipkin.router.yaml:/etc/config/configuration.yaml diff --git a/helm/chart/router/Chart.yaml b/helm/chart/router/Chart.yaml index adfef3761f..4a61fad443 100644 --- a/helm/chart/router/Chart.yaml +++ b/helm/chart/router/Chart.yaml @@ -20,10 +20,10 @@ type: application # so it matches the shape of our release process and release automation. # By proxy of that decision, this version uses SemVer 2.0.0, though the prefix # of "v" is not included. -version: 1.57.0 +version: 1.57.1-rc.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: "v1.57.0" +appVersion: "v1.57.1-rc.0" diff --git a/helm/chart/router/README.md b/helm/chart/router/README.md index 44c483d39b..ac6739b199 100644 --- a/helm/chart/router/README.md +++ b/helm/chart/router/README.md @@ -2,7 +2,7 @@ [router](https://github.com/apollographql/router) Rust Graph Routing runtime for Apollo Federation -![Version: 1.57.0](https://img.shields.io/badge/Version-1.57.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v1.57.0](https://img.shields.io/badge/AppVersion-v1.57.0-informational?style=flat-square) +![Version: 1.57.1-rc.0](https://img.shields.io/badge/Version-1.57.1--rc.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v1.57.1-rc.0](https://img.shields.io/badge/AppVersion-v1.57.1--rc.0-informational?style=flat-square) ## Prerequisites @@ -11,7 +11,7 @@ ## Get Repo Info ```console -helm pull oci://ghcr.io/apollographql/helm-charts/router --version 1.57.0 +helm pull oci://ghcr.io/apollographql/helm-charts/router --version 1.57.1-rc.0 ``` ## Install Chart @@ -19,7 +19,7 @@ helm pull oci://ghcr.io/apollographql/helm-charts/router --version 1.57.0 **Important:** only helm3 is supported ```console -helm upgrade --install [RELEASE_NAME] oci://ghcr.io/apollographql/helm-charts/router --version 1.57.0 --values my-values.yaml +helm upgrade --install [RELEASE_NAME] oci://ghcr.io/apollographql/helm-charts/router --version 1.57.1-rc.0 --values my-values.yaml ``` _See [configuration](#configuration) below._ diff --git a/scripts/install.sh b/scripts/install.sh index 1123fb1802..3eabe4ac53 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -11,7 +11,7 @@ BINARY_DOWNLOAD_PREFIX="https://github.com/apollographql/router/releases/downloa # Router version defined in apollo-router's Cargo.toml # Note: Change this line manually during the release steps. -PACKAGE_VERSION="v1.57.0" +PACKAGE_VERSION="v1.57.1-rc.0" download_binary() { downloader --check From f8e9a5200e282c0fa872780e75aca88b723cd42b Mon Sep 17 00:00:00 2001 From: Coenen Benjamin Date: Wed, 30 Oct 2024 16:39:42 +0100 Subject: [PATCH 13/19] chore(telemetry): don't create a stub span for supergraph events if it already has a current span (#6096) Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com> --- .changesets/maint_bnjjj_fix_supergraph_events_span.md | 5 +++++ .../src/plugins/telemetry/config_new/events.rs | 10 +++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 .changesets/maint_bnjjj_fix_supergraph_events_span.md diff --git a/.changesets/maint_bnjjj_fix_supergraph_events_span.md b/.changesets/maint_bnjjj_fix_supergraph_events_span.md new file mode 100644 index 0000000000..00f5cf241d --- /dev/null +++ b/.changesets/maint_bnjjj_fix_supergraph_events_span.md @@ -0,0 +1,5 @@ +### Don't create a stub span for supergraph events if it already has a current span ([PR #6096](https://github.com/apollographql/router/pull/6096)) + +Don't create useless span when we already have a span available to use the span's extensions. + +By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/6096 \ No newline at end of file diff --git a/apollo-router/src/plugins/telemetry/config_new/events.rs b/apollo-router/src/plugins/telemetry/config_new/events.rs index a58264bfb7..c5fac133a2 100644 --- a/apollo-router/src/plugins/telemetry/config_new/events.rs +++ b/apollo-router/src/plugins/telemetry/config_new/events.rs @@ -741,9 +741,13 @@ where } // Stub span to make sure the custom attributes are saved in current span extensions // It won't be extracted or sampled at all - let span = info_span!("supergraph_event_send_event"); - let _entered = span.enter(); - inner.send_event(attributes); + if Span::current().is_none() { + let span = info_span!("supergraph_event_send_event"); + let _entered = span.enter(); + inner.send_event(attributes); + } else { + inner.send_event(attributes); + } } fn on_error(&self, error: &BoxError, ctx: &Context) { From bc7472e06b6d8ace64d4f4fbd6607da9d0e2674a Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 31 Oct 2024 15:34:25 +0100 Subject: [PATCH 14/19] do not count the wait time in deduplication as processing time (#6207) --- .changesets/fix_geal_deduplication_processing_time.md | 5 +++++ apollo-router/src/plugins/traffic_shaping/deduplication.rs | 1 + 2 files changed, 6 insertions(+) create mode 100644 .changesets/fix_geal_deduplication_processing_time.md diff --git a/.changesets/fix_geal_deduplication_processing_time.md b/.changesets/fix_geal_deduplication_processing_time.md new file mode 100644 index 0000000000..a3a75c467e --- /dev/null +++ b/.changesets/fix_geal_deduplication_processing_time.md @@ -0,0 +1,5 @@ +### do not count the wait time in deduplication as processing time ([PR #6207](https://github.com/apollographql/router/pull/6207)) + +waiting for a deduplicated request was incorrectly counted as time spent in the router overhead, while most of it was actually spent waiting for the subgraph response. + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/6207 \ No newline at end of file diff --git a/apollo-router/src/plugins/traffic_shaping/deduplication.rs b/apollo-router/src/plugins/traffic_shaping/deduplication.rs index 0df2574eb4..5c2165f775 100644 --- a/apollo-router/src/plugins/traffic_shaping/deduplication.rs +++ b/apollo-router/src/plugins/traffic_shaping/deduplication.rs @@ -98,6 +98,7 @@ where let mut receiver = waiter.subscribe(); drop(locked_wait_map); + let _guard = request.context.enter_active_request(); match receiver.recv().await { Ok(value) => { return value From 6f65fa4913e84e9c3113791a358cb00dc2b689e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e?= Date: Mon, 4 Nov 2024 08:58:48 +0000 Subject: [PATCH 15/19] Remove redundant clone (#6218) --- apollo-router/src/services/layers/query_analysis.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/src/services/layers/query_analysis.rs b/apollo-router/src/services/layers/query_analysis.rs index ce008c0a0f..4af57a5f38 100644 --- a/apollo-router/src/services/layers/query_analysis.rs +++ b/apollo-router/src/services/layers/query_analysis.rs @@ -205,7 +205,7 @@ impl QueryAnalysisLayer { doc.executable.clone(), op_name, self.schema.api_schema(), - &request.supergraph_request.body().variables.clone(), + &request.supergraph_request.body().variables, )) } else { None From 4caf16d4d7813fd4134cc1294b9a4041ffd835c5 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 4 Nov 2024 10:32:54 +0100 Subject: [PATCH 16/19] prep release: v1.57.1 --- .changesets/fix_geal_progressive_override_test.md | 5 ----- CHANGELOG.md | 12 ++++++++++++ Cargo.lock | 8 ++++---- apollo-federation/Cargo.toml | 2 +- apollo-router-benchmarks/Cargo.toml | 2 +- apollo-router-scaffold/Cargo.toml | 2 +- .../templates/base/Cargo.template.toml | 2 +- .../templates/base/xtask/Cargo.template.toml | 2 +- apollo-router/Cargo.toml | 4 ++-- dockerfiles/tracing/docker-compose.datadog.yml | 2 +- dockerfiles/tracing/docker-compose.jaeger.yml | 2 +- dockerfiles/tracing/docker-compose.zipkin.yml | 2 +- helm/chart/router/Chart.yaml | 4 ++-- helm/chart/router/README.md | 6 +++--- scripts/install.sh | 2 +- 15 files changed, 32 insertions(+), 25 deletions(-) delete mode 100644 .changesets/fix_geal_progressive_override_test.md diff --git a/.changesets/fix_geal_progressive_override_test.md b/.changesets/fix_geal_progressive_override_test.md deleted file mode 100644 index eea5a214ab..0000000000 --- a/.changesets/fix_geal_progressive_override_test.md +++ /dev/null @@ -1,5 +0,0 @@ -### Progressive override: fix query planner cache warmup ([PR #6108](https://github.com/apollographql/router/pull/6108)) - -This fixes an issue in progressive override where the override labels would not be transmitted to the query planner during cache warmup, resulting in queries correctly using the overridden fields at first, but after an update, would revert to non overridden fields, and could not recover. - -By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/6108 \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index adc42690e4..bb4866bf62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to Router will be documented in this file. This project adheres to [Semantic Versioning v2.0.0](https://semver.org/spec/v2.0.0.html). +# [1.57.1] - 2024-10-31 + +## 🐛 Fixes + +### Progressive override: fix query planner cache warmup ([PR #6108](https://github.com/apollographql/router/pull/6108)) + +This fixes an issue in progressive override where the override labels were not transmitted to the query planner during cache warmup. Queries were correctly using the overridden fields at first, but after an update, reverted to non overridden fields, and could not recover. + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/6108 + + + # [1.57.0] - 2024-10-22 > [!IMPORTANT] diff --git a/Cargo.lock b/Cargo.lock index eb2db1dd1f..c97f22a4b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -178,7 +178,7 @@ dependencies = [ [[package]] name = "apollo-federation" -version = "1.57.1-rc.0" +version = "1.57.1" dependencies = [ "apollo-compiler", "derive_more", @@ -229,7 +229,7 @@ dependencies = [ [[package]] name = "apollo-router" -version = "1.57.1-rc.0" +version = "1.57.1" dependencies = [ "access-json", "ahash", @@ -397,7 +397,7 @@ dependencies = [ [[package]] name = "apollo-router-benchmarks" -version = "1.57.1-rc.0" +version = "1.57.1" dependencies = [ "apollo-parser", "apollo-router", @@ -413,7 +413,7 @@ dependencies = [ [[package]] name = "apollo-router-scaffold" -version = "1.57.1-rc.0" +version = "1.57.1" dependencies = [ "anyhow", "cargo-scaffold", diff --git a/apollo-federation/Cargo.toml b/apollo-federation/Cargo.toml index a0ae61368f..ce77a6af9e 100644 --- a/apollo-federation/Cargo.toml +++ b/apollo-federation/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "apollo-federation" -version = "1.57.1-rc.0" +version = "1.57.1" authors = ["The Apollo GraphQL Contributors"] edition = "2021" description = "Apollo Federation" diff --git a/apollo-router-benchmarks/Cargo.toml b/apollo-router-benchmarks/Cargo.toml index f12ef5d6ce..d26748c696 100644 --- a/apollo-router-benchmarks/Cargo.toml +++ b/apollo-router-benchmarks/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "apollo-router-benchmarks" -version = "1.57.1-rc.0" +version = "1.57.1" authors = ["Apollo Graph, Inc. "] edition = "2021" license = "Elastic-2.0" diff --git a/apollo-router-scaffold/Cargo.toml b/apollo-router-scaffold/Cargo.toml index ea717e4c6b..63bf887d7f 100644 --- a/apollo-router-scaffold/Cargo.toml +++ b/apollo-router-scaffold/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "apollo-router-scaffold" -version = "1.57.1-rc.0" +version = "1.57.1" authors = ["Apollo Graph, Inc. "] edition = "2021" license = "Elastic-2.0" diff --git a/apollo-router-scaffold/templates/base/Cargo.template.toml b/apollo-router-scaffold/templates/base/Cargo.template.toml index daacc01507..6b9873bf75 100644 --- a/apollo-router-scaffold/templates/base/Cargo.template.toml +++ b/apollo-router-scaffold/templates/base/Cargo.template.toml @@ -22,7 +22,7 @@ apollo-router = { path ="{{integration_test}}apollo-router" } apollo-router = { git="https://github.com/apollographql/router.git", branch="{{branch}}" } {{else}} # Note if you update these dependencies then also update xtask/Cargo.toml -apollo-router = "1.57.1-rc.0" +apollo-router = "1.57.1" {{/if}} {{/if}} async-trait = "0.1.52" diff --git a/apollo-router-scaffold/templates/base/xtask/Cargo.template.toml b/apollo-router-scaffold/templates/base/xtask/Cargo.template.toml index 70403df5ee..f58bd86237 100644 --- a/apollo-router-scaffold/templates/base/xtask/Cargo.template.toml +++ b/apollo-router-scaffold/templates/base/xtask/Cargo.template.toml @@ -13,7 +13,7 @@ apollo-router-scaffold = { path ="{{integration_test}}apollo-router-scaffold" } {{#if branch}} apollo-router-scaffold = { git="https://github.com/apollographql/router.git", branch="{{branch}}" } {{else}} -apollo-router-scaffold = { git = "https://github.com/apollographql/router.git", tag = "v1.57.1-rc.0" } +apollo-router-scaffold = { git = "https://github.com/apollographql/router.git", tag = "v1.57.1" } {{/if}} {{/if}} anyhow = "1.0.58" diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index 0ab9b7bd0b..ced84ad29f 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "apollo-router" -version = "1.57.1-rc.0" +version = "1.57.1" authors = ["Apollo Graph, Inc. "] repository = "https://github.com/apollographql/router/" documentation = "https://docs.rs/apollo-router" @@ -62,7 +62,7 @@ features = ["docs_rs"] access-json = "0.1.0" anyhow = "1.0.86" apollo-compiler.workspace = true -apollo-federation = { path = "../apollo-federation", version = "=1.57.1-rc.0" } +apollo-federation = { path = "../apollo-federation", version = "=1.57.1" } arc-swap = "1.6.0" async-channel = "1.9.0" async-compression = { version = "0.4.6", features = [ diff --git a/dockerfiles/tracing/docker-compose.datadog.yml b/dockerfiles/tracing/docker-compose.datadog.yml index 68df74f969..db46f878fb 100644 --- a/dockerfiles/tracing/docker-compose.datadog.yml +++ b/dockerfiles/tracing/docker-compose.datadog.yml @@ -3,7 +3,7 @@ services: apollo-router: container_name: apollo-router - image: ghcr.io/apollographql/router:v1.57.1-rc.0 + image: ghcr.io/apollographql/router:v1.57.1 volumes: - ./supergraph.graphql:/etc/config/supergraph.graphql - ./router/datadog.router.yaml:/etc/config/configuration.yaml diff --git a/dockerfiles/tracing/docker-compose.jaeger.yml b/dockerfiles/tracing/docker-compose.jaeger.yml index a4974f8290..a38d04b6ba 100644 --- a/dockerfiles/tracing/docker-compose.jaeger.yml +++ b/dockerfiles/tracing/docker-compose.jaeger.yml @@ -4,7 +4,7 @@ services: apollo-router: container_name: apollo-router #build: ./router - image: ghcr.io/apollographql/router:v1.57.1-rc.0 + image: ghcr.io/apollographql/router:v1.57.1 volumes: - ./supergraph.graphql:/etc/config/supergraph.graphql - ./router/jaeger.router.yaml:/etc/config/configuration.yaml diff --git a/dockerfiles/tracing/docker-compose.zipkin.yml b/dockerfiles/tracing/docker-compose.zipkin.yml index 9b40dc6ff5..366bff5506 100644 --- a/dockerfiles/tracing/docker-compose.zipkin.yml +++ b/dockerfiles/tracing/docker-compose.zipkin.yml @@ -4,7 +4,7 @@ services: apollo-router: container_name: apollo-router build: ./router - image: ghcr.io/apollographql/router:v1.57.1-rc.0 + image: ghcr.io/apollographql/router:v1.57.1 volumes: - ./supergraph.graphql:/etc/config/supergraph.graphql - ./router/zipkin.router.yaml:/etc/config/configuration.yaml diff --git a/helm/chart/router/Chart.yaml b/helm/chart/router/Chart.yaml index 4a61fad443..9d7c28d3d0 100644 --- a/helm/chart/router/Chart.yaml +++ b/helm/chart/router/Chart.yaml @@ -20,10 +20,10 @@ type: application # so it matches the shape of our release process and release automation. # By proxy of that decision, this version uses SemVer 2.0.0, though the prefix # of "v" is not included. -version: 1.57.1-rc.0 +version: 1.57.1 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: "v1.57.1-rc.0" +appVersion: "v1.57.1" diff --git a/helm/chart/router/README.md b/helm/chart/router/README.md index ac6739b199..6dd2aaea77 100644 --- a/helm/chart/router/README.md +++ b/helm/chart/router/README.md @@ -2,7 +2,7 @@ [router](https://github.com/apollographql/router) Rust Graph Routing runtime for Apollo Federation -![Version: 1.57.1-rc.0](https://img.shields.io/badge/Version-1.57.1--rc.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v1.57.1-rc.0](https://img.shields.io/badge/AppVersion-v1.57.1--rc.0-informational?style=flat-square) +![Version: 1.57.1](https://img.shields.io/badge/Version-1.57.1-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v1.57.1](https://img.shields.io/badge/AppVersion-v1.57.1-informational?style=flat-square) ## Prerequisites @@ -11,7 +11,7 @@ ## Get Repo Info ```console -helm pull oci://ghcr.io/apollographql/helm-charts/router --version 1.57.1-rc.0 +helm pull oci://ghcr.io/apollographql/helm-charts/router --version 1.57.1 ``` ## Install Chart @@ -19,7 +19,7 @@ helm pull oci://ghcr.io/apollographql/helm-charts/router --version 1.57.1-rc.0 **Important:** only helm3 is supported ```console -helm upgrade --install [RELEASE_NAME] oci://ghcr.io/apollographql/helm-charts/router --version 1.57.1-rc.0 --values my-values.yaml +helm upgrade --install [RELEASE_NAME] oci://ghcr.io/apollographql/helm-charts/router --version 1.57.1 --values my-values.yaml ``` _See [configuration](#configuration) below._ diff --git a/scripts/install.sh b/scripts/install.sh index 3eabe4ac53..a542864a9a 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -11,7 +11,7 @@ BINARY_DOWNLOAD_PREFIX="https://github.com/apollographql/router/releases/downloa # Router version defined in apollo-router's Cargo.toml # Note: Change this line manually during the release steps. -PACKAGE_VERSION="v1.57.1-rc.0" +PACKAGE_VERSION="v1.57.1" download_binary() { downloader --check From 284af97f05c4b0f18b13114dbfecd940ccb12c8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e?= Date: Tue, 5 Nov 2024 12:50:56 +0000 Subject: [PATCH 17/19] chore(federation): update outdated comments in QueryPlannerConfig (#6229) --- .../src/query_plan/query_planner.rs | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/apollo-federation/src/query_plan/query_planner.rs b/apollo-federation/src/query_plan/query_planner.rs index 24e0972944..1a3f1002f1 100644 --- a/apollo-federation/src/query_plan/query_planner.rs +++ b/apollo-federation/src/query_plan/query_planner.rs @@ -56,31 +56,26 @@ pub(crate) const CONTEXT_DIRECTIVE: &str = "context"; #[derive(Debug, Clone, Hash)] pub struct QueryPlannerConfig { - /// Whether the query planner should try to reused the named fragments of the planned query in + /// Whether the query planner should try to reuse the named fragments of the planned query in /// subgraph fetches. /// - /// This is often a good idea as it can prevent very large subgraph queries in some cases (named - /// fragments can make some relatively small queries (using said fragments) expand to a very large - /// query if all the spreads are inline). However, due to architecture of the query planner, this - /// optimization is done as an additional pass on the subgraph queries of the generated plan and - /// can thus increase the latency of building a plan. As long as query plans are sufficiently - /// cached, this should not be a problem, which is why this option is enabled by default, but if - /// the distribution of inbound queries prevents efficient caching of query plans, this may become - /// an undesirable trade-off and can be disabled in that case. + /// Reusing fragments requires complicated validations, so it can take a long time on large + /// queries with many fragments. This option may be removed in the future in favour of + /// [`generate_query_fragments`][QueryPlannerConfig::generate_query_fragments]. /// /// Defaults to true. pub reuse_query_fragments: bool, - /// NOTE: **not implemented yet** - /// /// If enabled, the query planner will extract inline fragments into fragment /// definitions before sending queries to subgraphs. This can significantly - /// reduce the size of the query sent to subgraphs, but may increase the time - /// it takes to plan the query. + /// reduce the size of the query sent to subgraphs. /// /// Defaults to false. pub generate_query_fragments: bool, + /// **TODO:** This option is not implemented, and the behaviour is *always enabled*. + /// + /// /// Whether to run GraphQL validation against the extracted subgraph schemas. Recommended in /// non-production settings or when debugging. /// @@ -112,8 +107,8 @@ impl Default for QueryPlannerConfig { fn default() -> Self { Self { reuse_query_fragments: true, - subgraph_graphql_validation: false, generate_query_fragments: false, + subgraph_graphql_validation: false, incremental_delivery: Default::default(), debug: Default::default(), type_conditioned_fetching: Default::default(), @@ -123,12 +118,15 @@ impl Default for QueryPlannerConfig { #[derive(Debug, Clone, Default, Hash)] pub struct QueryPlanIncrementalDeliveryConfig { - /// Enables @defer support by the query planner. + /// Enables `@defer` support in the query planner, breaking up the query plan with [DeferNode]s + /// as appropriate. + /// + /// If false, operations with `@defer` are still accepted, but are planned as if they did not + /// contain `@defer` directives. /// - /// If set, then the query plan for queries having some @defer will contains some `DeferNode` - /// (see `query_plan/mod.rs`). + /// Defaults to false. /// - /// Defaults to false (meaning that the @defer are ignored). + /// [DeferNode]: crate::query_plan::DeferNode pub enable_defer: bool, } From a8ba72620ee29a0c729b446be76b5b4cdc544c04 Mon Sep 17 00:00:00 2001 From: Maria Elisabeth Schreiber Date: Tue, 5 Nov 2024 08:57:54 -0700 Subject: [PATCH 18/19] docs: correct authorization directive composition (#6216) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Renée --- ...ify_authorization_directive_composition.md | 5 ++ docs/source/configuration/authorization.mdx | 53 ++++++++++++------- docs/source/errors.mdx | 5 +- 3 files changed, 42 insertions(+), 21 deletions(-) create mode 100644 .changesets/docs_clarify_authorization_directive_composition.md diff --git a/.changesets/docs_clarify_authorization_directive_composition.md b/.changesets/docs_clarify_authorization_directive_composition.md new file mode 100644 index 0000000000..52d0610cc9 --- /dev/null +++ b/.changesets/docs_clarify_authorization_directive_composition.md @@ -0,0 +1,5 @@ +### docs: correct authorization directive composition ([PR #6216](https://github.com/apollographql/router/pull/6216)) + +Make authorization directive composition clearer and correct code examples + +By [@Meschreiber](https://github.com/Meschreiber) in https://github.com/apollographql/router/pull/6216 diff --git a/docs/source/configuration/authorization.mdx b/docs/source/configuration/authorization.mdx index cd241d1d4a..f31e4e4226 100644 --- a/docs/source/configuration/authorization.mdx +++ b/docs/source/configuration/authorization.mdx @@ -688,12 +688,12 @@ When using subscriptions along with `@policy` authorization, subscription events ## Composition and federation -GraphOS's composition strategy for authorization directives is intentionally accumulative. When you define authorization directives on fields and types in subgraphs, GraphOS composes them into the supergraph schema. In other words, if subgraph fields or types include `@requiresScopes`, `@authenticated`, or `@policy` directives, they are set on the supergraph too. +GraphOS's composition strategy for authorization directives is intentionally accumulative. When you define authorization directives on fields and types in subgraphs, GraphOS composes them into the supergraph schema. In other words, if subgraph fields or types include `@requiresScopes`, `@authenticated`, or `@policy` directives, they are set on the supergraph too. Whether composition uses `AND` or `OR` logic depends on how the authorization directives are used. -#### Composition with `AND`/`OR` logic - -If shared subgraph fields include multiple directives, composition merges them. For example, suppose the `me` query requires `@authentication` in one subgraph: +### Composed fields with different authorization directives +If a shared field uses different authorization directives across subgraphs, composition merges them using `AND` logic. +For example, suppose the `me` query requires `@authenticated` in one subgraph and the `read:user` scope in another subgraph: ```graphql title="Subgraph A" type Query { @@ -707,8 +707,6 @@ type User { } ``` -and the `read:user` scope in another subgraph: - ```graphql title="Subgraph B" type Query { me: User @requiresScopes(scopes: [["read:user"]]) @@ -721,9 +719,19 @@ type User { } ``` -A request would need to both be authenticated **AND** have the required scope. Recall that the `@authenticated` directive only checks for the existence of the `apollo_authentication::JWT::claims` key in a request's context, so authentication is guaranteed if the request includes scopes. +A request must both be authenticated **AND** have the required `read:user` scope to succeed. + + + +Recall that the `@authenticated` directive only checks for the existence of the `apollo_authentication::JWT::claims` key in a request's context, so authentication is guaranteed if the request includes scopes. + + + +### Composed fields with the same authorization directives -If multiple shared subgraph fields include `@requiresScopes`, the supergraph schema merges them with the same logic used to [combine scopes for a single use of `@requiresScopes`](#combining-required-scopes-with-andor-logic). For example, if one subgraph requires the `read:others` scope on the `users` query: +If a shared field uses the same authorization directives across subgraphs, composition merges them using `OR` logic. +For example, suppose two subgraphs use the `@requiresScopes` directive on the `users` query. +One subgraph requires the `read:others` scope, and another subgraph requires the `read:profiles` scope: ```graphql title="Subgraph A" type Query { @@ -731,23 +739,32 @@ type Query { } ``` -and another subgraph requires the `read:profiles` scope on `users` query: - ```graphql title="Subgraph B" type Query { users: [User!]! @requiresScopes(scopes: [["read:profiles"]]) } ``` -Then the supergraph schema would require _both_ scopes for it. +A request would need either the `read:others` **OR** the `read:profiles` scope to be authorized. ```graphql title="Supergraph" type Query { - users: [User!]! @requiresScopes(scopes: [["read:others", "read:profiles"]]) + users: [User!]! @requiresScopes(scopes: [["read:others"], ["read:profiles"]]) } ``` -As with [combining scopes for a single use of `@requiresScopes`](#combining-required-scopes-with-andor-logic), you can use nested arrays to introduce **OR** logic: + + +Refer to the section on [Combining policies with AND/OR logic](#combining-policies-with-andor-logic) for a refresher of `@requiresScopes` boolean syntax. + + + +Using **OR** logic for shared directives simplifies schema updates. +If requirements change suddenly, you don't need to update the directive in all subgraphs simultaneously. + +#### Combining `AND`/`OR` logic with `@requiresScopes` + +As with [combining scopes for a single use of [`@requiresScopes`](#combining-required-scopes-with-andor-logic), you can use nested arrays to introduce **AND** logic in a single subgraph: ```graphql title="Subgraph A" type Query { @@ -761,7 +778,7 @@ type Query { } ``` -Since both `scopes` arrays are nested arrays, they would be composed using **OR** logic into the supergraph schema: +Since both subgraphs use the same authorization directive, composition [merges them using **OR** logic](#a-shared-field-with-the-same-authorization-directives-use-or-logic): ```graphql title="Supergraph" type Query { @@ -773,8 +790,8 @@ This syntax means a request needs either (`read:others` **AND** `read:users`) sc ### Authorization and `@key` fields -The [`@key` directive](https://www.apollographql.com/docs/federation/entities/) lets you create an entity whose fields resolve across multiple subgraphs. -If you use authorization directives on fields defined in [`@key` directives](https://www.apollographql.com/docs/federation/entities/), Apollo still uses those fields to compose entities between the subgraphs, but the client cannot query them directly. +The [`@key` directive](/graphos/reference/federation/directives#key) lets you create an entity whose fields resolve across multiple subgraphs. +If you use authorization directives on fields defined in `@key` directives, Apollo still uses those fields to compose entities between the subgraphs, but the client cannot query them directly. Consider these example subgraph schemas: @@ -825,11 +842,11 @@ query { } ``` -This behavior resembles what you can create with [contracts](/graphos/delivery/contracts/) and the [`@inaccessible` directive](https://www.apollographql.com/docs/federation/federated-types/federated-directives/#inaccessible). +This behavior resembles what you can create with [contracts](/graphos/delivery/contracts/) and the [`@inaccessible` directive](/graphos/reference/federation/directives#inaccessible). ### Authorization and interfaces -If a type [implementing an interface](https://www.apollographql.com/docs/apollo-server/schema/unions-interfaces/#interface-type) requires authorization, unauthorized requests can query the interface, but not any parts of the type that require authorization. +If a type [implementing an interface](/apollo-server/schema/unions-interfaces/#interface-type) requires authorization, unauthorized requests can query the interface, but not any parts of the type that require authorization. For example, consider this schema where the `Post` interface doesn't require authentication, but the `PrivateBlog` type, which implements `Post`, does: diff --git a/docs/source/errors.mdx b/docs/source/errors.mdx index 497471d8d9..aa4bd41c88 100644 --- a/docs/source/errors.mdx +++ b/docs/source/errors.mdx @@ -94,14 +94,13 @@ The actual cost of the query was greater than the configured maximum cost. The query could not be parsed. - + The response from a subgraph did not match the GraphQL schema. - - + A subgraph returned a field with a different type that mandated by the GraphQL schema. From 36bdb5e8ee83c4d248d20611dd68be9e9f219e1d Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 5 Nov 2024 17:23:45 +0100 Subject: [PATCH 19/19] Query planner cache key improvements (#6206) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #5160 This splits part of the work from #5255 to make it easier to merge. This covers improvements and fixes to the query planner cache key from changes related to the query hashing algorithm and query plan reuse during warmup. Fixed: * use prefixes for each part of the redis cache key so they become self describing * remove the custom Hash implementation for the cache key * remove JSON serialization * hash the Rust planner's config only once, not on every cache query Co-authored-by: Ivan Goncharov Co-authored-by: Jeremy Lempereur Co-authored-by: Gary Pennington Co-authored-by: Jesse Rosenberger Co-authored-by: Renée --- ...al_query_planner_cache_key_improvements.md | 8 ++ apollo-router/src/configuration/mod.rs | 16 +++ .../query_planner/caching_query_planner.rs | 99 ++++++++++++------- apollo-router/tests/integration/redis.rs | 12 +-- 4 files changed, 94 insertions(+), 41 deletions(-) create mode 100644 .changesets/maint_geal_query_planner_cache_key_improvements.md diff --git a/.changesets/maint_geal_query_planner_cache_key_improvements.md b/.changesets/maint_geal_query_planner_cache_key_improvements.md new file mode 100644 index 0000000000..720836429f --- /dev/null +++ b/.changesets/maint_geal_query_planner_cache_key_improvements.md @@ -0,0 +1,8 @@ +### Query planner cache key improvements ([Issue #5160](https://github.com/apollographql/router/issues/5160)) + +> [!IMPORTANT] +> If you have enabled [Distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), this release changes the hashing algorithm used for the cache keys. On account of this, you should anticipate additional cache regeneration cost when updating between these versions while the new hashing algorithm comes into service. + +This brings several performance improvements to the query plan cache key generation. In particular, it changes the distributed cache's key format, adding prefixes to the different key segments, to help in debugging. + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/6206 \ No newline at end of file diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 00657cc11d..85de05cd8f 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -412,6 +412,22 @@ impl Configuration { type_conditioned_fetching: self.experimental_type_conditioned_fetching, } } + + pub(crate) fn rust_query_planner_config( + &self, + ) -> apollo_federation::query_plan::query_planner::QueryPlannerConfig { + apollo_federation::query_plan::query_planner::QueryPlannerConfig { + reuse_query_fragments: self.supergraph.reuse_query_fragments.unwrap_or(true), + subgraph_graphql_validation: false, + generate_query_fragments: self.supergraph.generate_query_fragments, + incremental_delivery: + apollo_federation::query_plan::query_planner::QueryPlanIncrementalDeliveryConfig { + enable_defer: self.supergraph.defer_support, + }, + type_conditioned_fetching: self.experimental_type_conditioned_fetching, + debug: Default::default(), + } + } } impl Default for Configuration { diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs index 6ef9a671e8..688ce1c697 100644 --- a/apollo-router/src/query_planner/caching_query_planner.rs +++ b/apollo-router/src/query_planner/caching_query_planner.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::hash::Hash; +use std::hash::Hasher; use std::ops::Deref; use std::sync::Arc; use std::task; @@ -14,7 +15,6 @@ use router_bridge::planner::PlanOptions; use router_bridge::planner::Planner; use router_bridge::planner::QueryPlannerConfig; use router_bridge::planner::UsageReporting; -use serde::Serialize; use sha2::Digest; use sha2::Sha256; use tower::BoxError; @@ -57,11 +57,11 @@ pub(crate) type InMemoryCachePlanner = InMemoryCache>>; pub(crate) const APOLLO_OPERATION_ID: &str = "apollo_operation_id"; -#[derive(Debug, Clone, Hash, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, Hash)] pub(crate) enum ConfigMode { //FIXME: add the Rust planner structure once it is hashable and serializable, // for now use the JS config as it expected to be identical to the Rust one - Rust(Arc), + Rust(Arc), Both(Arc), BothBestEffort(Arc), Js(Arc), @@ -80,7 +80,7 @@ pub(crate) struct CachingQueryPlanner { subgraph_schemas: Arc>>>, plugins: Arc, enable_authorization_directives: bool, - config_mode: ConfigMode, + config_mode_hash: Arc, } fn init_query_plan_from_redis( @@ -125,20 +125,34 @@ where let enable_authorization_directives = AuthorizationPlugin::enable_directives(configuration, &schema).unwrap_or(false); - let config_mode = match configuration.experimental_query_planner_mode { + let mut hasher = StructHasher::new(); + match configuration.experimental_query_planner_mode { crate::configuration::QueryPlannerMode::New => { - ConfigMode::Rust(Arc::new(configuration.js_query_planner_config())) + "PLANNER-NEW".hash(&mut hasher); + ConfigMode::Rust(Arc::new(configuration.rust_query_planner_config())) + .hash(&mut hasher); } crate::configuration::QueryPlannerMode::Legacy => { - ConfigMode::Js(Arc::new(configuration.js_query_planner_config())) + "PLANNER-LEGACY".hash(&mut hasher); + ConfigMode::Js(Arc::new(configuration.js_query_planner_config())).hash(&mut hasher); } crate::configuration::QueryPlannerMode::Both => { + "PLANNER-BOTH".hash(&mut hasher); ConfigMode::Both(Arc::new(configuration.js_query_planner_config())) + .hash(&mut hasher); + ConfigMode::Rust(Arc::new(configuration.rust_query_planner_config())) + .hash(&mut hasher); } crate::configuration::QueryPlannerMode::BothBestEffort => { + "PLANNER-BOTH-BEST-EFFORT".hash(&mut hasher); ConfigMode::BothBestEffort(Arc::new(configuration.js_query_planner_config())) + .hash(&mut hasher); + ConfigMode::Rust(Arc::new(configuration.rust_query_planner_config())) + .hash(&mut hasher); } }; + let config_mode_hash = Arc::new(QueryHash(hasher.finalize())); + Ok(Self { cache, delegate, @@ -146,7 +160,7 @@ where subgraph_schemas, plugins: Arc::new(plugins), enable_authorization_directives, - config_mode, + config_mode_hash, }) } @@ -204,7 +218,7 @@ where hash: Some(hash.clone()), metadata: metadata.clone(), plan_options: plan_options.clone(), - config_mode: self.config_mode.clone(), + config_mode: self.config_mode_hash.clone(), }, ) .take(count) @@ -249,7 +263,7 @@ where hash: None, metadata: CacheKeyMetadata::default(), plan_options: PlanOptions::default(), - config_mode: self.config_mode.clone(), + config_mode: self.config_mode_hash.clone(), }); } } @@ -284,7 +298,7 @@ where schema_id: Arc::clone(&self.schema.schema_id), metadata, plan_options, - config_mode: self.config_mode.clone(), + config_mode: self.config_mode_hash.clone(), }; if experimental_reuse_query_plans { @@ -490,7 +504,7 @@ where schema_id: Arc::clone(&self.schema.schema_id), metadata, plan_options, - config_mode: self.config_mode.clone(), + config_mode: self.config_mode_hash.clone(), }; let context = request.context.clone(); @@ -632,7 +646,7 @@ fn stats_report_key_hash(stats_report_key: &str) -> String { hex::encode(result) } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub(crate) struct CachingQueryKey { pub(crate) query: String, pub(crate) schema_id: Arc, @@ -640,12 +654,12 @@ pub(crate) struct CachingQueryKey { pub(crate) hash: Arc, pub(crate) metadata: CacheKeyMetadata, pub(crate) plan_options: PlanOptions, - pub(crate) config_mode: ConfigMode, + pub(crate) config_mode: Arc, } // Update this key every time the cache key or the query plan format has to change. // When changed it MUST BE CALLED OUT PROMINENTLY IN THE CHANGELOG. -const CACHE_KEY_VERSION: usize = 0; +const CACHE_KEY_VERSION: usize = 1; const FEDERATION_VERSION: &str = std::env!("FEDERATION_VERSION"); impl std::fmt::Display for CachingQueryKey { @@ -654,34 +668,23 @@ impl std::fmt::Display for CachingQueryKey { hasher.update(self.operation.as_deref().unwrap_or("-")); let operation = hex::encode(hasher.finalize()); - let mut hasher = Sha256::new(); - hasher.update(serde_json::to_vec(&self.metadata).expect("serialization should not fail")); - hasher - .update(serde_json::to_vec(&self.plan_options).expect("serialization should not fail")); - hasher - .update(serde_json::to_vec(&self.config_mode).expect("serialization should not fail")); - hasher.update(&*self.schema_id); + let mut hasher = StructHasher::new(); + "^metadata".hash(&mut hasher); + self.metadata.hash(&mut hasher); + "^plan_options".hash(&mut hasher); + self.plan_options.hash(&mut hasher); + "^config_mode".hash(&mut hasher); + self.config_mode.hash(&mut hasher); let metadata = hex::encode(hasher.finalize()); write!( f, - "plan:{}:{}:{}:{}:{}", + "plan:cache:{}:federation:{}:{}:opname:{}:metadata:{}", CACHE_KEY_VERSION, FEDERATION_VERSION, self.hash, operation, metadata, ) } } -impl Hash for CachingQueryKey { - fn hash(&self, state: &mut H) { - self.schema_id.hash(state); - self.hash.0.hash(state); - self.operation.hash(state); - self.metadata.hash(state); - self.plan_options.hash(state); - self.config_mode.hash(state); - } -} - #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub(crate) struct WarmUpCachingQueryKey { pub(crate) query: String, @@ -689,7 +692,33 @@ pub(crate) struct WarmUpCachingQueryKey { pub(crate) hash: Option>, pub(crate) metadata: CacheKeyMetadata, pub(crate) plan_options: PlanOptions, - pub(crate) config_mode: ConfigMode, + pub(crate) config_mode: Arc, +} + +struct StructHasher { + hasher: Sha256, +} + +impl StructHasher { + fn new() -> Self { + Self { + hasher: Sha256::new(), + } + } + fn finalize(self) -> Vec { + self.hasher.finalize().as_slice().into() + } +} + +impl Hasher for StructHasher { + fn finish(&self) -> u64 { + unreachable!() + } + + fn write(&mut self, bytes: &[u8]) { + self.hasher.update(&[0xFF][..]); + self.hasher.update(bytes); + } } impl ValueType for Result> { diff --git a/apollo-router/tests/integration/redis.rs b/apollo-router/tests/integration/redis.rs index 9e6af18826..ff42bf8d7e 100644 --- a/apollo-router/tests/integration/redis.rs +++ b/apollo-router/tests/integration/redis.rs @@ -51,7 +51,7 @@ async fn query_planner_cache() -> Result<(), BoxError> { } // If this test fails and the cache key format changed you'll need to update the key here. // Look at the top of the file for instructions on getting the new cache key. - let known_cache_key = "plan:0:v2.9.3:70f115ebba5991355c17f4f56ba25bb093c519c4db49a30f3b10de279a4e3fa4:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:68e167191994b73c1892549ef57d0ec4cd76d518fad4dac5350846fe9af0b3f1"; + let known_cache_key = "plan:cache:1:federation:v2.9.3:70f115ebba5991355c17f4f56ba25bb093c519c4db49a30f3b10de279a4e3fa4:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:0ade8e18db172d9d51b36a2112513c15032d103100644df418a50596de3adfba"; let config = RedisConfig::from_url("redis://127.0.0.1:6379").unwrap(); let client = RedisClient::new(config, None, None, None); @@ -963,7 +963,7 @@ async fn connection_failure_blocks_startup() { async fn query_planner_redis_update_query_fragments() { test_redis_query_plan_config_update( include_str!("fixtures/query_planner_redis_config_update_query_fragments.router.yaml"), - "plan:0:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:d239cf1d493e71f4bcb05e727c38e4cf55b32eb806791fa415bb6f6c8e5352e5", + "plan:cache:1:federation:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:1cfc840090ac76a98f8bd51442f41fd6ca4c8d918b3f8d87894170745acf0734", ) .await; } @@ -993,7 +993,7 @@ async fn query_planner_redis_update_defer() { // test just passes locally. test_redis_query_plan_config_update( include_str!("fixtures/query_planner_redis_config_update_defer.router.yaml"), - "plan:0:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:752b870a0241594f54b7b593f16ab6cf6529eb5c9fe3d24e6bc4a618c24a5b81", + "plan:cache:1:federation:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:2f7fb939d2a8fc978e5a4e9d17998074fc30366dcc673236237a885819084fc0", ) .await; } @@ -1015,7 +1015,7 @@ async fn query_planner_redis_update_type_conditional_fetching() { include_str!( "fixtures/query_planner_redis_config_update_type_conditional_fetching.router.yaml" ), - "plan:0:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:e2145b320a44bebbd687c714dcfd046c032e56fe394aedcf50d9ab539f4354ea", + "plan:cache:1:federation:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:0fd0a376f59f0565768ea5ad8eadfbbf60d64c593c807457a0776d2f39773a25", ) .await; } @@ -1037,7 +1037,7 @@ async fn query_planner_redis_update_reuse_query_fragments() { include_str!( "fixtures/query_planner_redis_config_update_reuse_query_fragments.router.yaml" ), - "plan:0:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:8b6c1838a55cbc6327adb5507f103eed1d5b1071e9acb9c67e098c5b9ea2887e", + "plan:cache:1:federation:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:3f30f0e2d149d00c9370c8046e4dd5f23d6ceb6f05a6cf06d5eb021510564248", ) .await; } @@ -1062,7 +1062,7 @@ async fn test_redis_query_plan_config_update(updated_config: &str, new_cache_key router.clear_redis_cache().await; // If the tests above are failing, this is the key that needs to be changed first. - let starting_key = "plan:0:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:41ae54204ebb1911412cf23e8f1d458cb08d6fabce16f255f7a497fd2b6fe213"; + let starting_key = "plan:cache:1:federation:v2.9.3:e15b4f5cd51b8cc728e3f5171611073455601e81196cd3cbafc5610d9769a370:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:0ade8e18db172d9d51b36a2112513c15032d103100644df418a50596de3adfba"; assert_ne!(starting_key, new_cache_key, "starting_key (cache key for the initial config) and new_cache_key (cache key with the updated config) should not be equal. This either means that the cache key is not being generated correctly, or that the test is not actually checking the updated key."); router.execute_default_query().await;