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

docs: add signature generation configuration #5442

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
09e7412
Add raw info about operation signature configuration
Meschreiber Jun 12, 2024
6e791d6
Copyedit
Meschreiber Jun 13, 2024
a1dd372
use the JSON path implementation from serde-json-bytes (#5346)
Geal Jun 13, 2024
e3412dc
Rename the scaffold `Cargo.toml` files so that they don't confuse car…
dylan-apollo Jun 13, 2024
5de98ef
fixed a (type-position, schema) inconsistency in `compute_nodes_for_k…
duckki Jun 13, 2024
4fff2b4
Add example before/after signatures
Meschreiber Jun 13, 2024
6a7a384
Merge branch 'dev' into docs/add-signature-generation-configuration
Meschreiber Jun 13, 2024
e15d0d9
add selector for router service in custom telemetry (#5392)
bnjjj Jun 14, 2024
028b287
Add cost information to protobuf traces (#5430)
BrynCooke Jun 14, 2024
497c041
Remove Enterprise tag
Meschreiber Jun 14, 2024
7885a34
Merge branch 'dev' into docs/add-signature-generation-configuration
Meschreiber Jun 14, 2024
6be2b07
Fix bug in selection set equality checking (#5457)
sachindshinde Jun 14, 2024
bbe1f35
Fix bug in interface object type collection during query planner cons…
sachindshinde Jun 14, 2024
b4326a3
fix: fixed filter_on_schema (#5459)
duckki Jun 15, 2024
8f848ee
test: updated `plan_simple_root_field_query_for_multiple_subgraphs` t…
duckki Jun 15, 2024
1b3434c
Updated/added the comments on tests (#5445)
duckki Jun 15, 2024
3fc39ab
fixed `inputs_for_require` to use the entity_type/schema (#5460)
duckki Jun 15, 2024
0f388fc
fix(bridge_query_planner): change log level for mismatches (#5450)
lrlna Jun 17, 2024
e90de4b
Merge branch 'dev' into docs/add-signature-generation-configuration
Meschreiber Jun 17, 2024
4b2233e
prep release: v1.49.0-alpha.0
abernix Jun 12, 2024
dfe1bf2
prep release: v1.49.0-rc.0
bnjjj Jun 13, 2024
c132de4
prep release: v1.49.0-rc.1
lrlna Jun 17, 2024
085d924
fix(fed): bring in commit
duckki Jun 13, 2024
c4036af
fixed a (type-position, schema) inconsistency in `compute_nodes_for_k…
duckki Jun 13, 2024
4cb1fdb
Fix bug in interface object type collection during query planner cons…
sachindshinde Jun 14, 2024
6d2b65d
fixed `inputs_for_require` to use the entity_type/schema (#5460)
duckki Jun 15, 2024
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
18 changes: 18 additions & 0 deletions .changesets/feat_bnjjj_add_operation_name_router_selector.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
### Add selector for router service in custom telemetry ([PR #5392](https://github.com/apollographql/router/pull/5392))

Instead of having to access to the operation_name using the response_context at the router service, we now provide a selector for operation name at the router service in instrumentations.

example:

```yaml
telemetry:
instrumentation:
instruments:
router:
http.server.request.duration:
attributes:
graphql.operation.name:
operation_name: string
```

By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/5392
5 changes: 5 additions & 0 deletions .changesets/maint_bryn_demand_control_studio_traces.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
### Add cost information to protobuf traces ([PR #5430](https://github.com/apollographql/router/pull/5430))

If `experimental_demand_control` is enabled, cost information for queries is now exported on Apollo protobuf traces for display in studio.

By [@BrynCooke](https://github.com/BrynCooke) in https://github.com/apollographql/router/pull/5430
18 changes: 10 additions & 8 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ dependencies = [

[[package]]
name = "apollo-federation"
version = "1.48.1"
version = "1.49.0-rc.1"
dependencies = [
"apollo-compiler",
"derive_more",
Expand Down Expand Up @@ -266,7 +266,7 @@ dependencies = [

[[package]]
name = "apollo-router"
version = "1.48.1"
version = "1.49.0-rc.1"
dependencies = [
"access-json",
"anyhow",
Expand Down Expand Up @@ -430,7 +430,7 @@ dependencies = [

[[package]]
name = "apollo-router-benchmarks"
version = "1.48.1"
version = "1.49.0-rc.1"
dependencies = [
"apollo-parser",
"apollo-router",
Expand All @@ -446,7 +446,7 @@ dependencies = [

[[package]]
name = "apollo-router-scaffold"
version = "1.48.1"
version = "1.49.0-rc.1"
dependencies = [
"anyhow",
"cargo-scaffold",
Expand Down Expand Up @@ -5528,9 +5528,9 @@ dependencies = [

[[package]]
name = "regex"
version = "1.10.3"
version = "1.10.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b62dbe01f0b06f9d8dc7d49e05a0785f153b00b2c227856282f671e0318c9b15"
checksum = "c117dbdfde9c8308975b6a18d71f3f385c89461f7b3fb054288ecf2a2058ba4c"
dependencies = [
"aho-corasick",
"memchr",
Expand Down Expand Up @@ -6228,12 +6228,14 @@ dependencies = [

[[package]]
name = "serde_json_bytes"
version = "0.2.2"
version = "0.2.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "feb260b2939374fad6f939f803662d4971d03395fcd03752b674bdba06565779"
checksum = "9eb67259abd83636e3b2f4700ddc3ecfdbfe7b1b89e824da42bfea6530caf03b"
dependencies = [
"bytes",
"indexmap 2.2.3",
"jsonpath-rust",
"regex",
"serde",
"serde_json",
]
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ serde_json = { version = "1.0.114", features = [
"preserve_order",
"float_roundtrip",
] }
serde_json_bytes = { version = "0.2.2", features = ["preserve_order"] }
serde_json_bytes = { version = "0.2.3", features = ["preserve_order"] }
sha1 = "0.10.6"
tempfile = "3.10.0"
tokio = { version = "1.36.0", features = ["full"] }
Expand Down
2 changes: 1 addition & 1 deletion apollo-federation/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "apollo-federation"
version = "1.48.1"
version = "1.49.0-rc.1"
authors = ["The Apollo GraphQL Contributors"]
edition = "2021"
description = "Apollo Federation"
Expand Down
10 changes: 9 additions & 1 deletion apollo-federation/src/operation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,21 @@ impl Operation {
/// - For the type, stores the schema and the position in that schema instead of just the
/// `NamedType`.
/// - Stores selections in a map so they can be normalized efficiently.
#[derive(Debug, Clone, PartialEq, Eq)]
#[derive(Debug, Clone)]
pub(crate) struct SelectionSet {
pub(crate) schema: ValidFederationSchema,
pub(crate) type_position: CompositeTypeDefinitionPosition,
pub(crate) selections: Arc<SelectionMap>,
}

impl PartialEq for SelectionSet {
fn eq(&self, other: &Self) -> bool {
self.selections == other.selections
}
}

impl Eq for SelectionSet {}

mod selection_map {
use std::borrow::Cow;
use std::iter::Map;
Expand Down
2 changes: 0 additions & 2 deletions apollo-federation/src/operation/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1439,9 +1439,7 @@ fn add_at_path_merge_subselections() {
insta::assert_snapshot!(selection_set, @r#"{ a { b { c { d e(arg: 1) } } } }"#);
}

// TODO: `.add_at_path` should collapse unnecessary fragments
#[test]
#[ignore]
fn add_at_path_collapses_unnecessary_fragments() {
let schema =
apollo_compiler::Schema::parse_and_validate(ADD_AT_PATH_TEST_SCHEMA, "schema.graphql")
Expand Down
16 changes: 11 additions & 5 deletions apollo-federation/src/query_graph/graph_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3562,12 +3562,18 @@ impl OpPath {
match element.as_ref() {
OpPathElement::InlineFragment(fragment) => {
if let Some(type_condition) = &fragment.data().type_condition_position {
if schema.get_type(type_condition.type_name().clone()).is_ok() {
let updated_fragment = fragment.with_updated_type_condition(None);
filtered
.push(Arc::new(OpPathElement::InlineFragment(updated_fragment)));
if schema.get_type(type_condition.type_name().clone()).is_err() {
if element.directives().is_empty() {
continue; // skip this element
} else {
// Replace this element with an unconditioned inline fragment
let updated_fragment = fragment.with_updated_type_condition(None);
filtered.push(Arc::new(OpPathElement::InlineFragment(
updated_fragment,
)));
}
} else {
continue;
filtered.push(element.clone());
}
} else {
filtered.push(element.clone());
Expand Down
11 changes: 9 additions & 2 deletions apollo-federation/src/query_plan/fetch_dependency_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3062,6 +3062,10 @@ fn compute_nodes_for_key_resolution<'a>(
let dest = stack_item.tree.graph.node_weight(dest_id)?;
// We shouldn't have a key on a non-composite type
let source_type: CompositeTypeDefinitionPosition = source.type_.clone().try_into()?;
let source_schema: ValidFederationSchema = dependency_graph
.federated_query_graph
.schema_by_source(&source.source)?
.clone();
let dest_type: CompositeTypeDefinitionPosition = dest.type_.clone().try_into()?;
let dest_schema: ValidFederationSchema = dependency_graph
.federated_query_graph
Expand Down Expand Up @@ -3153,7 +3157,7 @@ fn compute_nodes_for_key_resolution<'a>(
let node =
FetchDependencyGraph::node_weight_mut(&mut dependency_graph.graph, stack_item.node_id)?;
let typename_field = Arc::new(OpPathElement::Field(Field::new_introspection_typename(
&dependency_graph.supergraph_schema,
&source_schema,
&source_type,
None,
)));
Expand Down Expand Up @@ -3852,6 +3856,7 @@ fn handle_requires(
let inputs = inputs_for_require(
dependency_graph,
entity_type_position.clone(),
entity_type_schema,
query_graph_edge_id,
context,
false,
Expand Down Expand Up @@ -4024,6 +4029,7 @@ fn defer_context_for_conditions(base_context: &DeferContext) -> DeferContext {
fn inputs_for_require(
fetch_dependency_graph: &mut FetchDependencyGraph,
entity_type_position: ObjectTypeDefinitionPosition,
entity_type_schema: ValidFederationSchema,
query_graph_edge_id: EdgeIndex,
context: &OpGraphPathContext,
include_key_inputs: bool,
Expand Down Expand Up @@ -4108,7 +4114,7 @@ fn inputs_for_require(
// should just use `entity_type` (that @interfaceObject type), not input type which will be an implementation the
// subgraph does not know in that particular case.
let mut key_inputs =
SelectionSet::for_composite_type(edge_conditions.schema.clone(), input_type.clone());
SelectionSet::for_composite_type(entity_type_schema, entity_type_position.into());
key_inputs.add_selection_set(&key_condition)?;

Ok((
Expand Down Expand Up @@ -4148,6 +4154,7 @@ fn add_post_require_inputs(
let (inputs, key_inputs) = inputs_for_require(
dependency_graph,
entity_type_position.clone(),
entity_type_schema.clone(),
query_graph_edge_id,
context,
true,
Expand Down
128 changes: 84 additions & 44 deletions apollo-federation/src/query_plan/query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,19 @@ use std::cell::Cell;
use std::num::NonZeroU32;
use std::sync::Arc;

use apollo_compiler::schema::ExtendedType;
use apollo_compiler::schema::Name;
use apollo_compiler::validation::Valid;
use apollo_compiler::ExecutableDocument;
use apollo_compiler::NodeStr;
use indexmap::IndexMap;
use indexmap::IndexSet;
use itertools::Itertools;
use petgraph::csr::NodeIndex;
use petgraph::stable_graph::IndexType;

use crate::error::FederationError;
use crate::error::SingleFederationError;
use crate::link::federation_spec_definition::FederationSpecDefinition;
use crate::link::federation_spec_definition::FEDERATION_INTERFACEOBJECT_DIRECTIVE_NAME_IN_SPEC;
use crate::link::spec::Identity;
use crate::operation::normalize_operation;
use crate::operation::NamedFragments;
use crate::operation::RebasedFragments;
Expand Down Expand Up @@ -219,34 +217,35 @@ impl QueryPlanner {
Some(true),
)?;

let metadata = supergraph_schema.metadata().unwrap();

let federation_link = metadata.for_identity(&Identity::federation_identity());
let interface_object_directive =
federation_link.map_or(FEDERATION_INTERFACEOBJECT_DIRECTIVE_NAME_IN_SPEC, |link| {
link.directive_name_in_schema(&FEDERATION_INTERFACEOBJECT_DIRECTIVE_NAME_IN_SPEC)
});

let is_interface_object =
|ty: &ExtendedType| ty.is_object() && ty.directives().has(&interface_object_directive);

let interface_types_with_interface_objects = supergraph
.schema
.get_types()
.filter_map(|position| match position {
TypeDefinitionPosition::Interface(interface_position) => Some(interface_position),
_ => None,
})
.filter(|position| {
query_graph.subgraphs().any(|(_name, schema)| {
schema
.schema()
.types
.get(&position.type_name)
.is_some_and(is_interface_object)
})
.map(|position| {
let is_interface_object = query_graph
.subgraphs()
.map(|(_name, schema)| {
let Some(position) = schema.try_get_type(position.type_name.clone()) else {
return Ok(false);
};
schema.is_interface_object_type(position)
})
.process_results(|mut iter| iter.any(|b| b))?;
Ok::<_, FederationError>((position, is_interface_object))
})
.collect::<IndexSet<_>>();
.process_results(|iter| {
iter.flat_map(|(position, is_interface_object)| {
if is_interface_object {
Some(position)
} else {
None
}
})
.collect::<IndexSet<_>>()
})?;

let is_inconsistent = |position: AbstractTypeDefinitionPosition| {
let mut sources = query_graph.subgraphs().filter_map(|(_name, subgraph)| {
Expand Down Expand Up @@ -995,10 +994,7 @@ type User
"###);
}

// TODO: This fails with "Subgraph unexpectedly does not use federation spec"
// which seems...unusual
#[test]
#[ignore]
fn plan_simple_root_field_query_for_multiple_subgraphs() {
let supergraph = Supergraph::new(TEST_SUPERGRAPH).unwrap();
let planner = QueryPlanner::new(&supergraph, Default::default()).unwrap();
Expand All @@ -1022,26 +1018,70 @@ type User
.unwrap();
let plan = planner.build_query_plan(&document, None).unwrap();
insta::assert_snapshot!(plan, @r###"
QueryPlan {
Parallel {
Fetch(service: "accounts") {
{
QueryPlan {
Parallel {
Fetch(service: "accounts") {
{
userById(id: 1) {
name
email
}
}
}
Fetch(service: "products") {
{
bestRatedProducts {
id
avg_rating
}
name
email
}
}
},
Sequence {
Fetch(service: "reviews") {
{
bestRatedProducts {
__typename
id
... on Book {
__typename
id
reviews {
rating
}
}
... on Movie {
__typename
id
reviews {
rating
}
}
}
}
},
Flatten(path: "bestRatedProducts.@") {
Fetch(service: "products") {
{
... on Book {
__typename
id
reviews {
rating
}
}
... on Movie {
__typename
id
reviews {
rating
}
}
} =>
{
... on Book {
avg_rating
}
... on Movie {
avg_rating
}
}
},
},
},
},
}
}
}
}
"###);
}

Expand Down
Loading