Merge branch 'dev' into tninesling/cost-directives

apollo-router/src/configuration/mod.rs

@@ -218,16 +218,32 @@ pub(crate) enum ApolloMetricsGenerationMode {
 /// Query planner modes.
 #[derive(Clone, PartialEq, Eq, Default, Derivative, Serialize, Deserialize, JsonSchema)]
 #[derivative(Debug)]
-#[serde(rename_all = "lowercase")]
+#[serde(rename_all = "snake_case")]
 pub(crate) enum QueryPlannerMode {
     /// Use the new Rust-based implementation.
+    ///
+    /// Raises an error at Router startup if the the new planner does not support the schema
+    /// (such as using legacy Apollo Federation 1)
     New,
     /// Use the old JavaScript-based implementation.
     #[default]
     Legacy,
-    /// Use Rust-based and Javascript-based implementations side by side, logging warnings if the
-    /// implementations disagree.
+    /// Use primarily the Javascript-based implementation,
+    /// but also schedule background jobs to run the Rust implementation and compare results,
+    /// logging warnings if the implementations disagree.
+    ///
+    /// Raises an error at Router startup if the the new planner does not support the schema
+    /// (such as using legacy Apollo Federation 1)
     Both,
+    /// Use primarily the Javascript-based implementation,
+    /// but also schedule on a best-effort basis background jobs
+    /// to run the Rust implementation and compare results,
+    /// logging warnings if the implementations disagree.
+    ///
+    /// Falls back to `legacy` with a warning
+    /// if the the new planner does not support the schema
+    /// (such as using legacy Apollo Federation 1)
+    BothBestEffort,
 }
 
 impl<'de> serde::Deserialize<'de> for Configuration {

apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap

@@ -4386,7 +4386,7 @@ expression: "&schema"
       "description": "Query planner modes.",
       "oneOf": [
         {
-          "description": "Use the new Rust-based implementation.",
+          "description": "Use the new Rust-based implementation.\n\nRaises an error at Router startup if the the new planner does not support the schema (such as using legacy Apollo Federation 1)",
           "enum": [
             "new"
           ],
@@ -4400,11 +4400,18 @@ expression: "&schema"
           "type": "string"
         },
         {
-          "description": "Use Rust-based and Javascript-based implementations side by side, logging warnings if the implementations disagree.",
+          "description": "Use primarily the Javascript-based implementation, but also schedule background jobs to run the Rust implementation and compare results, logging warnings if the implementations disagree.\n\nRaises an error at Router startup if the the new planner does not support the schema (such as using legacy Apollo Federation 1)",
           "enum": [
             "both"
           ],
           "type": "string"
+        },
+        {
+          "description": "Use primarily the Javascript-based implementation, but also schedule on a best-effort basis background jobs to run the Rust implementation and compare results, logging warnings if the implementations disagree.\n\nFalls back to `legacy` with a warning if the the new planner does not support the schema (such as using legacy Apollo Federation 1)",
+          "enum": [
+            "both_best_effort"
+          ],
+          "type": "string"
         }
       ]
     },

apollo-router/src/query_planner/bridge_query_planner.rs

@@ -137,6 +137,16 @@ impl PlannerMode {
                     "expected Rust QP instance for `experimental_query_planner_mode: both`",
                 ),
             },
+            QueryPlannerMode::BothBestEffort => {
+                if let Some(rust) = rust_planner {
+                    Self::Both {
+                        js: Self::js(&schema.raw_sdl, configuration, old_planner).await?,
+                        rust,
+                    }
+                } else {
+                    Self::Js(Self::js(&schema.raw_sdl, configuration, old_planner).await?)
+                }
+            }
         })
     }
 
@@ -149,6 +159,16 @@ impl PlannerMode {
             QueryPlannerMode::New | QueryPlannerMode::Both => {
                 Ok(Some(Self::rust(schema, configuration)?))
             }
+            QueryPlannerMode::BothBestEffort => match Self::rust(schema, configuration) {
+                Ok(planner) => Ok(Some(planner)),
+                Err(error) => {
+                    tracing::warn!(
+                        "Failed to initialize the new query planner, \
+                         falling back to legacy: {error}"
+                    );
+                    Ok(None)
+                }
+            },
         }
     }
 

apollo-router/src/query_planner/caching_query_planner.rs

@@ -62,6 +62,7 @@ pub(crate) enum ConfigMode {
     // for now use the JS config as it expected to be identical to the Rust one
     Rust(Arc<QueryPlannerConfig>),
     Both(Arc<QueryPlannerConfig>),
+    BothBestEffort(Arc<QueryPlannerConfig>),
     Js(Arc<QueryPlannerConfig>),
 }
 
@@ -135,6 +136,9 @@ where
             crate::configuration::QueryPlannerMode::Both => {
                 ConfigMode::Both(Arc::new(configuration.js_query_planner_config()))
             }
+            crate::configuration::QueryPlannerMode::BothBestEffort => {
+                ConfigMode::BothBestEffort(Arc::new(configuration.js_query_planner_config()))
+            }
         };
         Ok(Self {
             cache,

apollo-router/src/router_factory.rs

@@ -532,6 +532,40 @@ caused by
     );
 }
 
+#[allow(clippy::too_many_arguments)]
+pub(crate) async fn add_plugin(
+    name: String,
+    factory: &PluginFactory,
+    plugin_config: &Value,
+    schema: Arc<String>,
+    supergraph_schema: Arc<Valid<apollo_compiler::Schema>>,
+    subgraph_schemas: Arc<HashMap<String, Arc<Valid<apollo_compiler::Schema>>>>,
+    notify: &crate::notification::Notify<String, crate::graphql::Response>,
+    plugin_instances: &mut Plugins,
+    errors: &mut Vec<ConfigurationError>,
+) {
+    match factory
+        .create_instance(
+            PluginInit::builder()
+                .config(plugin_config.clone())
+                .supergraph_sdl(schema)
+                .supergraph_schema(supergraph_schema)
+                .subgraph_schemas(subgraph_schemas)
+                .notify(notify.clone())
+                .build(),
+        )
+        .await
+    {
+        Ok(plugin) => {
+            let _ = plugin_instances.insert(name, plugin);
+        }
+        Err(err) => errors.push(ConfigurationError::PluginConfiguration {
+            plugin: name,
+            error: err.to_string(),
+        }),
+    }
+}
+
 pub(crate) async fn create_plugins(
     configuration: &Configuration,
     schema: &Schema,
@@ -565,26 +599,18 @@ pub(crate) async fn create_plugins(
     // Use function-like macros to avoid borrow conflicts of captures
     macro_rules! add_plugin {
         ($name: expr, $factory: expr, $plugin_config: expr) => {{
-            match $factory
-                .create_instance(
-                    PluginInit::builder()
-                        .config($plugin_config)
-                        .supergraph_sdl(schema.as_string().clone())
-                        .supergraph_schema(supergraph_schema.clone())
-                        .subgraph_schemas(subgraph_schemas.clone())
-                        .notify(configuration.notify.clone())
-                        .build(),
-                )
-                .await
-            {
-                Ok(plugin) => {
-                    let _ = plugin_instances.insert($name, plugin);
-                }
-                Err(err) => errors.push(ConfigurationError::PluginConfiguration {
-                    plugin: $name,
-                    error: err.to_string(),
-                }),
-            }
+            add_plugin(
+                $name,
+                $factory,
+                &$plugin_config,
+                schema.as_string().clone(),
+                supergraph_schema.clone(),
+                subgraph_schemas.clone(),
+                &configuration.notify.clone(),
+                &mut plugin_instances,
+                &mut errors,
+            )
+            .await;
         }};
     }
 
@@ -842,7 +868,8 @@ fn can_use_with_experimental_query_planner(
 
             Ok(())
         }
-        crate::configuration::QueryPlannerMode::Legacy => Ok(()),
+        crate::configuration::QueryPlannerMode::Legacy
+        | crate::configuration::QueryPlannerMode::BothBestEffort => Ok(()),
     }
 }
 #[cfg(test)]

apollo-router/tests/integration/lifecycle.rs

@@ -460,3 +460,73 @@ fn test_plugin_ordering_push_trace(context: &Context, entry: String) {
         )
         .unwrap();
 }
+
+#[tokio::test(flavor = "multi_thread")]
+async fn fed1_schema_with_legacy_qp() {
+    let mut router = IntegrationTest::builder()
+        .config("experimental_query_planner_mode: legacy")
+        .supergraph("../examples/graphql/local.graphql")
+        .build()
+        .await;
+    router.start().await;
+    router.assert_started().await;
+    router.execute_default_query().await;
+    router.graceful_shutdown().await;
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn fed1_schema_with_new_qp() {
+    let mut router = IntegrationTest::builder()
+        .config("experimental_query_planner_mode: new")
+        .supergraph("../examples/graphql/local.graphql")
+        .build()
+        .await;
+    router.start().await;
+    router
+        .assert_log_contains(
+            "could not create router: \
+             The supergraph schema failed to produce a valid API schema: \
+             Supergraphs composed with federation version 1 are not supported.",
+        )
+        .await;
+    router.assert_shutdown().await;
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn fed1_schema_with_both_qp() {
+    let mut router = IntegrationTest::builder()
+        .config("experimental_query_planner_mode: both")
+        .supergraph("../examples/graphql/local.graphql")
+        .build()
+        .await;
+    router.start().await;
+    router
+        .assert_log_contains(
+            "could not create router: \
+             The supergraph schema failed to produce a valid API schema: \
+             Supergraphs composed with federation version 1 are not supported.",
+        )
+        .await;
+    router.assert_shutdown().await;
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn fed1_schema_with_both_best_effort_qp() {
+    let mut router = IntegrationTest::builder()
+        .config("experimental_query_planner_mode: both_best_effort")
+        .supergraph("../examples/graphql/local.graphql")
+        .build()
+        .await;
+    router.start().await;
+    router
+        .assert_log_contains(
+            "Failed to initialize the new query planner, falling back to legacy: \
+             The supergraph schema failed to produce a valid API schema: \
+             Supergraphs composed with federation version 1 are not supported. \
+             Please recompose your supergraph with federation version 2 or greater",
+        )
+        .await;
+    router.assert_started().await;
+    router.execute_default_query().await;
+    router.graceful_shutdown().await;
+}