Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking: remove the query_planner service #1552

Merged
merged 16 commits into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -143,24 +149,18 @@ mod supergraph {
type BoxCloneService = tower::util::BoxCloneService<Request, Response, BoxError>;
}

mod query_planner {
use apollo_router::services::QueryPlannerRequest as Request;
use apollo_router::services::QueryPlannerResponse as Response;
type BoxService = tower::util::BoxService<Request, Response, BoxError>;
type BoxCloneService = tower::util::BoxCloneService<Request, Response, BoxError>;
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<Request, Response, BoxError>;
pub type BoxCloneService = tower::util::BoxCloneService<Request, Response, BoxError>;
pub type Result = std::result::Result<Response, BoxError>;

mod execution {
use apollo_router::services::ExecutionRequest as Request;
use apollo_router::services::ExecutionResponse as Response;
type BoxService = tower::util::BoxService<Request, Response, BoxError>;
type BoxCloneService = tower::util::BoxCloneService<Request, Response, BoxError>;
// Reachable from Request or Response:
pub use crate::query_planner::QueryPlan;
pub use crate::services::QueryPlannerContent;
}

mod subgraph {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions apollo-router/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,13 @@ impl Configuration {
plugins
}

pub(crate) fn plugin_configuration(&self, plugin_name: &str) -> Option<Value> {
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)
Expand Down
32 changes: 0 additions & 32 deletions apollo-router/src/plugin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand Down
1 change: 0 additions & 1 deletion apollo-router/src/plugin/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions apollo-router/src/plugin/test/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
55 changes: 0 additions & 55 deletions apollo-router/src/plugins/rhai.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -402,7 +378,6 @@ impl Plugin for Rhai {
#[derive(Clone, Debug)]
pub(crate) enum ServiceStep {
Router(SharedMut<router::BoxService>),
QueryPlanner(SharedMut<query_planner::BoxService>),
Execution(SharedMut<execution::BoxService>),
Subgraph(SharedMut<subgraph::BoxService>),
}
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -1334,30 +1303,6 @@ impl Rhai {

register_rhai_interface!(engine, router, execution, subgraph);

engine
.register_get_result("context", |obj: &mut SharedMut<query_planner::Request>| {
Ok(obj.with_mut(|request| request.context.clone()))
})
.register_get_result("context", |obj: &mut SharedMut<query_planner::Response>| {
Ok(obj.with_mut(|response| response.context.clone()))
});

engine
.register_set_result(
"context",
|obj: &mut SharedMut<query_planner::Request>, context: Context| {
obj.with_mut(|request| request.context = context);
Ok(())
},
)
.register_set_result(
"context",
|obj: &mut SharedMut<query_planner::Response>, context: Context| {
obj.with_mut(|response| response.context = context);
Ok(())
},
);

engine
}

Expand Down
21 changes: 0 additions & 21 deletions apollo-router/src/plugins/telemetry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
)
})
o0Ignition0o marked this conversation as resolved.
Show resolved Hide resolved
.service(service)
.boxed()
}

fn execution_service(&self, service: execution::BoxService) -> execution::BoxService {
ServiceBuilder::new()
.instrument(move |req: &ExecutionRequest| {
Expand Down
Loading