always instantiate the traffic shaping plugin

.changesets/maint_geal_always_create_traffic_shaping.md

@@ -0,0 +1,5 @@
+### Always instanciate the traffic shaping plugin ([Issue #3327](https://github.com/apollographql/router/issues/3327))
+
+The traffic_shaping plugin is now always part of the plugins list and is always active, with default configuration.
+
+By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/3330

apollo-router/src/plugins/expose_query_plan.rs

@@ -122,7 +122,6 @@ register_plugin!("experimental", "expose_query_plan", ExposeQueryPlan);
 #[cfg(test)]
 mod tests {
     use once_cell::sync::Lazy;
-    use serde_json::Value as jValue;
     use serde_json_bytes::ByteString;
     use serde_json_bytes::Value;
     use tower::Service;
@@ -131,9 +130,7 @@ mod tests {
     use crate::graphql::Response;
     use crate::json_ext::Object;
     use crate::plugin::test::MockSubgraph;
-    use crate::plugin::DynPlugin;
-    use crate::query_planner::BridgeQueryPlanner;
-    use crate::services::PluggableSupergraphServiceBuilder;
+    use crate::MockedSubgraphs;
 
     static EXPECTED_RESPONSE_WITH_QUERY_PLAN: Lazy<Response> = Lazy::new(|| {
         serde_json::from_str(include_str!(
@@ -150,7 +147,7 @@ mod tests {
 
     static VALID_QUERY: &str = r#"query TopProducts($first: Int) { topProducts(first: $first) { upc name reviews { id product { name } author { id name } } } }"#;
 
-    async fn build_mock_supergraph(plugin: Box<dyn DynPlugin>) -> supergraph::BoxService {
+    async fn build_mock_supergraph(config: serde_json::Value) -> supergraph::BoxCloneService {
         let mut extensions = Object::new();
         extensions.insert("test", Value::String(ByteString::from("value")));
 
@@ -183,35 +180,32 @@ mod tests {
 
         let product_service = MockSubgraph::new(product_mocks).with_extensions(extensions);
 
-        let schema =
-            include_str!("../../../apollo-router-benchmarks/benches/fixtures/supergraph.graphql");
-        let planner = BridgeQueryPlanner::new(schema.to_string(), Default::default())
-            .await
-            .unwrap();
-
-        let builder = PluggableSupergraphServiceBuilder::new(planner);
-        let builder = builder
-            .with_dyn_plugin("experimental.expose_query_plan".to_string(), plugin)
-            .with_subgraph_service("accounts", account_service.clone())
-            .with_subgraph_service("reviews", review_service.clone())
-            .with_subgraph_service("products", product_service.clone());
-
-        builder.build().await.expect("should build").make().boxed()
-    }
-
-    async fn get_plugin(config: &jValue) -> Box<dyn DynPlugin> {
-        crate::plugin::plugins()
-            .find(|factory| factory.name == "experimental.expose_query_plan")
-            .expect("Plugin not found")
-            .create_instance_without_schema(config)
+        let subgraphs = MockedSubgraphs(
+            [
+                ("accounts", account_service),
+                ("reviews", review_service),
+                ("products", product_service),
+            ]
+            .into_iter()
+            .collect(),
+        );
+
+        crate::TestHarness::builder()
+            .schema(include_str!(
+                "../../../apollo-router-benchmarks/benches/fixtures/supergraph.graphql"
+            ))
+            .extra_plugin(subgraphs)
+            .configuration_json(config)
+            .unwrap()
+            .build_supergraph()
             .await
-            .expect("Plugin not created")
+            .unwrap()
     }
 
     async fn execute_supergraph_test(
         query: &str,
         body: &Response,
-        mut supergraph_service: supergraph::BoxService,
+        mut supergraph_service: supergraph::BoxCloneService,
     ) {
         let request = supergraph::Request::fake_builder()
             .query(query.to_string())
@@ -231,32 +225,47 @@ mod tests {
             .await
             .unwrap();
 
-        assert_eq!(response, *body);
+        assert_eq!(
+            serde_json::to_string(&response).unwrap(),
+            serde_json::to_string(body).unwrap()
+        );
     }
 
     #[tokio::test]
     async fn it_expose_query_plan() {
-        let plugin = get_plugin(&serde_json::json!(true)).await;
         execute_supergraph_test(
             VALID_QUERY,
             &EXPECTED_RESPONSE_WITH_QUERY_PLAN,
-            build_mock_supergraph(plugin).await,
+            build_mock_supergraph(serde_json::json! {{
+                "plugins": {
+                    "experimental.expose_query_plan": true
+                }
+            }})
+            .await,
         )
         .await;
         // let's try that again
-        let plugin = get_plugin(&serde_json::json!(true)).await;
         execute_supergraph_test(
             VALID_QUERY,
             &EXPECTED_RESPONSE_WITH_QUERY_PLAN,
-            build_mock_supergraph(plugin).await,
+            build_mock_supergraph(serde_json::json! {{
+                "plugins": {
+                    "experimental.expose_query_plan": true
+                }
+            }})
+            .await,
         )
         .await;
     }
 
     #[tokio::test]
     async fn it_doesnt_expose_query_plan() {
-        let plugin = get_plugin(&serde_json::json!(false)).await;
-        let supergraph = build_mock_supergraph(plugin).await;
+        let supergraph = build_mock_supergraph(serde_json::json! {{
+            "plugins": {
+                "experimental.expose_query_plan": false
+            }
+        }})
+        .await;
         execute_supergraph_test(
             VALID_QUERY,
             &EXPECTED_RESPONSE_WITHOUT_QUERY_PLAN,

apollo-router/src/router_factory.rs

@@ -9,7 +9,6 @@ use rustls::RootCertStore;
 use serde_json::Map;
 use serde_json::Value;
 use tower::service_fn;
-use tower::util::Either;
 use tower::BoxError;
 use tower::ServiceBuilder;
 use tower::ServiceExt;
@@ -185,39 +184,29 @@ impl RouterSuperServiceFactory for YamlRouterFactory {
                 .transpose()?
                 .or_else(|| tls_root_store.clone());
 
-            let subgraph_service = match plugins
+            let shaping = plugins
                 .iter()
                 .find(|i| i.0.as_str() == APOLLO_TRAFFIC_SHAPING)
                 .and_then(|plugin| (*plugin.1).as_any().downcast_ref::<TrafficShaping>())
-            {
-                Some(shaping) => Either::A(
-                    shaping.subgraph_service_internal(
-                        name,
-                        SubgraphService::new(
-                            name,
-                            configuration
-                                .apq
-                                .subgraph
-                                .subgraphs
-                                .get(name)
-                                .map(|apq| apq.enabled)
-                                .unwrap_or(configuration.apq.subgraph.all.enabled),
-                            subgraph_root_store,
-                            shaping.enable_subgraph_http2(name),
-                            subscription_plugin_conf.clone(),
-                            configuration.notify.clone(),
-                        ),
-                    ),
-                ),
-                None => Either::B(SubgraphService::new(
+                .expect("traffic shaping should always be part of the plugin list");
+
+            let subgraph_service = shaping.subgraph_service_internal(
+                name,
+                SubgraphService::new(
                     name,
-                    false,
+                    configuration
+                        .apq
+                        .subgraph
+                        .subgraphs
+                        .get(name)
+                        .map(|apq| apq.enabled)
+                        .unwrap_or(configuration.apq.subgraph.all.enabled),
                     subgraph_root_store,
-                    true,
+                    shaping.enable_subgraph_http2(name),
                     subscription_plugin_conf.clone(),
                     configuration.notify.clone(),
-                )),
-            };
+                ),
+            );
             builder = builder.with_subgraph_service(name, subgraph_service);
         }
 
@@ -308,39 +297,29 @@ impl YamlRouterFactory {
                 .transpose()?
                 .or_else(|| tls_root_store.clone());
 
-            let subgraph_service = match plugins
+            let shaping = plugins
                 .iter()
                 .find(|i| i.0.as_str() == APOLLO_TRAFFIC_SHAPING)
                 .and_then(|plugin| (*plugin.1).as_any().downcast_ref::<TrafficShaping>())
-            {
-                Some(shaping) => Either::A(
-                    shaping.subgraph_service_internal(
-                        name,
-                        SubgraphService::new(
-                            name,
-                            configuration
-                                .apq
-                                .subgraph
-                                .subgraphs
-                                .get(name)
-                                .map(|apq| apq.enabled)
-                                .unwrap_or(configuration.apq.subgraph.all.enabled),
-                            subgraph_root_store,
-                            shaping.enable_subgraph_http2(name),
-                            subscription_plugin_conf.clone(),
-                            configuration.notify.clone(),
-                        ),
-                    ),
-                ),
-                None => Either::B(SubgraphService::new(
+                .expect("traffic shaping should always be part of the plugin list");
+
+            let subgraph_service = shaping.subgraph_service_internal(
+                name,
+                SubgraphService::new(
                     name,
-                    false,
+                    configuration
+                        .apq
+                        .subgraph
+                        .subgraphs
+                        .get(name)
+                        .map(|apq| apq.enabled)
+                        .unwrap_or(configuration.apq.subgraph.all.enabled),
                     subgraph_root_store,
-                    true,
+                    shaping.enable_subgraph_http2(name),
                     subscription_plugin_conf.clone(),
                     configuration.notify.clone(),
-                )),
-            };
+                ),
+            );
             builder = builder.with_subgraph_service(name, subgraph_service);
         }
 
@@ -511,7 +490,7 @@ pub(crate) async fn create_plugins(
     add_mandatory_apollo_plugin!("csrf");
     add_mandatory_apollo_plugin!("headers");
     add_mandatory_apollo_plugin!("telemetry");
-    add_optional_apollo_plugin!("traffic_shaping");
+    add_mandatory_apollo_plugin!("traffic_shaping");
     add_optional_apollo_plugin!("forbid_mutations");
     add_optional_apollo_plugin!("subscription");
     add_optional_apollo_plugin!("override_subgraph_url");

apollo-router/src/services/supergraph_service.rs

@@ -11,7 +11,6 @@ use http::StatusCode;
 use indexmap::IndexMap;
 use router_bridge::planner::Planner;
 use tokio::sync::Mutex;
-use tower::util::Either;
 use tower::BoxError;
 use tower::ServiceBuilder;
 use tower::ServiceExt;
@@ -440,15 +439,14 @@ impl SupergraphCreator {
             .schema(self.schema.clone())
             .build();
 
-        let supergraph_service = match self
+        let shaping = self
             .plugins
             .iter()
             .find(|i| i.0.as_str() == APOLLO_TRAFFIC_SHAPING)
             .and_then(|plugin| plugin.1.as_any().downcast_ref::<TrafficShaping>())
-        {
-            Some(shaping) => Either::A(shaping.supergraph_service_internal(supergraph_service)),
-            None => Either::B(supergraph_service),
-        };
+            .expect("traffic shaping should always be part of the plugin list");
+
+        let supergraph_service = shaping.supergraph_service_internal(supergraph_service);
 
         ServiceBuilder::new()
             .layer(content_negociation::SupergraphLayer::default())