Reconcile dev after merge to main for v1.45.1 (#5037)

Follow-up to the v1.45.1 being officially released, bringing version bumps and changelog updates into the dev branch. Separating this from #5036 to solve the conflicts between main and dev without making an additional commit to main
apollographql · Apr 29, 2024 · c23816a · c23816a
2 parents b3bb6a4 + 50eb674
commit c23816a
Show file tree

Hide file tree

Showing 37 changed files with 414 additions and 397 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -48,14 +48,14 @@ executors:
       # See https://circleci.com/docs/xcode-policy along with the support matrix
       # at https://circleci.com/docs/using-macos#supported-xcode-versions.
       # We use the major.minor notation to bring in compatible patches.
-      xcode: 14.2
+      xcode: "14.2.0"
     resource_class: macos.m1.medium.gen1
   macos_test: &macos_test_executor
     macos:
       # See https://circleci.com/docs/xcode-policy along with the support matrix
       # at https://circleci.com/docs/using-macos#supported-xcode-versions.
       # We use the major.minor notation to bring in compatible patches.
-      xcode: 14.2
+      xcode: "14.2.0"
     resource_class: macos.m1.medium.gen1
   windows_build: &windows_build_executor
     machine:
@@ -201,7 +201,7 @@ commands:
             find xtask/src -type f | while read name; do md5sum $name; done | sort -k 2 | md5sum > ~/.xtask_version
             # The closest common ancestor to the default branch, so that test jobs can take advantage previous compiles
             git remote set-head origin -a
-            TARGET_BRANCH=$(git rev-parse --abbrev-ref origin/HEAD)    
+            TARGET_BRANCH=$(git rev-parse --abbrev-ref origin/HEAD)
             echo "Target branch is ${TARGET_BRANCH}"
             COMMON_ANCESTOR_REF=$(git merge-base HEAD "${TARGET_BRANCH}")
             echo "Common ancestor is ${COMMON_ANCESTOR_REF}"
@@ -737,8 +737,8 @@ jobs:
                   # save containers for analysis
                   mkdir built-containers
                   docker save -o built-containers/router_${VERSION}-debug.tar ${ROUTER_TAG}:${VERSION}-debug
-                  docker save -o built-containers/router_${VERSION}.tar ${ROUTER_TAG}:${VERSION}  
-                 
+                  docker save -o built-containers/router_${VERSION}.tar ${ROUTER_TAG}:${VERSION}
+
             - persist_to_workspace:
                 root: .
                 paths:
@@ -993,8 +993,8 @@ workflows:
           # Disables all PR comments from this job
           do-pr-comments: false
           # Scan job will return 1 if findings violating the Wiz policy are found.
-          # Toggle off to prevent any CI failures OR 
-          # contact Apollo's Security team to adjust what violates the 
+          # Toggle off to prevent any CI failures OR
+          # contact Apollo's Security team to adjust what violates the
           # Wiz policy used in this scan.
           fail-on-findings: true
           # Configure scan job to use a policy specific to apollo-router.
@@ -1091,7 +1091,7 @@ workflows:
               ignore: /.*/
             tags:
               only: /v.*/
-  
+
   security-scans:
     when:
       not: << pipeline.parameters.nightly >>

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,8 +4,37 @@ 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.45.1] - 2024-04-26
+
+## 🐛 Fixes
+
+### Correct v1.44.0 regression in query plan cache ([PR #5028](https://github.com/apollographql/router/pull/5028))
+
+Correct a critical regression that was introduced in [v1.44.0](https://github.com/apollographql/router/pull/4883) which could lead to execution of an incorrect query plan. This issue only affects Routers that use [distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), enabled via the `supergraph.query_planning.cache.redis.urls` configuration property.
+
+By [@o0Ignition0o](https://github.com/o0Ignition0o) in https://github.com/apollographql/router/pull/5028
+
+### Use entire schema when hashing an introspection query ([Issue #5006](https://github.com/apollographql/router/issues/5006))
+
+Correct a _different_ hashing bug which impacted introspection queries which was also introduced in [v1.44.0](https://github.com/apollographql/router/pull/4883).  This other hashing bug failed to account for introspection queries, resulting in introspection results being misaligned to the current schema. This issue only affects Routers that use [distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), enabled via the `supergraph.query_planning.cache.redis.urls` configuration property. 
+
+This release fixes the hashing mechanism by adding the schema string to hashed data if an introspection field is encountered. As a result, the entire schema is taken into account and the correct introspection result is returned.
+
+By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/5007
+
+### Fix subgraph name mapping of Datadog exporter ([PR #5012](https://github.com/apollographql/router/pull/5012))
+
+Previously in the router v1.45.0, subgraph name mapping didn't work correctly in the router's Datadog exporter. The exporter used the incorrect value `apollo.subgraph.name` for mapping attributes when it should have used the value `subgraph.name`. This issue has been fixed in this release.
+
+By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/5012
+
+
 # [1.45.0] - 2024-04-22
 
+> **Warning**
+>
+> **This version has a critical bug impacting users of [distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching).  See the _Fixes_ in [v1.45.1](https://github.com/apollographql/router/releases/tag/v1.45.1) for details.  We highly recommend using v1.45.1 or v1.43.2 over v1.45.0.**
+
 ## 🚀 Features
 
 ### Query validation process with Rust ([PR #4551](https://github.com/apollographql/router/pull/4551))
@@ -154,6 +183,11 @@ By [@bonnici](https://github.com/bonnici) in https://github.com/apollographql/ro
 
 # [1.44.0] - 2024-04-12
 
+> **Warning**
+>
+> **This version has a critical bug impacting users of [distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching).  See the _Fixes_ in [v1.45.1](https://github.com/apollographql/router/releases/tag/v1.45.1) for details.  We highly recommend using v1.45.1 or v1.43.2 over v1.44.0.**
+
+
 ## 🚀 Features
 
 ### Add details to `router service call failed` errors ([Issue #4899](https://github.com/apollographql/router/issues/4899))

diff --git a/Cargo.lock b/Cargo.lock
@@ -250,7 +250,7 @@ dependencies = [
 
 [[package]]
 name = "apollo-router"
-version = "1.45.0"
+version = "1.45.1"
 dependencies = [
  "access-json",
  "anyhow",
@@ -412,7 +412,7 @@ dependencies = [
 
 [[package]]
 name = "apollo-router-benchmarks"
-version = "1.45.0"
+version = "1.45.1"
 dependencies = [
  "apollo-parser",
  "apollo-router",
@@ -428,7 +428,7 @@ dependencies = [
 
 [[package]]
 name = "apollo-router-scaffold"
-version = "1.45.0"
+version = "1.45.1"
 dependencies = [
  "anyhow",
  "cargo-scaffold",

diff --git a/apollo-router-benchmarks/Cargo.toml b/apollo-router-benchmarks/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "apollo-router-benchmarks"
-version = "1.45.0"
+version = "1.45.1"
 authors = ["Apollo Graph, Inc. <packages@apollographql.com>"]
 edition = "2021"
 license = "Elastic-2.0"

diff --git a/apollo-router-scaffold/Cargo.toml b/apollo-router-scaffold/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "apollo-router-scaffold"
-version = "1.45.0"
+version = "1.45.1"
 authors = ["Apollo Graph, Inc. <packages@apollographql.com>"]
 edition = "2021"
 license = "Elastic-2.0"

diff --git a/apollo-router-scaffold/templates/base/Cargo.toml b/apollo-router-scaffold/templates/base/Cargo.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.45.0"
+apollo-router = "1.45.1"
 {{/if}}
 {{/if}}
 async-trait = "0.1.52"

diff --git a/apollo-router-scaffold/templates/base/xtask/Cargo.toml b/apollo-router-scaffold/templates/base/xtask/Cargo.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.45.0" }
+apollo-router-scaffold = { git = "https://github.com/apollographql/router.git", tag = "v1.45.1" }
 {{/if}}
 {{/if}}
 anyhow = "1.0.58"

diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "apollo-router"
-version = "1.45.0"
+version = "1.45.1"
 authors = ["Apollo Graph, Inc. <packages@apollographql.com>"]
 repository = "https://github.com/apollographql/router/"
 documentation = "https://docs.rs/apollo-router"

diff --git a/...lugins__progressive_override__tests__non_overridden_field_yields_expected_query_plan.snap b/...lugins__progressive_override__tests__non_overridden_field_yields_expected_query_plan.snap
@@ -18,7 +18,7 @@ expression: query_plan
           "id": null,
           "inputRewrites": null,
           "outputRewrites": null,
-          "schemaAwareHash": "9358047754b11522aac502a3c6a668cd4286c07d489680834e63d6e033db4eb5",
+          "schemaAwareHash": "12dda6193654ae4fe6e38bc09d4f81cc73d0c9e098692096f72d2158eef4776f",
           "authorization": {
             "is_authenticated": false,
             "scopes": [],

diff --git a/...r__plugins__progressive_override__tests__overridden_field_yields_expected_query_plan.snap b/...r__plugins__progressive_override__tests__overridden_field_yields_expected_query_plan.snap
@@ -23,7 +23,7 @@ expression: query_plan
               "id": null,
               "inputRewrites": null,
               "outputRewrites": null,
-              "schemaAwareHash": "8f445761c0bcdda90b8da35ccd13fd98e474514f3efc071bd2c39495b5af94e5",
+              "schemaAwareHash": "00ad582ea45fc1bce436b36b21512f3d2c47b74fdbdc61e4b349289722c9ecf2",
               "authorization": {
                 "is_authenticated": false,
                 "scopes": [],
@@ -61,7 +61,7 @@ expression: query_plan
                 "id": null,
                 "inputRewrites": null,
                 "outputRewrites": null,
-                "schemaAwareHash": "9a1feab7ee8c57c8a4ab4db29712412a9cfe94009bfcb40dc0d22ea54c410865",
+                "schemaAwareHash": "a8ebdc2151a2e5207882e43c6906c0c64167fd9a8e0c7c4becc47736a5105096",
                 "authorization": {
                   "is_authenticated": false,
                   "scopes": [],

diff --git a/...s/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan-2.snap b/...s/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan-2.snap
@@ -68,7 +68,7 @@ expression: "serde_json::to_value(response).unwrap()"
               "id": null,
               "inputRewrites": null,
               "outputRewrites": null,
-              "schemaAwareHash": "34be619a78867ab9d0670048f4c93574e38cd9253e9cc032f567078355b25086",
+              "schemaAwareHash": "7245d488e97c3b2ac9f5fa4dd4660940b94ad81af070013305b2c0f76337b2f9",
               "authorization": {
                 "is_authenticated": false,
                 "scopes": [],
@@ -107,7 +107,7 @@ expression: "serde_json::to_value(response).unwrap()"
                 "id": null,
                 "inputRewrites": null,
                 "outputRewrites": null,
-                "schemaAwareHash": "f1582d942020b23347d84f6ae46c018492ae7c59c9b1472e0b442121ddf16368",
+                "schemaAwareHash": "6e0b4156706ea0cf924500cfdc99dd44b9f0ed07e2d3f888d4aff156e6a33238",
                 "authorization": {
                   "is_authenticated": false,
                   "scopes": [],
@@ -153,7 +153,7 @@ expression: "serde_json::to_value(response).unwrap()"
                     "id": null,
                     "inputRewrites": null,
                     "outputRewrites": null,
-                    "schemaAwareHash": "6fa5a74c5af2b18f343e9e69bbcbc9335e9faaa46c3d8964d199002dfeb0026f",
+                    "schemaAwareHash": "ff649f3d70241d5a8cd5f5d03ff4c41ecff72b0e4129a480207b05ac92318042",
                     "authorization": {
                       "is_authenticated": false,
                       "scopes": [],
@@ -196,7 +196,7 @@ expression: "serde_json::to_value(response).unwrap()"
                     "id": null,
                     "inputRewrites": null,
                     "outputRewrites": null,
-                    "schemaAwareHash": "6fa5a74c5af2b18f343e9e69bbcbc9335e9faaa46c3d8964d199002dfeb0026f",
+                    "schemaAwareHash": "bf9f3beda78a7a565e47c862157bad4ec871d724d752218da1168455dddca074",
                     "authorization": {
                       "is_authenticated": false,
                       "scopes": [],

diff --git a/...ins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan.snap b/...ins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan.snap
@@ -68,7 +68,7 @@ expression: "serde_json::to_value(response).unwrap()"
               "id": null,
               "inputRewrites": null,
               "outputRewrites": null,
-              "schemaAwareHash": "34be619a78867ab9d0670048f4c93574e38cd9253e9cc032f567078355b25086",
+              "schemaAwareHash": "7245d488e97c3b2ac9f5fa4dd4660940b94ad81af070013305b2c0f76337b2f9",
               "authorization": {
                 "is_authenticated": false,
                 "scopes": [],
@@ -107,7 +107,7 @@ expression: "serde_json::to_value(response).unwrap()"
                 "id": null,
                 "inputRewrites": null,
                 "outputRewrites": null,
-                "schemaAwareHash": "f1582d942020b23347d84f6ae46c018492ae7c59c9b1472e0b442121ddf16368",
+                "schemaAwareHash": "6e0b4156706ea0cf924500cfdc99dd44b9f0ed07e2d3f888d4aff156e6a33238",
                 "authorization": {
                   "is_authenticated": false,
                   "scopes": [],
@@ -153,7 +153,7 @@ expression: "serde_json::to_value(response).unwrap()"
                     "id": null,
                     "inputRewrites": null,
                     "outputRewrites": null,
-                    "schemaAwareHash": "6fa5a74c5af2b18f343e9e69bbcbc9335e9faaa46c3d8964d199002dfeb0026f",
+                    "schemaAwareHash": "ff649f3d70241d5a8cd5f5d03ff4c41ecff72b0e4129a480207b05ac92318042",
                     "authorization": {
                       "is_authenticated": false,
                       "scopes": [],
@@ -196,7 +196,7 @@ expression: "serde_json::to_value(response).unwrap()"
                     "id": null,
                     "inputRewrites": null,
                     "outputRewrites": null,
-                    "schemaAwareHash": "6fa5a74c5af2b18f343e9e69bbcbc9335e9faaa46c3d8964d199002dfeb0026f",
+                    "schemaAwareHash": "bf9f3beda78a7a565e47c862157bad4ec871d724d752218da1168455dddca074",
                     "authorization": {
                       "is_authenticated": false,
                       "scopes": [],

diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs
@@ -631,7 +631,7 @@ impl BridgeQueryPlanner {
         plan_success
             .data
             .query_plan
-            .hash_subqueries(&self.subgraph_schemas);
+            .hash_subqueries(&self.subgraph_schemas, &self.schema.raw_sdl);
         plan_success
             .data
             .query_plan
@@ -830,19 +830,21 @@ impl Service<QueryPlannerRequest> for BridgeQueryPlanner {
                 Some(d) => d,
             };
 
-            let schema = this.schema.api_schema();
-            match add_defer_labels(schema, &doc.ast) {
+            let api_schema = this.schema.api_schema();
+            match add_defer_labels(api_schema, &doc.ast) {
                 Err(e) => {
                     return Err(QueryPlannerError::SpecError(SpecError::TransformError(
                         e.to_string(),
                     )))
                 }
                 Ok(modified_query) => {
                     let executable_document = modified_query
-                        .to_executable_validate(schema)
+                        .to_executable_validate(api_schema)
+                        // Assume transformation creates a valid document: ignore conversion errors
                         .map_err(|e| SpecError::ValidationError(e.into()))?;
                     let hash = QueryHashVisitor::hash_query(
-                        schema,
+                        this.schema.supergraph_schema(),
+                        &this.schema.raw_sdl,
                         &executable_document,
                         operation_name.as_deref(),
                     )
@@ -962,6 +964,7 @@ impl BridgeQueryPlanner {
                 .map_err(|e| SpecError::ValidationError(e.into()))?;
             let hash = QueryHashVisitor::hash_query(
                 self.schema.supergraph_schema(),
+                &self.schema.raw_sdl,
                 &executable_document,
                 key.operation_name.as_deref(),
             )
@@ -1054,9 +1057,13 @@ pub(super) struct QueryPlan {
 }
 
 impl QueryPlan {
-    fn hash_subqueries(&mut self, subgraph_schemas: &SubgraphSchemas) {
+    fn hash_subqueries(
+        &mut self,
+        subgraph_schemas: &SubgraphSchemas,
+        supergraph_schema_hash: &str,
+    ) {
         if let Some(node) = self.node.as_mut() {
-            node.hash_subqueries(subgraph_schemas);
+            node.hash_subqueries(subgraph_schemas, supergraph_schema_hash);
         }
     }
 

diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs
@@ -141,6 +141,7 @@ where
                             hash,
                             metadata,
                             plan_options,
+                            ..
                         },
                         _,
                     )| WarmUpCachingQueryKey {
@@ -206,6 +207,7 @@ where
                 query: query.clone(),
                 operation: operation.clone(),
                 hash: doc.hash.clone(),
+                sdl: Arc::clone(&self.schema.raw_sdl),
                 metadata,
                 plan_options,
             };
@@ -386,6 +388,7 @@ where
             query: request.query.clone(),
             operation: request.operation_name.to_owned(),
             hash: doc.hash.clone(),
+            sdl: Arc::clone(&self.schema.raw_sdl),
             metadata,
             plan_options,
         };
@@ -522,6 +525,7 @@ fn stats_report_key_hash(stats_report_key: &str) -> String {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(crate) struct CachingQueryKey {
     pub(crate) query: String,
+    pub(crate) sdl: Arc<String>,
     pub(crate) operation: Option<String>,
     pub(crate) hash: Arc<QueryHash>,
     pub(crate) metadata: CacheKeyMetadata,
@@ -541,6 +545,7 @@ impl std::fmt::Display for CachingQueryKey {
         hasher.update(
             &serde_json::to_vec(&self.plan_options).expect("serialization should not fail"),
         );
+        hasher.update(&serde_json::to_vec(&self.sdl).expect("serialization should not fail"));
         let metadata = hex::encode(hasher.finalize());
 
         write!(

diff --git a/apollo-router/src/query_planner/fetch.rs b/apollo-router/src/query_planner/fetch.rs
@@ -607,12 +607,20 @@ impl FetchNode {
         &self.operation_kind
     }
 
-    pub(crate) fn hash_subquery(&mut self, subgraph_schemas: &SubgraphSchemas) {
+    pub(crate) fn hash_subquery(
+        &mut self,
+        subgraph_schemas: &SubgraphSchemas,
+        supergraph_schema_hash: &str,
+    ) {
         let doc = self.parsed_operation(subgraph_schemas);
         let schema = &subgraph_schemas[self.service_name.as_str()];
 
-        if let Ok(hash) = QueryHashVisitor::hash_query(schema, doc, self.operation_name.as_deref())
-        {
+        if let Ok(hash) = QueryHashVisitor::hash_query(
+            schema,
+            supergraph_schema_hash,
+            doc,
+            self.operation_name.as_deref(),
+        ) {
             self.schema_aware_hash = Arc::new(QueryHash(hash));
         }
     }