Use the entire schema when hashing an introspection query

.changesets/fix_geal_schema_aware_hash_introspection.md

@@ -0,0 +1,6 @@
+### Use the entire schema when hashing an introspection query ([Issue #5006](https://github.com/apollographql/router/issues/5006))
+
+in https://github.com/apollographql/router/pull/4883 (1.44.0), we introduced a query hashing scheme that stays stable across schema updates if the update does not affect the query. Unfortunately, it was not taking introspection queries into account.
+This fixes the hashing mechanism to add the schema string to hashed data if we encounter an introspection field, so the entire schema is taken into account
+
+By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/5007

apollo-router/src/query_planner/bridge_query_planner.rs

@@ -379,7 +379,9 @@ impl BridgeQueryPlanner {
             .into_result()
         {
             Ok(mut plan) => {
-                plan.data.query_plan.hash_subqueries(&self.subgraph_schemas);
+                plan.data
+                    .query_plan
+                    .hash_subqueries(&self.subgraph_schemas, &self.schema.raw_sdl);
                 plan.data
                     .query_plan
                     .extract_authorization_metadata(&self.schema.definitions, &key);
@@ -583,19 +585,22 @@ impl Service<QueryPlannerRequest> for BridgeQueryPlanner {
                 Some(d) => d,
             };
 
-            let schema = &this.schema.api_schema().definitions;
-            match add_defer_labels(schema, &doc.ast) {
+            let api_schema = this.schema.api_schema();
+            let api_schema_definitions = &api_schema.definitions;
+            match add_defer_labels(api_schema_definitions, &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_definitions)
+                        // Assume transformation creates a valid document: ignore conversion errors
                         .map_err(|e| SpecError::ValidationError(e.into()))?;
                     let hash = QueryHashVisitor::hash_query(
-                        schema,
+                        api_schema_definitions,
+                        &api_schema.raw_sdl,
                         &executable_document,
                         operation_name.as_deref(),
                     )
@@ -715,6 +720,7 @@ impl BridgeQueryPlanner {
                 .map_err(|e| SpecError::ValidationError(e.into()))?;
             let hash = QueryHashVisitor::hash_query(
                 &self.schema.definitions,
+                &self.schema.raw_sdl,
                 &executable_document,
                 key.operation_name.as_deref(),
             )
@@ -807,9 +813,13 @@ struct QueryPlan {
 }
 
 impl QueryPlan {
-    fn hash_subqueries(&mut self, schemas: &HashMap<String, Arc<Valid<apollo_compiler::Schema>>>) {
+    fn hash_subqueries(
+        &mut self,
+        schemas: &HashMap<String, Arc<Valid<apollo_compiler::Schema>>>,
+        supergraph_schema_hash: &str,
+    ) {
         if let Some(node) = self.node.as_mut() {
-            node.hash_subqueries(schemas);
+            node.hash_subqueries(schemas, supergraph_schema_hash);
         }
     }
 

apollo-router/src/query_planner/fetch.rs

@@ -497,12 +497,20 @@ impl FetchNode {
         &self.operation_kind
     }
 
-    pub(crate) fn hash_subquery(&mut self, schema: &Valid<apollo_compiler::Schema>) {
+    pub(crate) fn hash_subquery(
+        &mut self,
+        schema: &Valid<apollo_compiler::Schema>,
+        supergraph_schema_hash: &str,
+    ) {
         let doc = ExecutableDocument::parse(schema, &self.operation, "query.graphql")
             .expect("subgraph queries should be valid");
 
-        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));
         }
     }

apollo-router/src/query_planner/plan.rs

@@ -315,40 +315,43 @@ impl PlanNode {
     pub(crate) fn hash_subqueries(
         &mut self,
         schemas: &HashMap<String, Arc<Valid<apollo_compiler::Schema>>>,
+        supergraph_schema_hash: &str,
     ) {
         match self {
             PlanNode::Fetch(fetch_node) => {
                 if let Some(schema) = schemas.get(&fetch_node.service_name) {
-                    fetch_node.hash_subquery(schema);
+                    fetch_node.hash_subquery(schema, supergraph_schema_hash);
                 }
             }
 
             PlanNode::Sequence { nodes } => {
                 for node in nodes {
-                    node.hash_subqueries(schemas);
+                    node.hash_subqueries(schemas, supergraph_schema_hash);
                 }
             }
             PlanNode::Parallel { nodes } => {
                 for node in nodes {
-                    node.hash_subqueries(schemas);
+                    node.hash_subqueries(schemas, supergraph_schema_hash);
                 }
             }
-            PlanNode::Flatten(flatten) => flatten.node.hash_subqueries(schemas),
+            PlanNode::Flatten(flatten) => flatten
+                .node
+                .hash_subqueries(schemas, supergraph_schema_hash),
             PlanNode::Defer { primary, deferred } => {
                 if let Some(node) = primary.node.as_mut() {
-                    node.hash_subqueries(schemas);
+                    node.hash_subqueries(schemas, supergraph_schema_hash);
                 }
                 for deferred_node in deferred {
                     if let Some(node) = deferred_node.node.take() {
                         let mut new_node = (*node).clone();
-                        new_node.hash_subqueries(schemas);
+                        new_node.hash_subqueries(schemas, supergraph_schema_hash);
                         deferred_node.node = Some(Arc::new(new_node));
                     }
                 }
             }
             PlanNode::Subscription { primary: _, rest } => {
                 if let Some(node) = rest.as_mut() {
-                    node.hash_subqueries(schemas);
+                    node.hash_subqueries(schemas, supergraph_schema_hash);
                 }
             }
             PlanNode::Condition {
@@ -357,10 +360,10 @@ impl PlanNode {
                 else_clause,
             } => {
                 if let Some(node) = if_clause.as_mut() {
-                    node.hash_subqueries(schemas);
+                    node.hash_subqueries(schemas, supergraph_schema_hash);
                 }
                 if let Some(node) = else_clause.as_mut() {
-                    node.hash_subqueries(schemas);
+                    node.hash_subqueries(schemas, supergraph_schema_hash);
                 }
             }
         }

apollo-router/src/spec/query.rs

@@ -280,8 +280,9 @@ impl Query {
                 return Err(SpecError::ParseError(errors.into()));
             }
         };
-        let schema = &schema.api_schema().definitions;
-        let executable_document = match ast.to_executable_validate(schema) {
+        let api_schema = schema.api_schema();
+        let api_schema_definitions = &api_schema.definitions;
+        let executable_document = match ast.to_executable_validate(api_schema_definitions) {
             Ok(doc) => doc,
             Err(errors) => {
                 return Err(SpecError::ValidationError(errors.into()));
@@ -292,8 +293,14 @@ impl Query {
         let recursion_limit = parser.recursion_reached();
         tracing::trace!(?recursion_limit, "recursion limit data");
 
-        let hash = QueryHashVisitor::hash_query(schema, &executable_document, operation_name)
-            .map_err(|e| SpecError::QueryHashing(e.to_string()))?;
+        let hash = QueryHashVisitor::hash_query(
+            api_schema_definitions,
+            &api_schema.raw_sdl,
+            &executable_document,
+            operation_name,
+        )
+        .map_err(|e| SpecError::QueryHashing(e.to_string()))?;
+
         Ok(Arc::new(ParsedDocumentInner {
             ast,
             executable: Arc::new(executable_document),
@@ -343,7 +350,7 @@ impl Query {
             .map(|operation| Operation::from_hir(operation, schema, &mut defer_stats, &fragments))
             .collect::<Result<Vec<_>, SpecError>>()?;
 
-        let mut visitor = QueryHashVisitor::new(&schema.definitions, document);
+        let mut visitor = QueryHashVisitor::new(&schema.definitions, &schema.raw_sdl, document);
         traverse::document(&mut visitor, document, operation_name).map_err(|e| {
             SpecError::QueryHashing(format!("could not calculate the query hash: {e}"))
         })?;

apollo-router/src/spec/query/change.rs

@@ -33,26 +33,34 @@ pub(crate) const JOIN_TYPE_DIRECTIVE_NAME: &str = "join__type";
 /// then the hash will stay the same
 pub(crate) struct QueryHashVisitor<'a> {
     schema: &'a schema::Schema,
+    // TODO: remove once introspection has been moved out of query planning
+    // For now, introspection is stiull handled by the planner, so when an
+    // introspection query is hashed, it should take the whole schema into account
+    schema_str: &'a str,
     hasher: Sha256,
     fragments: HashMap<&'a ast::Name, &'a Node<executable::Fragment>>,
     hashed_types: HashSet<String>,
     // name, field
     hashed_fields: HashSet<(String, String)>,
+    seen_introspection: bool,
     join_field_directive_name: Option<String>,
     join_type_directive_name: Option<String>,
 }
 
 impl<'a> QueryHashVisitor<'a> {
     pub(crate) fn new(
         schema: &'a schema::Schema,
+        schema_str: &'a str,
         executable: &'a executable::ExecutableDocument,
     ) -> Self {
         Self {
             schema,
+            schema_str,
             hasher: Sha256::new(),
             fragments: executable.fragments.iter().collect(),
             hashed_types: HashSet::new(),
             hashed_fields: HashSet::new(),
+            seen_introspection: false,
             // should we just return an error if we do not find those directives?
             join_field_directive_name: Schema::directive_name(
                 schema,
@@ -71,10 +79,11 @@ impl<'a> QueryHashVisitor<'a> {
 
     pub(crate) fn hash_query(
         schema: &'a schema::Schema,
+        schema_str: &'a str,
         executable: &'a executable::ExecutableDocument,
         operation_name: Option<&str>,
     ) -> Result<Vec<u8>, BoxError> {
-        let mut visitor = QueryHashVisitor::new(schema, executable);
+        let mut visitor = QueryHashVisitor::new(schema, schema_str, executable);
         traverse::document(&mut visitor, executable, operation_name)?;
         Ok(visitor.finish())
     }
@@ -357,6 +366,12 @@ impl<'a> Visitor for QueryHashVisitor<'a> {
         field_def: &ast::FieldDefinition,
         node: &executable::Field,
     ) -> Result<(), BoxError> {
+        if !self.seen_introspection && (field_def.name == "__schema" || field_def.name == "__type")
+        {
+            self.seen_introspection = true;
+            self.schema_str.hash(self);
+        }
+
         self.hash_field(
             parent_type.to_string(),
             field_def.name.as_str().to_string(),
@@ -412,8 +427,8 @@ mod tests {
     use crate::spec::query::traverse;
 
     #[track_caller]
-    fn hash(schema: &str, query: &str) -> String {
-        let schema = Schema::parse(schema, "schema.graphql")
+    fn hash(schema_str: &str, query: &str) -> String {
+        let schema = Schema::parse(schema_str, "schema.graphql")
             .unwrap()
             .validate()
             .unwrap();
@@ -424,22 +439,22 @@ mod tests {
             .unwrap()
             .validate(&schema)
             .unwrap();
-        let mut visitor = QueryHashVisitor::new(&schema, &exec);
+        let mut visitor = QueryHashVisitor::new(&schema, schema_str, &exec);
         traverse::document(&mut visitor, &exec, None).unwrap();
 
         hex::encode(visitor.finish())
     }
 
     #[track_caller]
-    fn hash_subgraph_query(schema: &str, query: &str) -> String {
-        let schema = Valid::assume_valid(Schema::parse(schema, "schema.graphql").unwrap());
+    fn hash_subgraph_query(schema_str: &str, query: &str) -> String {
+        let schema = Valid::assume_valid(Schema::parse(schema_str, "schema.graphql").unwrap());
         let doc = Document::parse(query, "query.graphql").unwrap();
         let exec = doc
             .to_executable(&schema)
             .unwrap()
             .validate(&schema)
             .unwrap();
-        let mut visitor = QueryHashVisitor::new(&schema, &exec);
+        let mut visitor = QueryHashVisitor::new(&schema, schema_str, &exec);
         traverse::document(&mut visitor, &exec, None).unwrap();
 
         hex::encode(visitor.finish())

docs/source/configuration/telemetry/instrumentation/standard-instruments.mdx

@@ -74,13 +74,11 @@ The coprocessor operations metric has the following attributes:
 </Tip>
 
 - `apollo_router_uplink_fetch_duration_seconds_bucket` - Uplink request duration, attributes:
-
-- `url`: The Uplink URL that was polled
-- `query`: The query that the router sent to Uplink (`SupergraphSdl` or `License`)
-- `kind`: (`new`, `unchanged`, `http_error`, `uplink_error`)
-- `code`: The error code depending on type (if an error occurred)
-- `error`: The error message (if an error occurred)
-
+  - `url`: The Uplink URL that was polled
+  - `query`: The query that the router sent to Uplink (`SupergraphSdl` or `License`)
+  - `kind`: (`new`, `unchanged`, `http_error`, `uplink_error`)
+  - `code`: The error code depending on type (if an error occurred)
+  - `error`: The error message (if an error occurred)
 - `apollo_router_uplink_fetch_count_total`
   - `status`: (`success`, `failure`)
   - `query`: The query that the router sent to Uplink (`SupergraphSdl` or `License`)

docs/source/customizations/native.mdx

@@ -100,8 +100,8 @@ impl Plugin for HelloWorld {
 
     fn supergraph_service(
         &self,
-        service: router::BoxService,
-    ) -> router::BoxService {
+        service: supergraph::BoxService,
+    ) -> supergraph::BoxService {
         service
     }
 

docs/source/executing-operations/subscription-support.mdx

@@ -173,6 +173,7 @@ If something crashes on your side for a specific subscription on specific events
 Also, when handling subscriptions with heartbeats disabled, make sure to store a subscription's request payload (including extensions data with callback URL and verifier) to be able to send the right events on the right callback URL when you start your lambda function triggered by an event.
 
 </Caution>
+
 ### Using a combination of modes
 
 If some of your subgraphs require [passthrough mode](#websocket-setup) and others require [callback mode](#http-callback-setup) for subscriptions, you can apply different modes to different subgraphs in your configuration: