release: v1.58.1

.changesets/fix_bnjjj_fix_880.md

@@ -0,0 +1,53 @@
+### Fix telemetry instrumentation using supergraph query selector ([PR #6324](https://github.com/apollographql/router/pull/6324))
+
+Query selector was raising error logs like `this is a bug and should not happen`. It's now fixed.
+Now this configuration will work properly:
+```yaml title=router.yaml
+telemetry:
+  exporters:
+    metrics:
+      common:
+        views:
+          # Define a custom view because operation limits are different than the default latency-oriented view of OpenTelemetry
+          - name: oplimits.*
+            aggregation:
+              histogram:
+                buckets:
+                  - 0
+                  - 5
+                  - 10
+                  - 25
+                  - 50
+                  - 100
+                  - 500
+                  - 1000
+  instrumentation:
+    instruments:
+      supergraph:
+        oplimits.aliases:
+          value:
+            query: aliases
+          type: histogram
+          unit: number
+          description: "Aliases for an operation"
+        oplimits.depth:
+          value:
+            query: depth
+          type: histogram
+          unit: number
+          description: "Depth for an operation"
+        oplimits.height:
+          value:
+            query: height
+          type: histogram
+          unit: number
+          description: "Height for an operation"
+        oplimits.root_fields:
+          value:
+            query: root_fields
+          type: histogram
+          unit: number
+          description: "Root fields for an operation"
+```
+
+By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/6324

.changesets/fix_renee_max_evaluated_plans_rs.md

@@ -0,0 +1,7 @@
+### fix: propagate evaluated plans limit and paths limit to the native query planner ([PR #6316](https://github.com/apollographql/router/pull/6316))
+
+Two experimental query planning complexity limiting options now work with the native query planner:
+- `supergraph.query_planning.experimental_plans_limit`
+- `supergraph.query_planning.experimental_paths_limit`
+
+By [@goto-bus-stop](https://github.com/goto-bus-stop) in https://github.com/apollographql/router/pull/6316

.circleci/config.yml

@@ -1,6 +1,6 @@
 version: 2.1
 
-# Cache key bump: 2
+# Cache key bump: 3
 
 # These "CircleCI Orbs" are reusable bits of configuration that can be shared
 # across projects.  See https://circleci.com/orbs/ for more information.

CHANGELOG.md

@@ -133,6 +133,12 @@ experimental_query_planner_mode: new
 
 By [@clenfest](https://github.com/clenfest), [@TylerBloom](https://github.com/TylerBloom) in https://github.com/apollographql/router/pull/6310
 
+### Allow configuring host via Helm template for virtual service ([PR #5545](https://github.com/apollographql/router/pull/5795))
+
+When deploying via Helm, you can now configure hosts in `virtualservice.yaml` as a single host or a range of hosts. This is helpful when different hosts could be used within a cluster.
+
+By [@nicksephora](https://github.com/nicksephora) in https://github.com/apollographql/router/pull/5545
+
 ## 🐛 Fixes
 
 ### Remove noisy demand control logs ([PR #6192](https://github.com/apollographql/router/pull/6192))

apollo-router/src/configuration/mod.rs

@@ -5,6 +5,7 @@ use std::io::BufReader;
 use std::iter;
 use std::net::IpAddr;
 use std::net::SocketAddr;
+use std::num::NonZeroU32;
 use std::num::NonZeroUsize;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -416,16 +417,31 @@ impl Configuration {
     pub(crate) fn rust_query_planner_config(
         &self,
     ) -> apollo_federation::query_plan::query_planner::QueryPlannerConfig {
-        apollo_federation::query_plan::query_planner::QueryPlannerConfig {
+        use apollo_federation::query_plan::query_planner::QueryPlanIncrementalDeliveryConfig;
+        use apollo_federation::query_plan::query_planner::QueryPlannerConfig;
+        use apollo_federation::query_plan::query_planner::QueryPlannerDebugConfig;
+
+        let max_evaluated_plans = self
+            .supergraph
+            .query_planning
+            .experimental_plans_limit
+            // Fails if experimental_plans_limit is zero; use our default.
+            .and_then(NonZeroU32::new)
+            .unwrap_or(NonZeroU32::new(10_000).expect("it is not zero"));
+
+        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,
-                },
+            incremental_delivery: QueryPlanIncrementalDeliveryConfig {
+                enable_defer: self.supergraph.defer_support,
+            },
             type_conditioned_fetching: self.experimental_type_conditioned_fetching,
-            debug: Default::default(),
+            debug: QueryPlannerDebugConfig {
+                bypass_planner_for_single_subgraph: false,
+                max_evaluated_plans,
+                paths_limit: self.supergraph.query_planning.experimental_paths_limit,
+            },
         }
     }
 }

apollo-router/src/plugins/telemetry/config_new/instruments.rs

@@ -1446,7 +1446,7 @@ where
                             })
                         }
                         None => {
-                            ::tracing::error!("cannot convert static instrument into a counter, this is an error; please fill an issue on GitHub");
+                            failfast_debug!("cannot convert static instrument into a counter, this is an error; please fill an issue on GitHub");
                         }
                     }
                 }
@@ -1501,7 +1501,7 @@ where
                             });
                         }
                         None => {
-                            ::tracing::error!("cannot convert static instrument into a histogram, this is an error; please fill an issue on GitHub");
+                            failfast_debug!("cannot convert static instrument into a histogram, this is an error; please fill an issue on GitHub");
                         }
                     }
                 }

apollo-router/src/plugins/telemetry/config_new/selectors.rs

@@ -1069,25 +1069,6 @@ impl Selector for SupergraphSelector {
         ctx: &Context,
     ) -> Option<opentelemetry::Value> {
         match self {
-            SupergraphSelector::Query { query, .. } => {
-                let limits_opt = ctx
-                    .extensions()
-                    .with_lock(|lock| lock.get::<OperationLimits<u32>>().cloned());
-                match query {
-                    Query::Aliases => {
-                        limits_opt.map(|limits| opentelemetry::Value::I64(limits.aliases as i64))
-                    }
-                    Query::Depth => {
-                        limits_opt.map(|limits| opentelemetry::Value::I64(limits.depth as i64))
-                    }
-                    Query::Height => {
-                        limits_opt.map(|limits| opentelemetry::Value::I64(limits.height as i64))
-                    }
-                    Query::RootFields => limits_opt
-                        .map(|limits| opentelemetry::Value::I64(limits.root_fields as i64)),
-                    Query::String => None,
-                }
-            }
             SupergraphSelector::ResponseData {
                 response_data,
                 default,
@@ -3289,12 +3270,6 @@ mod test {
                 .unwrap(),
             4.into()
         );
-        assert_eq!(
-            selector
-                .on_response_event(&crate::graphql::Response::builder().build(), &context)
-                .unwrap(),
-            4.into()
-        );
     }
 
     #[test]

apollo-router/src/query_planner/bridge_query_planner.rs

@@ -166,20 +166,7 @@ impl PlannerMode {
         schema: &Schema,
         configuration: &Configuration,
     ) -> Result<Arc<QueryPlanner>, ServiceBuildError> {
-        let config = apollo_federation::query_plan::query_planner::QueryPlannerConfig {
-            reuse_query_fragments: configuration
-                .supergraph
-                .reuse_query_fragments
-                .unwrap_or(true),
-            subgraph_graphql_validation: false,
-            generate_query_fragments: configuration.supergraph.generate_query_fragments,
-            incremental_delivery:
-                apollo_federation::query_plan::query_planner::QueryPlanIncrementalDeliveryConfig {
-                    enable_defer: configuration.supergraph.defer_support,
-                },
-            type_conditioned_fetching: configuration.experimental_type_conditioned_fetching,
-            debug: Default::default(),
-        };
+        let config = configuration.rust_query_planner_config();
         let result = QueryPlanner::new(schema.federation_supergraph(), config);
 
         match &result {

apollo-router/src/query_planner/dual_query_planner.rs

@@ -14,6 +14,7 @@ use apollo_compiler::ast;
 use apollo_compiler::validation::Valid;
 use apollo_compiler::ExecutableDocument;
 use apollo_compiler::Name;
+use apollo_compiler::Node;
 use apollo_federation::query_plan::query_planner::QueryPlanOptions;
 use apollo_federation::query_plan::query_planner::QueryPlanner;
 use apollo_federation::query_plan::QueryPlan;
@@ -1003,6 +1004,24 @@ fn same_ast_argument(x: &ast::Argument, y: &ast::Argument) -> bool {
     x.name == y.name && same_ast_argument_value(&x.value, &y.value)
 }
 
+fn same_ast_arguments(x: &[Node<ast::Argument>], y: &[Node<ast::Argument>]) -> bool {
+    vec_matches_sorted_by(
+        x,
+        y,
+        |a, b| a.name.cmp(&b.name),
+        |a, b| same_ast_argument(a, b),
+    )
+}
+
+fn same_directives(x: &ast::DirectiveList, y: &ast::DirectiveList) -> bool {
+    vec_matches_sorted_by(
+        x,
+        y,
+        |a, b| a.name.cmp(&b.name),
+        |a, b| a.name == b.name && same_ast_arguments(&a.arguments, &b.arguments),
+    )
+}
+
 fn get_ast_selection_key(
     selection: &ast::Selection,
     fragment_map: &HashMap<Name, Name>,
@@ -1035,24 +1054,20 @@ fn same_ast_selection(
         (ast::Selection::Field(x), ast::Selection::Field(y)) => {
             x.name == y.name
                 && x.alias == y.alias
-                && vec_matches_sorted_by(
-                    &x.arguments,
-                    &y.arguments,
-                    |a, b| a.name.cmp(&b.name),
-                    |a, b| same_ast_argument(a, b),
-                )
-                && x.directives == y.directives
+                && same_ast_arguments(&x.arguments, &y.arguments)
+                && same_directives(&x.directives, &y.directives)
                 && same_ast_selection_set_sorted(&x.selection_set, &y.selection_set, fragment_map)
         }
         (ast::Selection::FragmentSpread(x), ast::Selection::FragmentSpread(y)) => {
             let mapped_fragment_name = fragment_map
                 .get(&x.fragment_name)
                 .unwrap_or(&x.fragment_name);
-            *mapped_fragment_name == y.fragment_name && x.directives == y.directives
+            *mapped_fragment_name == y.fragment_name
+                && same_directives(&x.directives, &y.directives)
         }
         (ast::Selection::InlineFragment(x), ast::Selection::InlineFragment(y)) => {
             x.type_condition == y.type_condition
-                && x.directives == y.directives
+                && same_directives(&x.directives, &y.directives)
                 && same_ast_selection_set_sorted(&x.selection_set, &y.selection_set, fragment_map)
         }
         _ => false,
@@ -1200,6 +1215,15 @@ mod ast_comparison_tests {
         assert!(super::same_ast_document(&ast_x, &ast_y).is_ok());
     }
 
+    #[test]
+    fn test_selection_directive_order() {
+        let op_x = r#"{ x @include(if:true) @skip(if:false) }"#;
+        let op_y = r#"{ x @skip(if:false) @include(if:true) }"#;
+        let ast_x = ast::Document::parse(op_x, "op_x").unwrap();
+        let ast_y = ast::Document::parse(op_y, "op_y").unwrap();
+        assert!(super::same_ast_document(&ast_x, &ast_y).is_ok());
+    }
+
     #[test]
     fn test_string_to_id_coercion_difference() {
         // JS QP coerces strings into integer for ID type, while Rust QP doesn't.

apollo-router/tests/integration/fixtures/query_planner_max_evaluated_plans.graphql

@@ -0,0 +1,73 @@
+# Composed from subgraphs with hash: a9236eee956ed7fc219b2212696478159ced7eea
+schema
+  @link(url: "https://specs.apollo.dev/link/v1.0")
+  @link(url: "https://specs.apollo.dev/join/v0.5", for: EXECUTION)
+{
+  query: Query
+}
+
+directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION
+
+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, contextArguments: [join__ContextArgument!]) 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
+
+input join__ContextArgument {
+  name: String!
+  type: String!
+  context: String!
+  selection: join__FieldValue!
+}
+
+scalar join__DirectiveArguments
+
+scalar join__FieldSet
+
+scalar join__FieldValue
+
+enum join__Graph {
+  SUBGRAPH1 @join__graph(name: "Subgraph1", url: "none")
+  SUBGRAPH2 @join__graph(name: "Subgraph2", url: "none")
+}
+
+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)
+{
+  t: T
+}
+
+type T
+  @join__type(graph: SUBGRAPH1, key: "id")
+  @join__type(graph: SUBGRAPH2, key: "id")
+{
+  id: ID!
+  v1: Int
+  v2: Int
+  v3: Int
+  v4: Int
+}

apollo-router/tests/integration/query_planner.rs

@@ -3,6 +3,8 @@ use std::path::PathBuf;
 use crate::integration::common::graph_os_enabled;
 use crate::integration::IntegrationTest;
 
+mod max_evaluated_plans;
+
 const PROMETHEUS_METRICS_CONFIG: &str = include_str!("telemetry/fixtures/prometheus.router.yaml");
 const LEGACY_QP: &str = "experimental_query_planner_mode: legacy";
 const NEW_QP: &str = "experimental_query_planner_mode: new";