From f39984dd5c125c1103f64e7c7bae9076a8072958 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Wed, 10 Jul 2024 17:09:09 +0200 Subject: [PATCH 01/10] invalidation by entity key --- apollo-router/src/plugins/cache/entity.rs | 16 +- .../src/plugins/cache/invalidation.rs | 10 + .../invalidation-entity-key/README.md | 6 + .../configuration.yaml | 17 ++ .../invalidation-entity-key/plan.json | 239 ++++++++++++++++++ .../supergraph.graphql | 90 +++++++ 6 files changed, 372 insertions(+), 6 deletions(-) create mode 100644 apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/README.md create mode 100644 apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/configuration.yaml create mode 100644 apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/plan.json create mode 100644 apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/supergraph.graphql diff --git a/apollo-router/src/plugins/cache/entity.rs b/apollo-router/src/plugins/cache/entity.rs index 3562343994..6b0afa4ee6 100644 --- a/apollo-router/src/plugins/cache/entity.rs +++ b/apollo-router/src/plugins/cache/entity.rs @@ -849,7 +849,7 @@ fn extract_cache_key_root( let mut key = String::new(); let _ = write!( &mut key, - "subgraph:{subgraph_name}:Query:{query_hash}:{additional_data_hash}" + "subgraph:{subgraph_name}:type:Query:hash:{query_hash}:data:{additional_data_hash}" ); if is_known_private { @@ -892,10 +892,7 @@ fn extract_cache_keys( let typename = opt_type.as_str().unwrap_or("-"); - // We have to hash the representation because it can contains PII - let mut digest = Sha256::new(); - digest.update(serde_json::to_string(&representation).unwrap().as_bytes()); - let hashed_entity_key = hex::encode(digest.finalize().as_slice()); + let hashed_entity_key = hash_entity_key(&representation); // the cache key is written to easily find keys matching a prefix for deletion: // - subgraph name: caching is done per subgraph @@ -904,7 +901,7 @@ fn extract_cache_keys( // - query hash: invalidate the entry for a specific query and operation name // - additional data: separate cache entries depending on info like authorization status let mut key = String::new(); - let _ = write!(&mut key, "subgraph:{subgraph_name}:{typename}:{hashed_entity_key}:{query_hash}:{additional_data_hash}"); + let _ = write!(&mut key, "subgraph:{subgraph_name}:type:{typename}:entity:{hashed_entity_key}:hash:{query_hash}:data:{additional_data_hash}"); if is_known_private { if let Some(id) = private_id { let _ = write!(&mut key, ":{id}"); @@ -919,6 +916,13 @@ fn extract_cache_keys( Ok(res) } +pub(crate) fn hash_entity_key(representation: &Value) -> String { + // We have to hash the representation because it can contains PII + let mut digest = Sha256::new(); + digest.update(serde_json::to_string(&representation).unwrap().as_bytes()); + hex::encode(digest.finalize().as_slice()) +} + /// represents the result of a cache lookup for an entity type and key struct IntermediateResult { key: String, diff --git a/apollo-router/src/plugins/cache/invalidation.rs b/apollo-router/src/plugins/cache/invalidation.rs index 966914640c..5cb3175c0e 100644 --- a/apollo-router/src/plugins/cache/invalidation.rs +++ b/apollo-router/src/plugins/cache/invalidation.rs @@ -13,6 +13,7 @@ use crate::cache::redis::RedisCacheStorage; use crate::cache::redis::RedisKey; use crate::notification::Handle; use crate::notification::HandleStream; +use crate::plugins::cache::entity::hash_entity_key; use crate::Notify; #[derive(Clone)] @@ -143,6 +144,15 @@ impl InvalidationRequest { InvalidationRequest::Subgraph { subgraph } => { format!("subgraph:{subgraph}*",) } + InvalidationRequest::Entity { + subgraph, + r#type, + key, + } => { + let entity_key = hash_entity_key(&key); + let ty = r#type; + format!("subgraph:{subgraph}:type:{ty}:entity:{entity_key}*") + } _ => { todo!() } diff --git a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/README.md b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/README.md new file mode 100644 index 0000000000..15f004693c --- /dev/null +++ b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/README.md @@ -0,0 +1,6 @@ +# Entity cache invalidation + +This tests entity cache invalidation based on entity keys. This is the expected process: +- a query is sent to the router, for which multiple entities will be requested +- we reload the subgraph with a mock mutation where the response has an extension to invalidate one of the entities +- we do the same query, we should see an `_entities` query that only requests that specific entity \ No newline at end of file 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 new file mode 100644 index 0000000000..b297fee443 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/configuration.yaml @@ -0,0 +1,17 @@ +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 \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/plan.json b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/plan.json new file mode 100644 index 0000000000..b505259570 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/plan.json @@ -0,0 +1,239 @@ +{ + "enterprise": true, + "redis": true, + "actions": [ + { + "type": "Start", + "schema_path": "./supergraph.graphql", + "configuration_path": "./configuration.yaml", + "subgraphs": { + "products": { + "requests": [ + { + "request": { + "body": {"query":"{topProducts{__typename upc}}"} + }, + "response": { + "headers": { + "Cache-Control": "public, max-age=10", + "Content-Type": "application/json" + }, + "body": {"data": { "topProducts": [{ "__typename": "Product", "upc": "0" }, { "__typename": "Product", "upc": "1"} ] } } + } + } + ] + }, + "reviews": { + "requests": [ + { + "request": { + "body": { + "query":"query($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{body}}}}", + "variables":{"representations":[{"upc":"0","__typename":"Product"},{"upc":"1","__typename":"Product"}]} + } + }, + "response": { + "headers": { + "Cache-Control": "public, max-age=10", + "Content-Type": "application/json" + }, + "body": {"data": { "_entities": [ + { + "reviews": [ + { "body": "A"}, + { "body": "B"} + ] + }, + { + "reviews": [ + { "body": "C"} + ] + }] + }} + } + } + ] + } + } + }, + { + "type": "Request", + "request": { + "query": "{ topProducts { reviews { body } } }" + }, + "expected_response": { + "data":{ + "topProducts": [{ + "reviews": [{ + "body": "A" + },{ + "body": "B" + }] + }, + { + "reviews": [{ + "body": "C" + }] + }] + } + } + }, + { + "type": "ReloadSubgraphs", + "subgraphs": { + "reviews": { + "requests": [ + { + "request": { + "body": {"query":"mutation{invalidateProductReview}"} + }, + "response": { + "headers": { + "Content-Type": "application/json" + }, + "body": { + "data": { "invalidateProductReview": 1 }, + "extensions": { + "invalidation": [{ + "kind": "entity", + "subgraph": "reviews", + "type": "Product", + "key": { + "upc": "1" + } + }] + } + } + } + }, + { + "request": { + "body": { + "query":"query($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{body}}}}", + "variables":{"representations":[{"upc":"1","__typename":"Product"}]} + } + }, + "response": { + "status": 500, + "headers": { + "Cache-Control": "public, max-age=10", + "Content-Type": "application/json" + }, + "body": {} + } + } + ] + } + } + }, + { + "type": "Request", + "request": { + "query": "{ topProducts { reviews { body } } }" + }, + "expected_response": { + "data":{ + "topProducts": [{ + "reviews": [{ + "body": "A" + },{ + "body": "B" + }] + }, + { + "reviews": [{ + "body": "C" + }] + }] + } + } + }, + { + "type": "Request", + "request": { + "query": "mutation { invalidateProductReview }" + }, + "expected_response": { + "data":{ + "invalidateProductReview": 1 + } + } + }, + { + "type": "Request", + "request": { + "query": "{ topProducts { reviews { body } } }" + }, + "expected_response":{ + "data":{ + "topProducts":[{"reviews":null},{"reviews":null}] + }, + "errors":[ + { + "message":"HTTP fetch failed from 'reviews': 500: Internal Server Error", + "extensions":{"code":"SUBREQUEST_HTTP_ERROR","service":"reviews","reason":"500: Internal Server Error","http":{"status":500}} + }, + { + "message":"service 'reviews' response was malformed: {}", + "extensions":{"service":"reviews","reason":"{}","code":"SUBREQUEST_MALFORMED_RESPONSE"} + } + ] + } + }, + { + "type": "ReloadSubgraphs", + "subgraphs": { + "reviews": { + "requests": [ + { + "request": { + "body": { + "query":"query($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{body}}}}", + "variables":{"representations":[{"upc":"1","__typename":"Product"}]} + } + }, + "response": { + "headers": { + "Cache-Control": "public, max-age=10", + "Content-Type": "application/json" + }, + "body": {"data": { "_entities": [ + { + "reviews": [ + { "body": "C"} + ] + }] + }} + } + } + ] + } + } + }, + { + "type": "Request", + "request": { + "query": "{ topProducts { reviews { body } } }" + }, + "expected_response": { + "data":{ + "topProducts": [{ + "reviews": [{ + "body": "A" + },{ + "body": "B" + }] + }, + { + "reviews": [{ + "body": "C" + }] + }] + } + } + }, + { + "type": "Stop" + } + ] +} \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/supergraph.graphql b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/supergraph.graphql new file mode 100644 index 0000000000..8f4b1aa05b --- /dev/null +++ b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/supergraph.graphql @@ -0,0 +1,90 @@ + +schema + @core(feature: "https://specs.apollo.dev/core/v0.2"), + @core(feature: "https://specs.apollo.dev/join/v0.1", for: EXECUTION) + @core(feature: "https://specs.apollo.dev/inaccessible/v0.1", for: SECURITY) +{ + query: Query + mutation: Mutation +} + +directive @core(as: String, feature: String!, for: core__Purpose) repeatable on SCHEMA + +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION + +directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE + +directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @tag(name: String!) repeatable on FIELD_DEFINITION | INTERFACE | OBJECT | UNION + +directive @inaccessible on OBJECT | FIELD_DEFINITION | INTERFACE | UNION + +enum core__Purpose { + """ + `EXECUTION` features provide metadata necessary to for operation execution. + """ + EXECUTION + + """ + `SECURITY` features provide metadata necessary to securely resolve fields. + """ + SECURITY +} + +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") +} +type Mutation { + updateMyAccount: User @join__field(graph: ACCOUNTS) + invalidateProductReview: Int @join__field(graph: REVIEWS) +} + +type Product + @join__owner(graph: PRODUCTS) + @join__type(graph: PRODUCTS, key: "upc") + @join__type(graph: INVENTORY, key: "upc") + @join__type(graph: REVIEWS, key: "upc") +{ + inStock: Boolean @join__field(graph: INVENTORY) @tag(name: "private") @inaccessible + name: String @join__field(graph: PRODUCTS) + price: Int @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! @join__field(graph: PRODUCTS) + weight: Int @join__field(graph: PRODUCTS) +} + +type Query { + me: User @join__field(graph: ACCOUNTS) + topProducts(first: Int = 5): [Product] @join__field(graph: PRODUCTS) +} + +type Review + @join__owner(graph: REVIEWS) + @join__type(graph: REVIEWS, key: "id") +{ + author: User @join__field(graph: REVIEWS, provides: "username") + body: String @join__field(graph: REVIEWS) + id: ID! @join__field(graph: REVIEWS) + product: Product @join__field(graph: REVIEWS) +} + +type User + @join__owner(graph: ACCOUNTS) + @join__type(graph: ACCOUNTS, key: "id") + @join__type(graph: REVIEWS, key: "id") +{ + id: ID! @join__field(graph: ACCOUNTS) + name: String @join__field(graph: ACCOUNTS) + reviews: [Review] @join__field(graph: REVIEWS) + username: String @join__field(graph: ACCOUNTS) +} From 56c34c1058edf9dcd72b27b9df70fd4f98951628 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Fri, 19 Jul 2024 13:56:14 +0100 Subject: [PATCH 02/10] disable entity cache entity key invalidation In expectation that it will probably fail and we don't want it to. It wil be re-enabled later. --- .../invalidation-entity-key/{plan.json => skipped.json} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/{plan.json => skipped.json} (100%) diff --git a/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/plan.json b/apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/skipped.json similarity index 100% rename from apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/plan.json rename to apollo-router/tests/samples/enterprise/entity-cache/invalidation-entity-key/skipped.json From 00b62d024f3eba57193edc2106496c64d0169825 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Fri, 19 Jul 2024 14:08:24 +0100 Subject: [PATCH 03/10] We don't need ty, type will do --- apollo-router/src/plugins/cache/invalidation.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apollo-router/src/plugins/cache/invalidation.rs b/apollo-router/src/plugins/cache/invalidation.rs index cea6dcb9bc..8bd48c0dad 100644 --- a/apollo-router/src/plugins/cache/invalidation.rs +++ b/apollo-router/src/plugins/cache/invalidation.rs @@ -191,8 +191,7 @@ impl InvalidationRequest { key, } => { let entity_key = hash_entity_key(&key); - let ty = r#type; - format!("subgraph:{subgraph}:type:{ty}:entity:{entity_key}*") + format!("subgraph:{subgraph}:type:{type}:entity:{entity_key}*") } } } From d15e7beeae3bb65993ee6c98047e99f765438c02 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Fri, 19 Jul 2024 14:09:47 +0100 Subject: [PATCH 04/10] Fix the lint errors --- apollo-router/src/plugins/cache/entity.rs | 2 +- apollo-router/src/plugins/cache/invalidation.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/plugins/cache/entity.rs b/apollo-router/src/plugins/cache/entity.rs index b19c38e18b..10547906e9 100644 --- a/apollo-router/src/plugins/cache/entity.rs +++ b/apollo-router/src/plugins/cache/entity.rs @@ -960,7 +960,7 @@ fn extract_cache_keys( let typename = opt_type.as_str().unwrap_or("-"); - let hashed_entity_key = hash_entity_key(&representation); + let hashed_entity_key = hash_entity_key(representation); // the cache key is written to easily find keys matching a prefix for deletion: // - subgraph name: caching is done per subgraph diff --git a/apollo-router/src/plugins/cache/invalidation.rs b/apollo-router/src/plugins/cache/invalidation.rs index 8bd48c0dad..60840ac907 100644 --- a/apollo-router/src/plugins/cache/invalidation.rs +++ b/apollo-router/src/plugins/cache/invalidation.rs @@ -190,7 +190,7 @@ impl InvalidationRequest { r#type, key, } => { - let entity_key = hash_entity_key(&key); + let entity_key = hash_entity_key(key); format!("subgraph:{subgraph}:type:{type}:entity:{entity_key}*") } } From e4990b74af93c8ca949b940c5fe1a26388c17d06 Mon Sep 17 00:00:00 2001 From: Jon Christiansen <467023+theJC@users.noreply.github.com> Date: Mon, 22 Jul 2024 10:23:53 -0500 Subject: [PATCH 05/10] =?UTF-8?q?Support=20router=20configured=20health=5F?= =?UTF-8?q?check=20path=20for=20liveness=20and=20readines=E2=80=A6=20(#565?= =?UTF-8?q?3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jesse Rosenberger Co-authored-by: Gary Pennington --- .changesets/feat_helm_health_check_path.md | 5 +++++ helm/chart/router/templates/deployment.yaml | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 .changesets/feat_helm_health_check_path.md diff --git a/.changesets/feat_helm_health_check_path.md b/.changesets/feat_helm_health_check_path.md new file mode 100644 index 0000000000..264ff0f43a --- /dev/null +++ b/.changesets/feat_helm_health_check_path.md @@ -0,0 +1,5 @@ +### Provide helm support for when router's health_check's default path is not being used([Issue #5652](https://github.com/apollographql/router/issues/5652)) + +When helm chart is defining the liveness and readiness check probes, if the router has been configured to use a non-default health_check path, use that rather than the default ( /health ) + +By [Jon Christiansen](https://github.com/theJC) in https://github.com/apollographql/router/pull/5653 \ No newline at end of file diff --git a/helm/chart/router/templates/deployment.yaml b/helm/chart/router/templates/deployment.yaml index e21fff823e..d6af1a19b4 100644 --- a/helm/chart/router/templates/deployment.yaml +++ b/helm/chart/router/templates/deployment.yaml @@ -114,12 +114,12 @@ spec: {{- toYaml .Values.lifecycle | nindent 12 }} livenessProbe: httpGet: - path: "/health?live" + path: {{ (.Values.router.configuration.health_check.path | default "/health") }}{{"?live"}} port: {{ splitList ":" ((index .Values.router.configuration "health_check").listen | default ":8088") | last }} initialDelaySeconds: {{ ((.Values.probes).liveness).initialDelaySeconds | default 0 }} readinessProbe: httpGet: - path: "/health?ready" + path: {{ (.Values.router.configuration.health_check.path | default "/health") }}{{"?ready"}} port: {{ (splitList ":" ((index .Values.router.configuration "health_check").listen | default ":8088")) | last }} initialDelaySeconds: {{ ((.Values.probes).readiness).initialDelaySeconds | default 0 }} resources: From 2ffa6ddb3a082498475415ac6817c1ffe7b88b64 Mon Sep 17 00:00:00 2001 From: Bryn Cooke Date: Mon, 22 Jul 2024 18:11:45 +0100 Subject: [PATCH 06/10] Datadog sampling priority not set (#5703) Co-authored-by: bryn --- .../fix_bryn_datadog_exporter_span_metrics.md | 2 +- .../datadog_exporter/exporter/model/v05.rs | 8 +--- .../telemetry/tracing/datadog_exporter/mod.rs | 41 ++++--------------- .../tests/integration/telemetry/datadog.rs | 14 +++++++ 4 files changed, 24 insertions(+), 41 deletions(-) diff --git a/.changesets/fix_bryn_datadog_exporter_span_metrics.md b/.changesets/fix_bryn_datadog_exporter_span_metrics.md index c8e01f96ae..9c3d8143b7 100644 --- a/.changesets/fix_bryn_datadog_exporter_span_metrics.md +++ b/.changesets/fix_bryn_datadog_exporter_span_metrics.md @@ -29,4 +29,4 @@ telemetry: my_custom_span: true ``` -By [@BrynCooke](https://github.com/BrynCooke) in https://github.com/apollographql/router/pull/5609 +By [@BrynCooke](https://github.com/BrynCooke) in https://github.com/apollographql/router/pull/5609 and https://github.com/apollographql/router/pull/5703 diff --git a/apollo-router/src/plugins/telemetry/tracing/datadog_exporter/exporter/model/v05.rs b/apollo-router/src/plugins/telemetry/tracing/datadog_exporter/exporter/model/v05.rs index fd1590966e..8cd3f8e66f 100644 --- a/apollo-router/src/plugins/telemetry/tracing/datadog_exporter/exporter/model/v05.rs +++ b/apollo-router/src/plugins/telemetry/tracing/datadog_exporter/exporter/model/v05.rs @@ -128,12 +128,8 @@ fn write_unified_tag<'a>( Ok(()) } -fn get_sampling_priority(span: &SpanData) -> f64 { - if span.span_context.trace_state().priority_sampling_enabled() { - 1.0 - } else { - 0.0 - } +fn get_sampling_priority(_span: &SpanData) -> f64 { + 1.0 } fn get_measuring(span: &SpanData) -> f64 { diff --git a/apollo-router/src/plugins/telemetry/tracing/datadog_exporter/mod.rs b/apollo-router/src/plugins/telemetry/tracing/datadog_exporter/mod.rs index 1c586d48c8..d632eb5872 100644 --- a/apollo-router/src/plugins/telemetry/tracing/datadog_exporter/mod.rs +++ b/apollo-router/src/plugins/telemetry/tracing/datadog_exporter/mod.rs @@ -176,7 +176,7 @@ pub(crate) mod propagator { const DATADOG_SAMPLING_PRIORITY_HEADER: &str = "x-datadog-sampling-priority"; const TRACE_FLAG_DEFERRED: TraceFlags = TraceFlags::new(0x02); - const TRACE_STATE_PRIORITY_SAMPLING: &str = "psr"; + pub(crate) const TRACE_STATE_PRIORITY_SAMPLING: &str = "psr"; pub(crate) const TRACE_STATE_MEASURE: &str = "m"; pub(crate) const TRACE_STATE_TRUE_VALUE: &str = "1"; pub(crate) const TRACE_STATE_FALSE_VALUE: &str = "0"; @@ -243,10 +243,6 @@ pub(crate) mod propagator { fn with_measuring(&self, enabled: bool) -> TraceState; fn measuring_enabled(&self) -> bool; - - fn with_priority_sampling(&self, enabled: bool) -> TraceState; - - fn priority_sampling_enabled(&self) -> bool; } impl DatadogTraceState for TraceState { @@ -260,20 +256,6 @@ pub(crate) mod propagator { .map(trace_flag_to_boolean) .unwrap_or_default() } - - fn with_priority_sampling(&self, enabled: bool) -> TraceState { - self.insert( - TRACE_STATE_PRIORITY_SAMPLING, - boolean_to_trace_state_flag(enabled), - ) - .unwrap_or_else(|_err| self.clone()) - } - - fn priority_sampling_enabled(&self) -> bool { - self.get(TRACE_STATE_PRIORITY_SAMPLING) - .map(trace_flag_to_boolean) - .unwrap_or_default() - } } enum SamplingPriority { @@ -311,16 +293,7 @@ pub(crate) mod propagator { } fn create_trace_state_and_flags(trace_flags: TraceFlags) -> (TraceState, TraceFlags) { - if trace_flags & TRACE_FLAG_DEFERRED == TRACE_FLAG_DEFERRED { - (TraceState::default(), trace_flags) - } else { - ( - DatadogTraceStateBuilder::default() - .with_priority_sampling(trace_flags.is_sampled()) - .build(), - TraceFlags::SAMPLED, - ) - } + (TraceState::default(), trace_flags) } impl DatadogPropagator { @@ -400,7 +373,7 @@ pub(crate) mod propagator { } fn get_sampling_priority(span_context: &SpanContext) -> SamplingPriority { - if span_context.trace_state().priority_sampling_enabled() { + if span_context.is_sampled() { SamplingPriority::AutoKeep } else { SamplingPriority::AutoReject @@ -460,8 +433,8 @@ pub(crate) mod propagator { (vec![(DATADOG_TRACE_ID_HEADER, "garbage")], SpanContext::empty_context()), (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "garbage")], SpanContext::new(TraceId::from_u128(1234), SpanId::INVALID, TRACE_FLAG_DEFERRED, true, TraceState::default())), (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TRACE_FLAG_DEFERRED, true, TraceState::default())), - (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12"), (DATADOG_SAMPLING_PRIORITY_HEADER, "0")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TraceFlags::SAMPLED, true, DatadogTraceStateBuilder::default().with_priority_sampling(false).build())), - (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12"), (DATADOG_SAMPLING_PRIORITY_HEADER, "1")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TraceFlags::SAMPLED, true, DatadogTraceStateBuilder::default().with_priority_sampling(true).build())), + (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12"), (DATADOG_SAMPLING_PRIORITY_HEADER, "0")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TraceFlags::default(), true, TraceState::default())), + (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12"), (DATADOG_SAMPLING_PRIORITY_HEADER, "1")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TraceFlags::SAMPLED, true, TraceState::default())), ] } @@ -473,8 +446,8 @@ pub(crate) mod propagator { (vec![], SpanContext::new(TraceId::from_hex("1234").unwrap(), SpanId::INVALID, TRACE_FLAG_DEFERRED, true, TraceState::default())), (vec![], SpanContext::new(TraceId::from_hex("1234").unwrap(), SpanId::INVALID, TraceFlags::SAMPLED, true, TraceState::default())), (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TRACE_FLAG_DEFERRED, true, TraceState::default())), - (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12"), (DATADOG_SAMPLING_PRIORITY_HEADER, "0")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TraceFlags::SAMPLED, true, DatadogTraceStateBuilder::default().with_priority_sampling(false).build())), - (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12"), (DATADOG_SAMPLING_PRIORITY_HEADER, "1")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TraceFlags::SAMPLED, true, DatadogTraceStateBuilder::default().with_priority_sampling(true).build())), + (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12"), (DATADOG_SAMPLING_PRIORITY_HEADER, "0")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TraceFlags::default(), true, TraceState::default())), + (vec![(DATADOG_TRACE_ID_HEADER, "1234"), (DATADOG_PARENT_ID_HEADER, "12"), (DATADOG_SAMPLING_PRIORITY_HEADER, "1")], SpanContext::new(TraceId::from_u128(1234), SpanId::from_u64(12), TraceFlags::SAMPLED, true, TraceState::default())), ] } diff --git a/apollo-router/tests/integration/telemetry/datadog.rs b/apollo-router/tests/integration/telemetry/datadog.rs index 9a13512e79..613242ca39 100644 --- a/apollo-router/tests/integration/telemetry/datadog.rs +++ b/apollo-router/tests/integration/telemetry/datadog.rs @@ -427,6 +427,7 @@ impl TraceSpec { tracing::debug!("{}", serde_json::to_string_pretty(&trace)?); self.verify_trace_participants(&trace)?; self.verify_operation_name(&trace)?; + self.verify_priority_sampled(&trace)?; self.verify_version(&trace)?; self.verify_spans_present(&trace)?; self.validate_span_kinds(&trace)?; @@ -576,4 +577,17 @@ impl TraceSpec { } Ok(()) } + + fn verify_priority_sampled(&self, trace: &Value) -> Result<(), BoxError> { + let binding = trace.select_path("$.._sampling_priority_v1")?; + let sampling_priority = binding.first(); + assert_eq!( + sampling_priority + .expect("sampling priority expected") + .as_f64() + .expect("sampling priority must be a number"), + 1.0 + ); + Ok(()) + } } From a53b50d1505b0e067c8c7d448d0358450af74927 Mon Sep 17 00:00:00 2001 From: Lenny Burdette Date: Mon, 22 Jul 2024 14:55:15 -0400 Subject: [PATCH 07/10] fix: don't add inaccessible twice (#5705) --- apollo-federation/src/merge.rs | 18 ++++- ...ederation__merge__tests__inaccessible.snap | 38 ++++----- .../expand/merge/inaccessible_2.graphql | 77 +++++++++++++++++++ 3 files changed, 113 insertions(+), 20 deletions(-) create mode 100644 apollo-federation/src/sources/connect/expand/merge/inaccessible_2.graphql diff --git a/apollo-federation/src/merge.rs b/apollo-federation/src/merge.rs index 655402ab03..0aed048c60 100644 --- a/apollo-federation/src/merge.rs +++ b/apollo-federation/src/merge.rs @@ -687,6 +687,9 @@ impl Merger { if original_directives .iter() .any(|d| d.as_ref().name == directive_names.inaccessible) + && !new_directives + .iter() + .any(|d| d.as_ref().name == INACCESSIBLE_DIRECTIVE_NAME_IN_SPEC) { self.needs_inaccessible = true; @@ -1708,14 +1711,25 @@ mod tests { #[test] fn test_inaccessible() { let one_sdl = include_str!("./sources/connect/expand/merge/inaccessible.graphql"); + let two_sdl = include_str!("./sources/connect/expand/merge/inaccessible_2.graphql"); let mut subgraphs = ValidFederationSubgraphs::new(); subgraphs .add(ValidFederationSubgraph { - name: "basic_1".to_string(), + name: "inaccessible".to_string(), url: "".to_string(), schema: ValidFederationSchema::new( - Schema::parse_and_validate(one_sdl, "./basic_1.graphql").unwrap(), + Schema::parse_and_validate(one_sdl, "./inaccessible.graphql").unwrap(), + ) + .unwrap(), + }) + .unwrap(); + subgraphs + .add(ValidFederationSubgraph { + name: "inaccessible_2".to_string(), + url: "".to_string(), + schema: ValidFederationSchema::new( + Schema::parse_and_validate(two_sdl, "./inaccessible_2.graphql").unwrap(), ) .unwrap(), }) diff --git a/apollo-federation/src/snapshots/apollo_federation__merge__tests__inaccessible.snap b/apollo-federation/src/snapshots/apollo_federation__merge__tests__inaccessible.snap index 071ac0883d..29501545a7 100644 --- a/apollo-federation/src/snapshots/apollo_federation__merge__tests__inaccessible.snap +++ b/apollo-federation/src/snapshots/apollo_federation__merge__tests__inaccessible.snap @@ -36,41 +36,43 @@ scalar link__Import scalar join__FieldSet enum join__Graph { - BASIC_1 @join__graph(name: "basic_1", url: "") + INACCESSIBLE @join__graph(name: "inaccessible", url: "") + INACCESSIBLE_2 @join__graph(name: "inaccessible_2", url: "") } -type Query @join__type(graph: BASIC_1) { +type Query @join__type(graph: INACCESSIBLE) @join__type(graph: INACCESSIBLE_2) { a( input: Input @inaccessible, - ): A @join__field(graph: BASIC_1) - b: B @inaccessible @join__field(graph: BASIC_1) + ): A @join__field(graph: INACCESSIBLE) + b: B @inaccessible @join__field(graph: INACCESSIBLE) + as: [A] @inaccessible @join__field(graph: INACCESSIBLE_2) } -type A @join__type(graph: BASIC_1, key: "id") { - id: ID! @join__field(graph: BASIC_1) - c: Int @inaccessible @join__field(graph: BASIC_1) - d: Enum @inaccessible @join__field(graph: BASIC_1) +type A @join__type(graph: INACCESSIBLE, key: "id") @join__type(graph: INACCESSIBLE_2, key: "id") { + id: ID! @join__field(graph: INACCESSIBLE) @join__field(graph: INACCESSIBLE_2) + c: Int @inaccessible @join__field(graph: INACCESSIBLE) @join__field(graph: INACCESSIBLE_2) + d: Enum @inaccessible @join__field(graph: INACCESSIBLE) } -type B implements Interface @join__type(graph: BASIC_1) @inaccessible @join__implements(graph: BASIC_1, interface: "Interface") { - b: Scalar @join__field(graph: BASIC_1) +type B implements Interface @join__type(graph: INACCESSIBLE) @inaccessible @join__implements(graph: INACCESSIBLE, interface: "Interface") { + b: Scalar @join__field(graph: INACCESSIBLE) } -enum Enum @join__type(graph: BASIC_1) @inaccessible { - A @join__enumValue(graph: BASIC_1) - B @join__enumValue(graph: BASIC_1) - C @inaccessible @join__enumValue(graph: BASIC_1) +enum Enum @join__type(graph: INACCESSIBLE) @inaccessible { + A @join__enumValue(graph: INACCESSIBLE) + B @join__enumValue(graph: INACCESSIBLE) + C @inaccessible @join__enumValue(graph: INACCESSIBLE) } -input Input @join__type(graph: BASIC_1) @inaccessible { +input Input @join__type(graph: INACCESSIBLE) @inaccessible { a: Int @inaccessible b: String } -scalar Scalar @join__type(graph: BASIC_1) @inaccessible +scalar Scalar @join__type(graph: INACCESSIBLE) @inaccessible -interface Interface @join__type(graph: BASIC_1) @inaccessible { +interface Interface @join__type(graph: INACCESSIBLE) @inaccessible { b: Scalar } -union Union @join__type(graph: BASIC_1) @inaccessible @join__unionMember(graph: BASIC_1, member: "A") @join__unionMember(graph: BASIC_1, member: "B") = A | B +union Union @join__type(graph: INACCESSIBLE) @inaccessible @join__unionMember(graph: INACCESSIBLE, member: "A") @join__unionMember(graph: INACCESSIBLE, member: "B") = A | B diff --git a/apollo-federation/src/sources/connect/expand/merge/inaccessible_2.graphql b/apollo-federation/src/sources/connect/expand/merge/inaccessible_2.graphql new file mode 100644 index 0000000000..2f8641a81b --- /dev/null +++ b/apollo-federation/src/sources/connect/expand/merge/inaccessible_2.graphql @@ -0,0 +1,77 @@ +schema { + query: Query +} + +extend schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/federation/v2.5") + +directive @link( + url: String + as: String + for: link__Purpose + import: [link__Import] +) repeatable on SCHEMA + +directive @federation__key( + fields: federation__FieldSet! + resolvable: Boolean = true +) repeatable on OBJECT | INTERFACE + +directive @federation__requires( + fields: federation__FieldSet! +) on FIELD_DEFINITION + +directive @federation__provides( + fields: federation__FieldSet! +) on FIELD_DEFINITION + +directive @federation__external(reason: String) on OBJECT | FIELD_DEFINITION + +directive @federation__tag( + name: String! +) repeatable on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION | SCHEMA + +directive @federation__extends on OBJECT | INTERFACE + +directive @federation__shareable on OBJECT | FIELD_DEFINITION + +directive @federation__inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + +directive @federation__override(from: String!) on FIELD_DEFINITION + +directive @federation__composeDirective(name: String) repeatable on SCHEMA + +directive @federation__interfaceObject on OBJECT + +directive @federation__authenticated on FIELD_DEFINITION | OBJECT | INTERFACE | SCALAR | ENUM + +directive @federation__requiresScopes( + scopes: [[federation__Scope!]!]! +) on FIELD_DEFINITION | OBJECT | INTERFACE | SCALAR | ENUM + +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 +} + +scalar federation__FieldSet + +scalar federation__Scope + +type Query { + as: [A] @federation__inaccessible +} + +type A @federation__key(fields: "id") { + id: ID! + c: Int @federation__inaccessible +} From 821124451fa88b174e477e24c5a0a485aef948d7 Mon Sep 17 00:00:00 2001 From: Coenen Benjamin Date: Tue, 23 Jul 2024 14:21:15 +0200 Subject: [PATCH 08/10] add version in the entity cache hash (#5701) Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- .changesets/maint_bnjjj_entity_cache_versioning.md | 6 ++++++ apollo-router/src/plugins/cache/entity.rs | 8 ++++++-- apollo-router/tests/integration/redis.rs | 12 ++++++------ 3 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 .changesets/maint_bnjjj_entity_cache_versioning.md diff --git a/.changesets/maint_bnjjj_entity_cache_versioning.md b/.changesets/maint_bnjjj_entity_cache_versioning.md new file mode 100644 index 0000000000..70ef593913 --- /dev/null +++ b/.changesets/maint_bnjjj_entity_cache_versioning.md @@ -0,0 +1,6 @@ +### Add version in the entity cache hash ([PR #5701](https://github.com/apollographql/router/pull/5701)) + +[!IMPORTANT] +If you have enabled [entity caching](https://www.apollographql.com/docs/router/configuration/entity-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. + +By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/5701 \ No newline at end of file diff --git a/apollo-router/src/plugins/cache/entity.rs b/apollo-router/src/plugins/cache/entity.rs index 10547906e9..2375d4fde4 100644 --- a/apollo-router/src/plugins/cache/entity.rs +++ b/apollo-router/src/plugins/cache/entity.rs @@ -50,6 +50,8 @@ use crate::services::supergraph; use crate::spec::TYPENAME; use crate::Context; +/// Change this key if you introduce a breaking change in entity caching algorithm to make sure it won't take the previous entries +pub(crate) const ENTITY_CACHE_VERSION: &str = "1.0"; pub(crate) const ENTITIES: &str = "_entities"; pub(crate) const REPRESENTATIONS: &str = "representations"; pub(crate) const CONTEXT_CACHE_KEY: &str = "apollo_entity_cache::key"; @@ -910,6 +912,7 @@ fn extract_cache_key_root( let entity_type = entity_type_opt.unwrap_or("Query"); // the cache key is written to easily find keys matching a prefix for deletion: + // - entity cache version: current version of the hash // - subgraph name: subgraph name // - entity type: entity type // - query hash: invalidate the entry for a specific query and operation name @@ -917,7 +920,7 @@ fn extract_cache_key_root( let mut key = String::new(); let _ = write!( &mut key, - "subgraph:{subgraph_name}:type:{entity_type}:hash:{query_hash}:data:{additional_data_hash}" + "version:{ENTITY_CACHE_VERSION}:subgraph:{subgraph_name}:type:{entity_type}:hash:{query_hash}:data:{additional_data_hash}" ); if is_known_private { @@ -963,13 +966,14 @@ fn extract_cache_keys( let hashed_entity_key = hash_entity_key(representation); // the cache key is written to easily find keys matching a prefix for deletion: + // - entity cache version: current version of the hash // - subgraph name: caching is done per subgraph // - type: can invalidate all instances of a type // - entity key: invalidate a specific entity // - query hash: invalidate the entry for a specific query and operation name // - additional data: separate cache entries depending on info like authorization status let mut key = String::new(); - let _ = write!(&mut key, "subgraph:{subgraph_name}:type:{typename}:entity:{hashed_entity_key}:hash:{query_hash}:data:{additional_data_hash}"); + let _ = write!(&mut key, "version:{ENTITY_CACHE_VERSION}:subgraph:{subgraph_name}:type:{typename}:entity:{hashed_entity_key}:hash:{query_hash}:data:{additional_data_hash}"); if is_known_private { if let Some(id) = private_id { let _ = write!(&mut key, ":{id}"); diff --git a/apollo-router/tests/integration/redis.rs b/apollo-router/tests/integration/redis.rs index 7ca25b8fbc..9cf5912c60 100644 --- a/apollo-router/tests/integration/redis.rs +++ b/apollo-router/tests/integration/redis.rs @@ -407,13 +407,13 @@ async fn entity_cache() -> Result<(), BoxError> { insta::assert_json_snapshot!(response); let s:String = client - .get("subgraph:products:type:Query:hash:0df945dc1bc08f7fc02e8905b4c72aa9112f29bb7a214e4a38d199f0aa635b48:data:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("version:1.0:subgraph:products:type:Query:hash:0df945dc1bc08f7fc02e8905b4c72aa9112f29bb7a214e4a38d199f0aa635b48:data:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); insta::assert_json_snapshot!(v.as_object().unwrap().get("data").unwrap()); - let s: String = client.get("subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c").await.unwrap(); + let s: String = client.get("version:1.0:subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c").await.unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); insta::assert_json_snapshot!(v.as_object().unwrap().get("data").unwrap()); @@ -517,7 +517,7 @@ async fn entity_cache() -> Result<(), BoxError> { insta::assert_json_snapshot!(response); let s:String = client - .get("subgraph:reviews:Product:d9a4cd73308dd13ca136390c10340823f94c335b9da198d2339c886c738abf0d:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("version:1.0:subgraph:reviews:Product:d9a4cd73308dd13ca136390c10340823f94c335b9da198d2339c886c738abf0d:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); @@ -734,7 +734,7 @@ async fn entity_cache_authorization() -> Result<(), BoxError> { insta::assert_json_snapshot!(response); let s:String = client - .get("subgraph:products:type:Query:hash:0df945dc1bc08f7fc02e8905b4c72aa9112f29bb7a214e4a38d199f0aa635b48:data:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("version:1.0:subgraph:products:type:Query:hash:0df945dc1bc08f7fc02e8905b4c72aa9112f29bb7a214e4a38d199f0aa635b48:data:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); @@ -755,7 +755,7 @@ async fn entity_cache_authorization() -> Result<(), BoxError> { ); let s: String = client - .get("subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("version:1.0:subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); @@ -799,7 +799,7 @@ async fn entity_cache_authorization() -> Result<(), BoxError> { insta::assert_json_snapshot!(response); let s:String = client - .get("subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:3b6ef3c8fd34c469d59f513942c5f4c8f91135e828712de2024e2cd4613c50ae:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("version:1.0:subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:3b6ef3c8fd34c469d59f513942c5f4c8f91135e828712de2024e2cd4613c50ae:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); From eb6cb39ab9743b48aa8f1a1f86930fbb039e660a Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Tue, 23 Jul 2024 14:43:51 +0100 Subject: [PATCH 09/10] Add support for entity caching version to all invalidation mechanisms --- apollo-router/src/plugins/cache/invalidation.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apollo-router/src/plugins/cache/invalidation.rs b/apollo-router/src/plugins/cache/invalidation.rs index 60840ac907..96c863e437 100644 --- a/apollo-router/src/plugins/cache/invalidation.rs +++ b/apollo-router/src/plugins/cache/invalidation.rs @@ -14,6 +14,7 @@ use crate::cache::redis::RedisKey; use crate::notification::Handle; use crate::notification::HandleStream; use crate::plugins::cache::entity::hash_entity_key; +use crate::plugins::cache::entity::ENTITY_CACHE_VERSION; use crate::Notify; #[derive(Clone)] @@ -180,10 +181,10 @@ impl InvalidationRequest { fn key_prefix(&self) -> String { match self { InvalidationRequest::Subgraph { subgraph } => { - format!("subgraph:{subgraph}*",) + format!("version:{ENTITY_CACHE_VERSION}:subgraph:{subgraph}*",) } InvalidationRequest::Type { subgraph, r#type } => { - format!("subgraph:{subgraph}:type:{type}*",) + format!("version:{ENTITY_CACHE_VERSION}:subgraph:{subgraph}:type:{type}*",) } InvalidationRequest::Entity { subgraph, @@ -191,7 +192,7 @@ impl InvalidationRequest { key, } => { let entity_key = hash_entity_key(key); - format!("subgraph:{subgraph}:type:{type}:entity:{entity_key}*") + format!("version:{ENTITY_CACHE_VERSION}:subgraph:{subgraph}:type:{type}:entity:{entity_key}*") } } } From be97fb8b35b3aa84ef0e3d76a9d531c85689cf10 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Wed, 24 Jul 2024 09:57:26 +0100 Subject: [PATCH 10/10] Fix the tests to match the changes in the key structure --- apollo-router/tests/integration/redis.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apollo-router/tests/integration/redis.rs b/apollo-router/tests/integration/redis.rs index 9cf5912c60..b7bfe2ea52 100644 --- a/apollo-router/tests/integration/redis.rs +++ b/apollo-router/tests/integration/redis.rs @@ -413,7 +413,7 @@ async fn entity_cache() -> Result<(), BoxError> { let v: Value = serde_json::from_str(&s).unwrap(); insta::assert_json_snapshot!(v.as_object().unwrap().get("data").unwrap()); - let s: String = client.get("version:1.0:subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c").await.unwrap(); + let s: String = client.get("version:1.0:subgraph:reviews:type:Product:entity:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:hash:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:data:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c").await.unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); insta::assert_json_snapshot!(v.as_object().unwrap().get("data").unwrap()); @@ -517,7 +517,7 @@ async fn entity_cache() -> Result<(), BoxError> { insta::assert_json_snapshot!(response); let s:String = client - .get("version:1.0:subgraph:reviews:Product:d9a4cd73308dd13ca136390c10340823f94c335b9da198d2339c886c738abf0d:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("version:1.0:subgraph:reviews:type:Product:entity:d9a4cd73308dd13ca136390c10340823f94c335b9da198d2339c886c738abf0d:hash:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:data:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); @@ -755,7 +755,7 @@ async fn entity_cache_authorization() -> Result<(), BoxError> { ); let s: String = client - .get("version:1.0:subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("version:1.0:subgraph:reviews:type:Product:entity:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:hash:1de543dab57fde0f00247922ccc4f76d4c916ae26a89dd83cd1a62300d0cda20:data:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); @@ -799,7 +799,7 @@ async fn entity_cache_authorization() -> Result<(), BoxError> { insta::assert_json_snapshot!(response); let s:String = client - .get("version:1.0:subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:3b6ef3c8fd34c469d59f513942c5f4c8f91135e828712de2024e2cd4613c50ae:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("version:1.0:subgraph:reviews:type:Product:entity:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:hash:3b6ef3c8fd34c469d59f513942c5f4c8f91135e828712de2024e2cd4613c50ae:data:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap();