diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index a5bfc21957..292c66d41e 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -27,6 +27,13 @@ By [@USERNAME](https://github.com/USERNAME) in https://github.com/apollographql/ ## ❗ BREAKING ❗ + +### Remove QueryPlannerService ([PR #1552](https://github.com/apollographql/router/pull/1552)) + +This service was redundant, since anything done as part of the `QueryPlannerService` could be done either at the `SupergraphService` or at the `ExecutionService` level. + +By [@o0Ignition0o](https://github.com/o0Ignition0o) + ### Rename map_future_with_context to map_future_with_request_data ([PR #1547](https://github.com/apollographql/router/pull/1547)) The function is not very well named since it's in fact used to extract any data from a request for use in a future. This rename makes it clear. @@ -128,7 +135,6 @@ At the crate root: In the `apollo_router::plugin::Plugin` trait: * `router_service` → `supergraph_service` -* `query_planning_service` → `query_planner_service` A new `apollo_router::stages` module replaces `apollo_router::services` in the public API, reexporting its items and adding `BoxService` and `BoxCloneService` type aliases. @@ -143,24 +149,18 @@ mod supergraph { type BoxCloneService = tower::util::BoxCloneService; } -mod query_planner { - use apollo_router::services::QueryPlannerRequest as Request; - use apollo_router::services::QueryPlannerResponse as Response; - type BoxService = tower::util::BoxService; - type BoxCloneService = tower::util::BoxCloneService; +pub mod execution { + use tower::BoxError; - // Reachable from Request or Response: - use apollo_router::query_planner::QueryPlan; - use apollo_router::query_planner::QueryPlanOptions; - use apollo_router::services::QueryPlannerContent; - use apollo_router::spec::Query; -} + pub use crate::services::ExecutionRequest as Request; + pub use crate::services::ExecutionResponse as Response; + pub type BoxService = tower::util::BoxService; + pub type BoxCloneService = tower::util::BoxCloneService; + pub type Result = std::result::Result; -mod execution { - use apollo_router::services::ExecutionRequest as Request; - use apollo_router::services::ExecutionResponse as Response; - type BoxService = tower::util::BoxService; - type BoxCloneService = tower::util::BoxCloneService; + // Reachable from Request or Response: + pub use crate::query_planner::QueryPlan; + pub use crate::services::QueryPlannerContent; } mod subgraph { diff --git a/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs b/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs index cd22d81fe2..b1bac75cf1 100644 --- a/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs +++ b/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs @@ -4,7 +4,6 @@ use apollo_router::register_plugin; use apollo_router::stages::supergraph; {{#if type_basic}} use apollo_router::stages::execution; -use apollo_router::stages::query_planner; use apollo_router::stages::subgraph; {{/if}} {{#if type_auth}} @@ -62,14 +61,6 @@ impl Plugin for {{pascal_name}} { service } - // Delete this function if you are not customizing it. - fn query_planner_service( - &self, - service: query_planner::BoxService, - ) -> query_planner::BoxService { - service - } - // Delete this function if you are not customizing it. fn execution_service( &self, diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 90e9169bf3..a52d7de9e5 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -149,6 +149,13 @@ impl Configuration { plugins } + pub(crate) fn plugin_configuration(&self, plugin_name: &str) -> Option { + self.plugins() + .iter() + .find(|(name, _)| name == plugin_name) + .map(|(_, value)| value.clone()) + } + // checks that we can reload configuration from the current one to the new one pub(crate) fn is_compatible(&self, new: &Configuration) -> Result<(), &'static str> { if self.apollo_plugins.plugins.get(TELEMETRY_KEY) diff --git a/apollo-router/src/plugin/mod.rs b/apollo-router/src/plugin/mod.rs index e7d03bc500..b18f743ffd 100644 --- a/apollo-router/src/plugin/mod.rs +++ b/apollo-router/src/plugin/mod.rs @@ -41,7 +41,6 @@ use tower::ServiceBuilder; use crate::layers::ServiceBuilderExt; use crate::stages; use crate::stages::execution; -use crate::stages::query_planner; use crate::stages::subgraph; use crate::stages::supergraph; @@ -177,19 +176,6 @@ pub trait Plugin: Send + Sync + 'static + Sized { service } - /// This service handles generating the query plan for each incoming request. - /// Define `query_planner_service` if your customization needs to interact with query planning functionality (for example, to log query plan details). - /// - /// Query planning uses a cache that will store the result of the query planner and query planning plugins execution, so if the same query is - /// performed twice, the query planner plugins will onyl see it once. The caching key contains the query and operation name. If modifications - /// must be performed on the query, they should be done in router service plugins. - fn query_planner_service( - &self, - service: query_planner::BoxService, - ) -> query_planner::BoxService { - service - } - /// This service handles initiating the execution of a query plan after it's been generated. /// Define `execution_service` if your customization includes logic to govern execution (for example, if you want to block a particular query based on a policy decision). fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { @@ -240,17 +226,6 @@ pub(crate) trait DynPlugin: Send + Sync + 'static { /// For example, this is a good opportunity to perform JWT verification before allowing a request to proceed further. fn supergraph_service(&self, service: supergraph::BoxService) -> supergraph::BoxService; - /// This service handles generating the query plan for each incoming request. - /// Define `query_planner_service` if your customization needs to interact with query planning functionality (for example, to log query plan details). - /// - /// Query planning uses a cache that will store the result of the query planner and query planning plugins execution, so if the same query is - /// performed twice, the query planner plugins will onyl see it once. The caching key contains the query and operation name. If modifications - /// must be performed on the query, they should be done in router service plugins. - fn query_planner_service( - &self, - service: query_planner::BoxService, - ) -> query_planner::BoxService; - /// This service handles initiating the execution of a query plan after it's been generated. /// Define `execution_service` if your customization includes logic to govern execution (for example, if you want to block a particular query based on a policy decision). fn execution_service(&self, service: execution::BoxService) -> execution::BoxService; @@ -287,13 +262,6 @@ where self.supergraph_service(service) } - fn query_planner_service( - &self, - service: query_planner::BoxService, - ) -> query_planner::BoxService { - self.query_planner_service(service) - } - fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { self.execution_service(service) } diff --git a/apollo-router/src/plugin/test/mod.rs b/apollo-router/src/plugin/test/mod.rs index 2733d06598..7957078a2a 100644 --- a/apollo-router/src/plugin/test/mod.rs +++ b/apollo-router/src/plugin/test/mod.rs @@ -9,7 +9,6 @@ use std::sync::Arc; pub use mock::subgraph::MockSubgraph; pub use service::MockExecutionService; -pub use service::MockQueryPlannerService; pub use service::MockSubgraphService; pub use service::MockSupergraphService; use tower::util::BoxService; diff --git a/apollo-router/src/plugin/test/service.rs b/apollo-router/src/plugin/test/service.rs index 95d3c81228..3b049e0868 100644 --- a/apollo-router/src/plugin/test/service.rs +++ b/apollo-router/src/plugin/test/service.rs @@ -3,8 +3,6 @@ use crate::ExecutionRequest; use crate::ExecutionResponse; -use crate::QueryPlannerRequest; -use crate::QueryPlannerResponse; use crate::SubgraphRequest; use crate::SubgraphResponse; use crate::SupergraphRequest; @@ -46,6 +44,5 @@ macro_rules! mock_service { } mock_service!(Supergraph, SupergraphRequest, SupergraphResponse); -mock_service!(QueryPlanner, QueryPlannerRequest, QueryPlannerResponse); mock_service!(Execution, ExecutionRequest, ExecutionResponse); mock_service!(Subgraph, SubgraphRequest, SubgraphResponse); diff --git a/apollo-router/src/plugins/rhai.rs b/apollo-router/src/plugins/rhai.rs index b43d1baf87..4c4e6285e3 100644 --- a/apollo-router/src/plugins/rhai.rs +++ b/apollo-router/src/plugins/rhai.rs @@ -92,10 +92,6 @@ mod router { pub(crate) type Response = super::RhaiSupergraphResponse; } -mod query_planner { - pub(crate) use crate::stages::query_planner::*; -} - mod execution { pub(crate) use crate::stages::execution::*; pub(crate) type Response = super::RhaiExecutionResponse; @@ -344,26 +340,6 @@ impl Plugin for Rhai { shared_service.take_unwrap() } - fn query_planner_service( - &self, - service: query_planner::BoxService, - ) -> query_planner::BoxService { - const FUNCTION_NAME_SERVICE: &str = "query_planner_service"; - if !self.ast_has_function(FUNCTION_NAME_SERVICE) { - return service; - } - tracing::debug!("query_planner_service function found"); - let shared_service = Arc::new(Mutex::new(Some(service))); - if let Err(error) = self.run_rhai_service( - FUNCTION_NAME_SERVICE, - None, - ServiceStep::QueryPlanner(shared_service.clone()), - ) { - tracing::error!("service callback failed: {error}"); - } - shared_service.take_unwrap() - } - fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { const FUNCTION_NAME_SERVICE: &str = "execution_service"; if !self.ast_has_function(FUNCTION_NAME_SERVICE) { @@ -402,7 +378,6 @@ impl Plugin for Rhai { #[derive(Clone, Debug)] pub(crate) enum ServiceStep { Router(SharedMut), - QueryPlanner(SharedMut), Execution(SharedMut), Subgraph(SharedMut), } @@ -723,9 +698,6 @@ impl ServiceStep { .boxed() }) } - ServiceStep::QueryPlanner(service) => { - gen_map_request!(query_planner, service, rhai_service, callback); - } ServiceStep::Execution(service) => { //gen_map_request!(execution, service, rhai_service, callback); service.replace(|service| { @@ -895,9 +867,6 @@ impl ServiceStep { )) }) } - ServiceStep::QueryPlanner(service) => { - gen_map_response!(query_planner, service, rhai_service, callback); - } ServiceStep::Execution(service) => { //gen_map_response!(execution, service, rhai_service, callback); service.replace(|service| { @@ -1334,30 +1303,6 @@ impl Rhai { register_rhai_interface!(engine, router, execution, subgraph); - engine - .register_get_result("context", |obj: &mut SharedMut| { - Ok(obj.with_mut(|request| request.context.clone())) - }) - .register_get_result("context", |obj: &mut SharedMut| { - Ok(obj.with_mut(|response| response.context.clone())) - }); - - engine - .register_set_result( - "context", - |obj: &mut SharedMut, context: Context| { - obj.with_mut(|request| request.context = context); - Ok(()) - }, - ) - .register_set_result( - "context", - |obj: &mut SharedMut, context: Context| { - obj.with_mut(|response| response.context = context); - Ok(()) - }, - ); - engine } diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index 66d7a84f84..831f97160c 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -68,12 +68,10 @@ use crate::query_planner::USAGE_REPORTING; use crate::register_plugin; use crate::stages; use crate::stages::execution; -use crate::stages::query_planner; use crate::stages::subgraph; use crate::stages::supergraph; use crate::Context; use crate::ExecutionRequest; -use crate::QueryPlannerRequest; use crate::SubgraphRequest; use crate::SubgraphResponse; use crate::SupergraphRequest; @@ -258,25 +256,6 @@ impl Plugin for Telemetry { .boxed() } - fn query_planner_service( - &self, - service: query_planner::BoxService, - ) -> query_planner::BoxService { - ServiceBuilder::new() - .instrument(move |req: &QueryPlannerRequest| { - let query = req.query.clone(); - let operation_name = req.operation_name.clone().unwrap_or_default(); - - info_span!("query_planning", - graphql.document = query.as_str(), - graphql.operation.name = operation_name.as_str(), - "otel.kind" = %SpanKind::Internal - ) - }) - .service(service) - .boxed() - } - fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { ServiceBuilder::new() .instrument(move |req: &ExecutionRequest| { diff --git a/apollo-router/src/plugins/traffic_shaping/mod.rs b/apollo-router/src/plugins/traffic_shaping/mod.rs index cdddde1f37..2f5c24ac66 100644 --- a/apollo-router/src/plugins/traffic_shaping/mod.rs +++ b/apollo-router/src/plugins/traffic_shaping/mod.rs @@ -38,10 +38,9 @@ use crate::plugin::PluginInit; use crate::plugins::traffic_shaping::deduplication::QueryDeduplicationLayer; use crate::register_plugin; use crate::services::subgraph_service::Compression; -use crate::stages::query_planner; use crate::stages::subgraph; use crate::stages::supergraph; -use crate::QueryPlannerRequest; +use crate::Configuration; use crate::SubgraphRequest; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30); @@ -95,7 +94,10 @@ struct RouterShaping { #[derive(PartialEq, Debug, Clone, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] -struct Config { + +// FIXME: This struct is pub(crate) because we need its configuration in the query planner service. +// Remove this once the configuration yml changes. +pub(crate) struct Config { #[serde(default)] /// Applied at the router level router: Option, @@ -132,7 +134,9 @@ impl Merge for RateLimitConf { } } -struct TrafficShaping { +// FIXME: This struct is pub(crate) because we need its configuration in the query planner service. +// Remove this once the configuration yml changes. +pub(crate) struct TrafficShaping { config: Config, rate_limit_router: Option, rate_limit_subgraphs: Mutex>, @@ -232,22 +236,6 @@ impl Plugin for TrafficShaping { service } } - - fn query_planner_service( - &self, - service: query_planner::BoxService, - ) -> query_planner::BoxService { - if matches!(self.config.deduplicate_variables, Some(true)) { - service - .map_request(|mut req: QueryPlannerRequest| { - req.query_plan_options.enable_deduplicate_variables = true; - req - }) - .boxed() - } else { - service - } - } } impl TrafficShaping { @@ -260,6 +248,15 @@ impl TrafficShaping { } } +impl TrafficShaping { + pub(crate) fn get_configuration_deduplicate_variables(configuration: &Configuration) -> bool { + configuration + .plugin_configuration("apollo.traffic_shaping") + .map(|conf| conf.get("deduplicate_variables") == Some(&serde_json::Value::Bool(true))) + .unwrap_or_default() + } +} + register_plugin!("apollo", "traffic_shaping", TrafficShaping); #[cfg(test)] @@ -279,6 +276,7 @@ mod test { use crate::plugin::test::MockSubgraph; use crate::plugin::test::MockSupergraphService; use crate::plugin::DynPlugin; + use crate::Configuration; use crate::PluggableSupergraphServiceBuilder; use crate::Schema; use crate::SupergraphRequest; @@ -354,7 +352,16 @@ mod test { ); let schema: Arc = Arc::new(Schema::parse(schema, &Default::default()).unwrap()); - let builder = PluggableSupergraphServiceBuilder::new(schema.clone()); + let config: Configuration = serde_yaml::from_str( + r#" + traffic_shaping: + deduplicate_variables: true + "#, + ) + .unwrap(); + + let builder = PluggableSupergraphServiceBuilder::new(schema.clone()) + .with_configuration(Arc::new(config)); let builder = builder .with_dyn_plugin("apollo.traffic_shaping".to_string(), plugin) diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index af997a1fb5..98749105ef 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -19,6 +19,7 @@ use super::QueryKey; use super::QueryPlanOptions; use crate::error::QueryPlannerError; use crate::introspection::Introspection; +use crate::plugins::traffic_shaping::TrafficShaping; use crate::services::QueryPlannerContent; use crate::*; @@ -33,6 +34,7 @@ pub(crate) struct BridgeQueryPlanner { schema: Arc, introspection: Option>, configuration: Arc, + deduplicate_variables: bool, } impl BridgeQueryPlanner { @@ -41,6 +43,9 @@ impl BridgeQueryPlanner { introspection: Option>, configuration: Arc, ) -> Result { + // FIXME: The variables deduplication parameter lives in the traffic_shaping section of the config + let deduplicate_variables = + TrafficShaping::get_configuration_deduplicate_variables(&configuration); Ok(Self { planner: Arc::new( Planner::new( @@ -56,6 +61,7 @@ impl BridgeQueryPlanner { schema, introspection, configuration, + deduplicate_variables, }) } @@ -94,7 +100,6 @@ impl BridgeQueryPlanner { &self, query: String, operation: Option, - options: QueryPlanOptions, mut selections: Query, ) -> Result { let planner_result = self @@ -116,7 +121,9 @@ impl BridgeQueryPlanner { plan: Arc::new(query_planner::QueryPlan { usage_reporting, root: node, - options, + options: QueryPlanOptions { + enable_deduplicate_variables: self.deduplicate_variables, + }, }), query: Arc::new(selections), }) @@ -150,11 +157,7 @@ impl Service for BridgeQueryPlanner { let this = self.clone(); let fut = async move { match this - .get(( - req.query.clone(), - req.operation_name.to_owned(), - req.query_plan_options, - )) + .get((req.query.clone(), req.operation_name.to_owned())) .await { Ok(query_planner_content) => Ok(QueryPlannerResponse::new( @@ -178,7 +181,7 @@ impl BridgeQueryPlanner { return self.introspection(key.0).await; } - self.plan(key.0, key.1, key.2, selections).await + self.plan(key.0, key.1, selections).await } } @@ -207,11 +210,7 @@ mod tests { .await .unwrap(); let result = planner - .get(( - include_str!("testdata/query.graphql").into(), - None, - Default::default(), - )) + .get((include_str!("testdata/query.graphql").into(), None)) .await .unwrap(); if let QueryPlannerContent::Plan { plan, .. } = result { @@ -237,7 +236,6 @@ mod tests { .get(( "fragment UnusedTestFragment on User { id } query { me { id } }".to_string(), None, - Default::default(), )) .await .unwrap_err(); @@ -284,7 +282,6 @@ mod tests { .plan( include_str!("testdata/unknown_introspection_query.graphql").into(), None, - QueryPlanOptions::default(), Query::default(), ) .await @@ -311,7 +308,7 @@ mod tests { ) .await .unwrap(); - let result = planner.get(("".into(), None, Default::default())).await; + let result = planner.get(("".into(), None)).await; assert_eq!( "couldn't plan query: query validation errors: Syntax Error: Unexpected .", diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs index 9d7199c3e6..84a4ff7abe 100644 --- a/apollo-router/src/query_planner/caching_query_planner.rs +++ b/apollo-router/src/query_planner/caching_query_planner.rs @@ -53,11 +53,7 @@ where fn call(&mut self, request: QueryPlannerRequest) -> Self::Future { let mut qp = self.clone(); Box::pin(async move { - let key = ( - request.query.clone(), - request.operation_name.to_owned(), - request.query_plan_options.clone(), - ); + let key = (request.query.clone(), request.operation_name.to_owned()); let context = request.context.clone(); let entry = qp.cache.get(&key).await; if entry.is_first() { diff --git a/apollo-router/src/query_planner/mod.rs b/apollo-router/src/query_planner/mod.rs index 27b659ee96..dc92d896d9 100644 --- a/apollo-router/src/query_planner/mod.rs +++ b/apollo-router/src/query_planner/mod.rs @@ -43,7 +43,7 @@ pub(crate) struct QueryPlanOptions { /// /// This type consists of a query string, an optional operation string and the /// [`QueryPlanOptions`]. -pub(crate) type QueryKey = (String, Option, QueryPlanOptions); +pub(crate) type QueryKey = (String, Option); /// A plan for a given GraphQL query #[derive(Debug)] diff --git a/apollo-router/src/services/mod.rs b/apollo-router/src/services/mod.rs index 3f7b07971a..eec8564101 100644 --- a/apollo-router/src/services/mod.rs +++ b/apollo-router/src/services/mod.rs @@ -32,7 +32,6 @@ use crate::json_ext::Path; use crate::json_ext::Value; use crate::query_planner::fetch::OperationKind; use crate::query_planner::QueryPlan; -use crate::query_planner::QueryPlanOptions; use crate::spec::Query; use crate::Context; @@ -329,13 +328,10 @@ impl SupergraphResponse { assert_impl_all!(QueryPlannerRequest: Send); /// [`Context`] for the request. #[derive(Clone, Debug)] -pub struct QueryPlannerRequest { - pub query: String, - pub operation_name: Option, - /// Query plan options - pub(crate) query_plan_options: QueryPlanOptions, - - pub context: Context, +pub(crate) struct QueryPlannerRequest { + pub(crate) query: String, + pub(crate) operation_name: Option, + pub(crate) context: Context, } #[buildstructor::buildstructor] @@ -343,7 +339,7 @@ impl QueryPlannerRequest { /// This is the constructor (or builder) to use when constructing a real QueryPlannerRequest. /// /// Required parameters are required in non-testing code to create a QueryPlannerRequest. - #[builder(visibility = "pub")] + #[builder] pub(crate) fn new( query: String, operation_name: Option, @@ -352,7 +348,6 @@ impl QueryPlannerRequest { Self { query, operation_name, - query_plan_options: QueryPlanOptions::default(), context, } } @@ -360,9 +355,9 @@ impl QueryPlannerRequest { assert_impl_all!(QueryPlannerResponse: Send); /// [`Context`] and [`QueryPlan`] for the response. -pub struct QueryPlannerResponse { +pub(crate) struct QueryPlannerResponse { pub(crate) content: QueryPlannerContent, - pub context: Context, + pub(crate) context: Context, } /// Query, QueryPlan and Introspection data. @@ -387,32 +382,6 @@ impl QueryPlannerResponse { pub(crate) fn new(content: QueryPlannerContent, context: Context) -> QueryPlannerResponse { Self { content, context } } - - /// This is the constructor (or builder) to use when constructing a QueryPlannerResponse that represents a global error. - /// It has no path and no response data. - /// This is useful for things such as authentication errors. - #[allow(unused_variables)] - #[builder(visibility = "pub")] - fn error_new( - errors: Vec, - status_code: Option, - headers: MultiMap, - context: Context, - ) -> Result { - tracing::warn!("no way to propagate error response from QueryPlanner"); - Ok(QueryPlannerResponse::new( - QueryPlannerContent::Plan { - plan: Arc::new(QueryPlan::fake_builder().build()), - query: Arc::new(Query::default()), - }, - context, - )) - } - - /// Get a reference of the query plan - pub fn query_plan(&self) -> &QueryPlannerContent { - &self.content - } } assert_impl_all!(SubgraphRequest: Send); diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index a78ee62838..81b382dcfc 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -14,7 +14,7 @@ use futures::TryFutureExt; use http::StatusCode; use indexmap::IndexMap; use lazy_static::__Deref; -use tower::buffer::Buffer; +use opentelemetry::trace::SpanKind; use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; @@ -36,7 +36,6 @@ use crate::graphql::Response; use crate::http_ext::Request; use crate::introspection::Introspection; use crate::json_ext::ValueExt; -use crate::layers::DEFAULT_BUFFER_SIZE; use crate::plugin::DynPlugin; use crate::plugin::Handler; use crate::query_planner::BridgeQueryPlanner; @@ -44,7 +43,6 @@ use crate::query_planner::CachingQueryPlanner; use crate::router_factory::SupergraphServiceFactory; use crate::services::layers::apq::APQLayer; use crate::services::layers::ensure_query_presence::EnsureQueryPresence; -use crate::stages::query_planner; use crate::Configuration; use crate::ExecutionRequest; use crate::ExecutionResponse; @@ -59,20 +57,18 @@ pub(crate) type Plugins = IndexMap>; /// Containing [`Service`] in the request lifecyle. #[derive(Clone)] -pub(crate) struct SupergraphService { - query_planner_service: QueryPlannerService, +pub(crate) struct SupergraphService { execution_service_factory: ExecutionFactory, - ready_query_planner_service: Option, + query_planner_service: CachingQueryPlanner, + ready_query_planner_service: Option>, schema: Arc, } #[buildstructor::buildstructor] -impl - SupergraphService -{ +impl SupergraphService { #[builder] pub(crate) fn new( - query_planner_service: QueryPlannerService, + query_planner_service: CachingQueryPlanner, execution_service_factory: ExecutionFactory, schema: Arc, ) -> Self { @@ -85,14 +81,8 @@ impl } } -impl Service - for SupergraphService +impl Service for SupergraphService where - QueryPlannerService: Service - + Clone - + Send - + 'static, - QueryPlannerService::Future: Send + 'static, ExecutionFactory: ExecutionServiceFactory, { type Response = SupergraphResponse; @@ -133,6 +123,11 @@ where .context(context) .build(), ) + .instrument(tracing::info_span!("query_planning", + graphql.document = body.query.clone().expect("the query presence was already checked by a plugin").as_str(), + graphql.operation.name = body.operation_name.clone().unwrap_or_default().as_str(), + "otel.kind" = %SpanKind::Internal + )) .await?; match content { @@ -333,7 +328,7 @@ impl PluggableSupergraphServiceBuilder { &mut self.plugins } - pub(crate) async fn build(mut self) -> Result { + pub(crate) async fn build(self) -> Result { // Note: The plugins are always applied in reverse, so that the // fold is applied in the correct sequence. We could reverse // the list of plugins, but we want them back in the original @@ -359,21 +354,8 @@ impl PluggableSupergraphServiceBuilder { BridgeQueryPlanner::new(self.schema.clone(), introspection, configuration) .await .map_err(ServiceBuildError::QueryPlannerError)?; - let query_planner_service = ServiceBuilder::new().service( - CachingQueryPlanner::new( - Buffer::new( - self.plugins - .iter_mut() - .rev() - .fold(bridge_query_planner.boxed(), |acc, (_, e)| { - e.query_planner_service(acc) - }), - DEFAULT_BUFFER_SIZE, - ), - plan_cache_limit, - ) - .await, - ); + let query_planner_service = + CachingQueryPlanner::new(bridge_query_planner, plan_cache_limit).await; let plugins = Arc::new(self.plugins); @@ -397,8 +379,7 @@ impl PluggableSupergraphServiceBuilder { /// A collection of services and data which may be used to create a "router". #[derive(Clone)] pub(crate) struct RouterCreator { - query_planner_service: - CachingQueryPlanner>, + query_planner_service: CachingQueryPlanner, subgraph_creator: Arc, schema: Arc, plugins: Arc, @@ -479,6 +460,8 @@ impl RouterCreator { pub(crate) fn test_service( &self, ) -> tower::util::BoxCloneService { + use tower::buffer::Buffer; + Buffer::new(self.make(), 512).boxed_clone() } } diff --git a/apollo-router/src/spec/mod.rs b/apollo-router/src/spec/mod.rs index 59144defc5..a120643786 100644 --- a/apollo-router/src/spec/mod.rs +++ b/apollo-router/src/spec/mod.rs @@ -7,7 +7,7 @@ mod selection; use displaydoc::Display; pub(crate) use field_type::*; pub(crate) use fragments::*; -pub use query::Query; +pub(crate) use query::Query; pub(crate) use schema::Schema; pub(crate) use selection::*; use thiserror::Error; diff --git a/apollo-router/src/stages.rs b/apollo-router/src/stages.rs index 7fc9d5421f..39e061067e 100644 --- a/apollo-router/src/stages.rs +++ b/apollo-router/src/stages.rs @@ -22,11 +22,11 @@ pub mod supergraph { pub type Result = std::result::Result; } -pub mod query_planner { +pub mod execution { use tower::BoxError; - pub use crate::services::QueryPlannerRequest as Request; - pub use crate::services::QueryPlannerResponse as Response; + pub use crate::services::ExecutionRequest as Request; + pub use crate::services::ExecutionResponse as Response; pub type BoxService = tower::util::BoxService; pub type BoxCloneService = tower::util::BoxCloneService; pub type Result = std::result::Result; @@ -34,17 +34,6 @@ pub mod query_planner { // Reachable from Request or Response: pub use crate::query_planner::QueryPlan; pub use crate::services::QueryPlannerContent; - pub use crate::spec::Query; -} - -pub mod execution { - use tower::BoxError; - - pub use crate::services::ExecutionRequest as Request; - pub use crate::services::ExecutionResponse as Response; - pub type BoxService = tower::util::BoxService; - pub type BoxCloneService = tower::util::BoxCloneService; - pub type Result = std::result::Result; } pub mod subgraph { diff --git a/apollo-router/src/test_harness.rs b/apollo-router/src/test_harness.rs index 511c122011..5ba3f2eb96 100644 --- a/apollo-router/src/test_harness.rs +++ b/apollo-router/src/test_harness.rs @@ -11,7 +11,6 @@ use crate::plugin::PluginInit; use crate::router_factory::SupergraphServiceConfigurator; use crate::router_factory::YamlSupergraphServiceFactory; use crate::stages::execution; -use crate::stages::query_planner; use crate::stages::subgraph; use crate::stages::supergraph; use crate::Schema; @@ -138,17 +137,6 @@ impl<'a> TestHarness<'a> { self.extra_plugin(SupergraphServicePlugin(callback)) } - /// Adds an ad-hoc plugin that has [`Plugin::query_planner_service`] implemented with `callback`. - pub fn extra_query_planner_plugin( - self, - callback: impl Fn(query_planner::BoxService) -> query_planner::BoxService - + Send - + Sync - + 'static, - ) -> Self { - self.extra_plugin(QueryPlannerServicePlugin(callback)) - } - /// Adds an ad-hoc plugin that has [`Plugin::execution_service`] implemented with `callback`. pub fn extra_execution_plugin( self, @@ -218,7 +206,6 @@ impl<'a> TestHarness<'a> { } struct SupergraphServicePlugin(F); -struct QueryPlannerServicePlugin(F); struct ExecutionServicePlugin(F); struct SubgraphServicePlugin(F); @@ -238,25 +225,6 @@ where } } -#[async_trait::async_trait] -impl Plugin for QueryPlannerServicePlugin -where - F: 'static + Send + Sync + Fn(query_planner::BoxService) -> query_planner::BoxService, -{ - type Config = (); - - async fn new(_: PluginInit) -> Result { - unreachable!() - } - - fn query_planner_service( - &self, - service: query_planner::BoxService, - ) -> query_planner::BoxService { - (self.0)(service) - } -} - #[async_trait::async_trait] impl Plugin for ExecutionServicePlugin where diff --git a/apollo-router/tests/fixtures/test_callbacks.rhai b/apollo-router/tests/fixtures/test_callbacks.rhai index edec0ef728..340cde9d5e 100644 --- a/apollo-router/tests/fixtures/test_callbacks.rhai +++ b/apollo-router/tests/fixtures/test_callbacks.rhai @@ -10,16 +10,6 @@ fn supergraph_service(service) { }); } -fn query_planner_service(service) { - log_info("query_planner_service setup"); - service.map_request(|request| { - log_info("from_query_planner_request"); - }); - service.map_response(|response| { - log_info("from_query_planner_response"); - }); -} - fn execution_service(service) { log_info("execution_service setup"); service.map_request(|request| { diff --git a/apollo-router/tests/rhai_tests.rs b/apollo-router/tests/rhai_tests.rs index 5e49e83090..df5ee89904 100644 --- a/apollo-router/tests/rhai_tests.rs +++ b/apollo-router/tests/rhai_tests.rs @@ -42,9 +42,6 @@ async fn all_rhai_callbacks_are_invoked() { "supergraph_service setup", "from_router_request", "from_router_response", - "query_planner_service setup", - "from_query_planner_response", - "from_query_planner_request", "execution_service setup", "from_execution_request", "from_execution_response", diff --git a/apollo-router/tests/snapshots/integration_tests__traced_basic_composition.snap b/apollo-router/tests/snapshots/integration_tests__traced_basic_composition.snap index aa12ef4934..f0f24774b1 100644 --- a/apollo-router/tests/snapshots/integration_tests__traced_basic_composition.snap +++ b/apollo-router/tests/snapshots/integration_tests__traced_basic_composition.snap @@ -59,8 +59,8 @@ expression: get_spans() } }, "children": { - "apollo_router::plugins::telemetry::query_planning": { - "name": "apollo_router::plugins::telemetry::query_planning", + "apollo_router::services::supergraph_service::query_planning": { + "name": "apollo_router::services::supergraph_service::query_planning", "record": { "entries": [ [ @@ -78,9 +78,9 @@ expression: get_spans() ], "metadata": { "name": "query_planning", - "target": "apollo_router::plugins::telemetry", + "target": "apollo_router::services::supergraph_service", "level": "INFO", - "module_path": "apollo_router::plugins::telemetry", + "module_path": "apollo_router::services::supergraph_service", "fields": { "names": [ "graphql.document", diff --git a/apollo-router/tests/snapshots/integration_tests__traced_basic_request.snap b/apollo-router/tests/snapshots/integration_tests__traced_basic_request.snap index 5bb33140e5..9e390714b7 100644 --- a/apollo-router/tests/snapshots/integration_tests__traced_basic_request.snap +++ b/apollo-router/tests/snapshots/integration_tests__traced_basic_request.snap @@ -59,8 +59,8 @@ expression: get_spans() } }, "children": { - "apollo_router::plugins::telemetry::query_planning": { - "name": "apollo_router::plugins::telemetry::query_planning", + "apollo_router::services::supergraph_service::query_planning": { + "name": "apollo_router::services::supergraph_service::query_planning", "record": { "entries": [ [ @@ -78,9 +78,9 @@ expression: get_spans() ], "metadata": { "name": "query_planning", - "target": "apollo_router::plugins::telemetry", + "target": "apollo_router::services::supergraph_service", "level": "INFO", - "module_path": "apollo_router::plugins::telemetry", + "module_path": "apollo_router::services::supergraph_service", "fields": { "names": [ "graphql.document", diff --git a/docs/source/customizations/native.mdx b/docs/source/customizations/native.mdx index aa06379c8e..8b6ec55826 100644 --- a/docs/source/customizations/native.mdx +++ b/docs/source/customizations/native.mdx @@ -102,13 +102,6 @@ impl Plugin for HelloWorld { service } - fn query_planner_service( - &mut self, - service: query_planner::BoxService, - ) -> query_planner::BoxService { - service - } - fn execution_service( &mut self, service: execution::BoxService, diff --git a/docs/source/customizations/overview.mdx b/docs/source/customizations/overview.mdx index 82ee79cb8c..a5f4a0a96c 100644 --- a/docs/source/customizations/overview.mdx +++ b/docs/source/customizations/overview.mdx @@ -61,7 +61,6 @@ Each Apollo Router service has a corresponding function that a customization can | Service | Function | Description | |---------|----------|-------------| | `SupergraphService` | `supergraph_service` |

This service runs at the very beginning and very end of the request lifecycle.

Define `supergraph_service` if your customization needs to interact at the earliest or latest point possible. For example, this is a good opportunity to perform JWT verification before allowing a request to proceed further.

| -| `QueryPlannerService` | `query_planner_service` |

This service handles generating the query plan for each incoming request.

Define `query_planner_service` if your customization needs to interact with query planning functionality (for example, to log query plan details).

| | `ExecutionService` | `execution_service` |

This service handles initiating the execution of a query plan after it's been generated.

Define `execution_service` if your customization includes logic to govern execution (for example, if you want to block a particular query based on a policy decision).

| | `SubgraphService` | `subgraph_service` | This service handles communication between the Apollo Router and your subgraphs. Define `subgraph_service` to configure this communication (for example, to dynamically add headers to pass to a subgraph). | diff --git a/docs/source/customizations/rhai.mdx b/docs/source/customizations/rhai.mdx index b5405f2964..9a55e2c4a2 100644 --- a/docs/source/customizations/rhai.mdx +++ b/docs/source/customizations/rhai.mdx @@ -100,7 +100,6 @@ fn supergraph_service(service) { Similar to native Rust plugins, Rhai scripts can hook into the Apollo Router's [four services](./overview/#how-customizations-work) that handle requests. Just like native Rust plugins, Rhai scripts use a single hook for each service. Like native Rust plugins, the script author can then choose to map requests/response and generally configure the service for different behaviour. - `supergraph_service` - - `query_planner_service` - `execution_service` - `subgraph_service` @@ -110,7 +109,6 @@ Each function takes a single parameter: `service`, this is typed for each of the ```javascript fn supergraph_service(service) {} -fn query_planner_service(service) {} fn execution_service(service) {} fn subgraph_service(service) {} ``` @@ -226,7 +224,7 @@ request.subgraph.headers.x-my-new-header = 42.to_string(); #### request.body.query -The request query is accessible. If modified make sure to do this before query planning is performed (i.e.: `supergraph_service()` or `query_planner_service()`) or the modification will have no effect on the query. For example, let's modify the query at the supergraph_service stage and turn it into a completely invalid query. +The request query is accessible. If modified make sure to do this before query planning is performed (i.e.: `supergraph_service()`) or the modification will have no effect on the query. For example, let's modify the query at the supergraph_service stage and turn it into a completely invalid query. ```javascript print(`${request.body.query}`); // log the query before modification diff --git a/examples/hello-world/src/hello_world.rs b/examples/hello-world/src/hello_world.rs index a596e8fedb..815181d2b9 100644 --- a/examples/hello-world/src/hello_world.rs +++ b/examples/hello-world/src/hello_world.rs @@ -2,7 +2,6 @@ use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; use apollo_router::stages::execution; -use apollo_router::stages::query_planner; use apollo_router::stages::subgraph; use apollo_router::stages::supergraph; use schemars::JsonSchema; @@ -53,15 +52,6 @@ impl Plugin for HelloWorld { .boxed() } - fn query_planner_service( - &self, - service: query_planner::BoxService, - ) -> query_planner::BoxService { - // This is the default implementation and does not modify the default service. - // The trait also has this implementation, and we just provide it here for illustration. - service - } - fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { //This is the default implementation and does not modify the default service. // The trait also has this implementation, and we just provide it here for illustration. diff --git a/examples/rhai-logging/src/rhai_logging.rhai b/examples/rhai-logging/src/rhai_logging.rhai index fd313f2ea4..4ab903fe95 100644 --- a/examples/rhai-logging/src/rhai_logging.rhai +++ b/examples/rhai-logging/src/rhai_logging.rhai @@ -24,15 +24,6 @@ fn supergraph_service(service) { service.map_response(response_callback); } -// At the query_planner_service stage, register callbacks for processing requests and -// responses. -fn query_planner_service(service) { - const request_callback = Fn("process_request"); - service.map_request(request_callback); - const response_callback = Fn("process_response"); - service.map_response(response_callback); -} - // At the execution_service stage, register callbacks for processing requests and // responses. fn execution_service(service) {