Skip to content

Commit

Permalink
always instantiate the traffic shaping plugin (#3330)
Browse files Browse the repository at this point in the history 
Fix #3327

that plugin is always part of the plugins list and should always be
active, with default configuration
  • Loading branch information
@Geal
Geal authored and bryn committed Jul 12, 2023
1 parent 46896e7 commit 279386c
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 95 deletions.
5 changes: 5 additions & 0 deletions .changesets/maint_geal_always_create_traffic_shaping.md
View file
Original file line number Diff line number Diff line change
@@ -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
79 changes: 44 additions & 35 deletions apollo-router/src/plugins/expose_query_plan.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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!(
Expand All @@ -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")));

Expand Down Expand Up @@ -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())
Expand All @@ -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,
Expand Down
87 changes: 33 additions & 54 deletions apollo-router/src/router_factory.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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");
Expand Down
10 changes: 4 additions & 6 deletions apollo-router/src/services/supergraph_service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 279386c

Please sign in to comment.