Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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
17 changes: 16 additions & 1 deletion apollo-federation/src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ impl From<SchemaRootKind> for String {
}
}

#[derive(Clone, Debug, strum_macros::Display, PartialEq, Eq)]
pub enum UnsupportedFeatureKind {
#[strum(to_string = "progressive overrides")]
ProgressiveOverrides,
#[strum(to_string = "defer")]
Defer,
#[strum(to_string = "context")]
Context,
#[strum(to_string = "alias")]
Alias,
}

#[derive(Debug, Clone, thiserror::Error)]
pub enum SingleFederationError {
#[error(
Expand Down Expand Up @@ -185,7 +197,10 @@ pub enum SingleFederationError {
#[error("{message}")]
OverrideOnInterface { message: String },
#[error("{message}")]
UnsupportedFeature { message: String },
UnsupportedFeature {
message: String,
kind: UnsupportedFeatureKind,
},
#[error("{message}")]
InvalidFederationSupergraph { message: String },
#[error("{message}")]
Expand Down
92 changes: 91 additions & 1 deletion apollo-federation/src/query_plan/query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::sync::Arc;

use apollo_compiler::collections::IndexMap;
use apollo_compiler::collections::IndexSet;
use apollo_compiler::schema::ExtendedType;
use apollo_compiler::validation::Valid;
use apollo_compiler::ExecutableDocument;
use apollo_compiler::Name;
Expand Down Expand Up @@ -46,6 +47,10 @@ use crate::utils::logging::snapshot;
use crate::ApiSchemaOptions;
use crate::Supergraph;

pub(crate) const OVERRIDE_LABEL_ARG_NAME: &str = "overrideLabel";
pub(crate) const CONTEXT_DIRECTIVE: &str = "context";
pub(crate) const JOIN_FIELD: &str = "join__field";

#[derive(Debug, Clone, Hash)]
pub struct QueryPlannerConfig {
/// Whether the query planner should try to reused the named fragments of the planned query in
Expand Down Expand Up @@ -208,6 +213,7 @@ impl QueryPlanner {
config: QueryPlannerConfig,
) -> Result<Self, FederationError> {
config.assert_valid();
Self::check_unsupported_features(supergraph)?;

let supergraph_schema = supergraph.schema.clone();
let api_schema = supergraph.to_api_schema(ApiSchemaOptions {
Expand Down Expand Up @@ -533,6 +539,89 @@ impl QueryPlanner {
pub fn api_schema(&self) -> &ValidFederationSchema {
&self.api_schema
}

fn check_unsupported_features(supergraph: &Supergraph) -> Result<(), FederationError> {
// We have a *progressive* override when `join__field` has a
// non-null value for `overrideLabel` field.
//
// This looks at object types' fields and their directive
// applications, looking specifically for `@join__field`
// arguments list.
let has_progressive_overrides = supergraph
.schema
.schema()
.types
.values()
.filter_map(|extended_type| {
// The override label args can be only on ObjectTypes
if let ExtendedType::Object(object_type) = extended_type {
Some(object_type)
} else {
None
}
})
.flat_map(|object_type| &object_type.fields)
.flat_map(|(_, field)| {
field
.directives
.iter()
.filter(|d| d.name.as_str() == JOIN_FIELD)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we look up the correct name of the directive on the supergraph's .metadata()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On one hand, probably yes. On the other hand, we have enough occurrences of "join__ in the code base that I doubt anything works if a supregraph uses import renames.

Maybe composition supports renames when reading subgraphs be never emits them in a supergraph?

})
.any(|join_directive| {
if let Some(override_label_arg) =
join_directive.argument_by_name(OVERRIDE_LABEL_ARG_NAME)
{
// Any argument value for `overrideLabel` that's not
// null can be considered as progressive override usage
if !override_label_arg.is_null() {
return true;
}
return false;
}
false
});
if has_progressive_overrides {
let message = "\
`experimental_query_planner_mode: new` or `both` cannot yet \
be used with progressive overrides. \
Remove uses of progressive overrides to try the experimental query planner, \
otherwise switch back to `legacy` or `both_best_effort`.\
";
return Err(SingleFederationError::UnsupportedFeature {
message: message.to_owned(),
kind: crate::error::UnsupportedFeatureKind::ProgressiveOverrides,
}
.into());
}

// We will only check for `@context` direcive, since
// `@fromContext` can only be used if `@context` is already
// applied, and we assume a correctly composed supergraph.
//
// `@context` can only be applied on Object Types, Interface
// Types and Unions. For simplicity of this function, we just
// check all 'extended_type` directives.
let has_set_context = supergraph
.schema
.schema()
.types
.values()
.any(|extended_type| extended_type.directives().has(CONTEXT_DIRECTIVE));
if has_set_context {
let message = "\
`experimental_query_planner_mode: new` or `both` cannot yet \
be used with `@context`. \
Remove uses of `@context` to try the experimental query planner, \
otherwise switch back to `legacy` or `both_best_effort`.\
";
return Err(SingleFederationError::UnsupportedFeature {
message: message.to_owned(),
kind: crate::error::UnsupportedFeatureKind::Context,
}
.into());
}
Ok(())
}
}

fn compute_root_serial_dependency_graph(
Expand Down Expand Up @@ -767,8 +856,9 @@ fn compute_plan_for_defer_conditionals(
_parameters: &mut QueryPlanningParameters,
_defer_conditions: IndexMap<String, IndexSet<String>>,
) -> Result<Option<PlanNode>, FederationError> {
Err(SingleFederationError::Internal {
Err(SingleFederationError::UnsupportedFeature {
message: String::from("@defer is currently not supported"),
kind: crate::error::UnsupportedFeatureKind::Defer,
}
.into())
}
Expand Down
3 changes: 2 additions & 1 deletion apollo-federation/src/schema/field_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ fn check_absence_of_aliases(selection_set: &SelectionSet) -> Result<(), Federati
errors.push(SingleFederationError::UnsupportedFeature {
// PORT_NOTE: The JS version also quotes the directive name in the error message.
// For example, "aliases are not currently supported in @requires".
message: format!(r#"Cannot use alias "{alias}" in "{}": aliases are not currently supported in the used directive"#, field.field)
message: format!(r#"Cannot use alias "{alias}" in "{}": aliases are not currently supported in the used directive"#, field.field),
kind: crate::error::UnsupportedFeatureKind::Alias
}.into());
}
if let Some(selection_set) = &field.selection_set {
Expand Down
94 changes: 90 additions & 4 deletions apollo-router/src/query_planner/bridge_query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use apollo_compiler::ast;
use apollo_compiler::validation::Valid;
use apollo_compiler::Name;
use apollo_federation::error::FederationError;
use apollo_federation::error::SingleFederationError;
use apollo_federation::query_plan::query_planner::QueryPlanner;
use futures::future::BoxFuture;
use opentelemetry_api::metrics::MeterProvider as _;
Expand Down Expand Up @@ -64,6 +65,10 @@ use crate::Configuration;

pub(crate) const RUST_QP_MODE: &str = "rust";
const JS_QP_MODE: &str = "js";
const UNSUPPORTED_CONTEXT: &str = "context";
const UNSUPPORTED_OVERRIDES: &str = "overrides";
const UNSUPPORTED_FED1: &str = "fed1";
const INTERNAL_INIT_ERROR: &str = "internal";

#[derive(Clone)]
/// A query planner that calls out to the nodejs router-bridge query planner.
Expand Down Expand Up @@ -189,10 +194,34 @@ impl PlannerMode {
},
debug: Default::default(),
};
Ok(Arc::new(QueryPlanner::new(
schema.federation_supergraph(),
config,
)?))
let result = QueryPlanner::new(schema.federation_supergraph(), config);

match &result {
Err(FederationError::SingleFederationError {
inner: error,
trace: _,
}) => match error {
SingleFederationError::UnsupportedFederationVersion { .. } => {
metric_rust_qp_init(Some(UNSUPPORTED_FED1));
}
SingleFederationError::UnsupportedFeature { message: _, kind } => match kind {
apollo_federation::error::UnsupportedFeatureKind::ProgressiveOverrides => {
metric_rust_qp_init(Some(UNSUPPORTED_OVERRIDES))
}
apollo_federation::error::UnsupportedFeatureKind::Context => {
metric_rust_qp_init(Some(UNSUPPORTED_CONTEXT))
}
_ => metric_rust_qp_init(Some(INTERNAL_INIT_ERROR)),
},
_ => {
metric_rust_qp_init(Some(INTERNAL_INIT_ERROR));
}
},
Err(_) => metric_rust_qp_init(Some(INTERNAL_INIT_ERROR)),
Ok(_) => metric_rust_qp_init(None),
}

Ok(Arc::new(result?))
}

async fn js(
Expand Down Expand Up @@ -975,6 +1004,25 @@ pub(crate) fn metric_query_planning_plan_duration(planner: &'static str, start:
);
}

pub(crate) fn metric_rust_qp_init(init_error_kind: Option<&'static str>) {
if let Some(init_error_kind) = init_error_kind {
u64_counter!(
"apollo.router.lifecycle.query_planner.init",
"Rust query planner initialization",
1,
"init.error_kind" = init_error_kind,
"init.is_success" = false
);
} else {
u64_counter!(
"apollo.router.lifecycle.query_planner.init",
"Rust query planner initialization",
1,
"init.is_success" = true
);
}
Comment on lines +1005 to +1020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok for a metric attribute to be sometimes missing?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. but it could maybe be collapsed to a single attribute (init.error_kind = "none"). Though it's the same cardinality either way

}

#[cfg(test)]
mod tests {
use std::fs;
Expand Down Expand Up @@ -1617,4 +1665,42 @@ mod tests {
"planner" = "js"
);
}

#[test]
fn test_metric_rust_qp_initialization() {
metric_rust_qp_init(None);
assert_counter!(
"apollo.router.lifecycle.query_planner.init",
1,
"init.is_success" = true
);
metric_rust_qp_init(Some(UNSUPPORTED_CONTEXT));
assert_counter!(
"apollo.router.lifecycle.query_planner.init",
1,
"init.error_kind" = "context",
"init.is_success" = false
);
metric_rust_qp_init(Some(UNSUPPORTED_OVERRIDES));
assert_counter!(
"apollo.router.lifecycle.query_planner.init",
1,
"init.error_kind" = "overrides",
"init.is_success" = false
);
metric_rust_qp_init(Some(UNSUPPORTED_FED1));
assert_counter!(
"apollo.router.lifecycle.query_planner.init",
1,
"init.error_kind" = "fed1",
"init.is_success" = false
);
metric_rust_qp_init(Some(INTERNAL_INIT_ERROR));
assert_counter!(
"apollo.router.lifecycle.query_planner.init",
1,
"init.error_kind" = "internal",
"init.is_success" = false
);
}
}
Loading