From ef08c20b97d987d72d9b09eab990fba2a481ebb4 Mon Sep 17 00:00:00 2001 From: Andrew McGivery Date: Thu, 23 Jan 2025 15:23:10 -0500 Subject: [PATCH 1/8] $request.headers and $response.headers for selection mapping in connectors --- .../sources/connect/validation/expression.rs | 41 +++++ .../src/sources/connect/variable.rs | 9 + .../plugins/connectors/handle_responses.rs | 162 +++++++++++++++++- .../src/plugins/connectors/make_requests.rs | 52 +++++- .../testdata/variables-subgraph.graphql | 12 +- .../connectors/testdata/variables.graphql | 4 +- .../src/plugins/connectors/tests/mod.rs | 19 +- .../config_new/connector/selectors.rs | 33 ++++ .../plugins/telemetry/config_new/events.rs | 65 +++++++ .../telemetry/config_new/instruments.rs | 36 ++++ .../src/plugins/telemetry/fmt_layer.rs | 61 +++++++ apollo-router/src/services/connect.rs | 13 ++ .../src/services/connector/request_service.rs | 4 + 13 files changed, 496 insertions(+), 15 deletions(-) diff --git a/apollo-federation/src/sources/connect/validation/expression.rs b/apollo-federation/src/sources/connect/validation/expression.rs index 8b4c399476..da3f36f1b4 100644 --- a/apollo-federation/src/sources/connect/validation/expression.rs +++ b/apollo-federation/src/sources/connect/validation/expression.rs @@ -53,6 +53,7 @@ impl<'schema> Context<'schema> { ), (Namespace::Config, Shape::unknown()), (Namespace::Context, Shape::unknown()), + (Namespace::Request, Shape::unknown()), ] .into_iter() .collect(); @@ -299,6 +300,46 @@ mod tests { assert!(validate(&expression(selection), &context).is_err()); } + #[rstest] + #[case("$args.int")] + #[case("$config.abc")] + #[case("$context.def")] + #[case("$request.headers.'apollo-client-name'")] + fn valid_variables(#[case] selection: &str) { + let schema = Schema::parse(SCHEMA, "schema").unwrap(); + let connect = name!("connect"); + let source = name!("source"); + let schema_info = SchemaInfo::new(&schema, "", &connect, &source); + let object = schema.get_object("Query").unwrap(); + let field = object.fields.get("aField").unwrap(); + let directive = field.directives.get("connect").unwrap(); + let coordinate = ConnectDirectiveCoordinate { + field_coordinate: FieldCoordinate { field, object }, + directive, + }; + let context = Context::for_connect_request(&schema_info, coordinate); + validate(&expression(selection), &context).unwrap(); + } + + #[rstest] + #[case("$status")] + #[case("$response.headers.etag")] + fn invalid_variables(#[case] selection: &str) { + let schema = Schema::parse(SCHEMA, "schema").unwrap(); + let connect = name!("connect"); + let source = name!("source"); + let schema_info = SchemaInfo::new(&schema, "", &connect, &source); + let object = schema.get_object("Query").unwrap(); + let field = object.fields.get("aField").unwrap(); + let directive = field.directives.get("connect").unwrap(); + let coordinate = ConnectDirectiveCoordinate { + field_coordinate: FieldCoordinate { field, object }, + directive, + }; + let context = Context::for_connect_request(&schema_info, coordinate); + assert!(validate(&expression(selection), &context).is_err()); + } + #[rstest] #[case("$args.int")] #[case("$args.string")] diff --git a/apollo-federation/src/sources/connect/variable.rs b/apollo-federation/src/sources/connect/variable.rs index 9c7c684cc8..b18903e92b 100644 --- a/apollo-federation/src/sources/connect/variable.rs +++ b/apollo-federation/src/sources/connect/variable.rs @@ -49,6 +49,8 @@ impl<'schema> VariableContext<'schema> { Namespace::Context, Namespace::Status, Namespace::This, + Namespace::Request, + Namespace::Response, ] } Phase::Request => { @@ -57,6 +59,7 @@ impl<'schema> VariableContext<'schema> { Namespace::Context, Namespace::This, Namespace::Args, + Namespace::Request, ] } } @@ -104,6 +107,8 @@ pub enum Namespace { Context, Status, This, + Request, + Response, } impl Namespace { @@ -114,6 +119,8 @@ impl Namespace { Self::Context => "$context", Self::Status => "$status", Self::This => "$this", + Self::Request => "$request", + Self::Response => "$response", } } } @@ -128,6 +135,8 @@ impl FromStr for Namespace { "$context" => Ok(Self::Context), "$status" => Ok(Self::Status), "$this" => Ok(Self::This), + "$request" => Ok(Self::Request), + "$response" => Ok(Self::Response), _ => Err(()), } } diff --git a/apollo-router/src/plugins/connectors/handle_responses.rs b/apollo-router/src/plugins/connectors/handle_responses.rs index caaa817353..b0f2c56e17 100644 --- a/apollo-router/src/plugins/connectors/handle_responses.rs +++ b/apollo-router/src/plugins/connectors/handle_responses.rs @@ -25,6 +25,7 @@ use crate::plugins::telemetry::config_new::events::log_event; use crate::plugins::telemetry::consts::OTEL_STATUS_CODE; use crate::plugins::telemetry::consts::OTEL_STATUS_CODE_ERROR; use crate::plugins::telemetry::consts::OTEL_STATUS_CODE_OK; +use crate::services::connect; use crate::services::connect::Response; use crate::services::connector; use crate::services::connector::request_service::transport::http::HttpResponse; @@ -76,6 +77,7 @@ impl RawResponse { connector: Arc, context: &Context, debug_context: &Option>>, + original_request: Arc, ) -> connector::request_service::Response { let mapped_response = match self { RawResponse::Error { error, key } => MappedResponse::Error { error, key }, @@ -90,6 +92,8 @@ impl RawResponse { connector.config.as_ref(), context, Some(parts.status.as_u16()), + original_request, + Some(&parts), ); let (res, apply_to_errors) = key.selection().apply_with_vars(&data, &inputs); @@ -297,6 +301,7 @@ pub(crate) async fn process_response( context: &Context, debug_request: Option, debug_context: &Option>>, + original_request: Arc, ) -> connector::request_service::Response { match result { // This occurs when we short-circuit the request when over the limit @@ -346,7 +351,7 @@ pub(crate) async fn process_response( }; if is_success { Span::current().record(OTEL_STATUS_CODE, OTEL_STATUS_CODE_OK); - raw.map_response(result, connector, context, debug_context) + raw.map_response(result, connector, context, debug_context, original_request) } else { Span::current().record(OTEL_STATUS_CODE, OTEL_STATUS_CODE_ERROR); raw.map_error(result, connector, context, debug_context) @@ -539,6 +544,8 @@ mod tests { use std::sync::Arc; use apollo_compiler::name; + use apollo_compiler::ExecutableDocument; + use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -549,8 +556,10 @@ mod tests { use insta::assert_debug_snapshot; use url::Url; + use crate::graphql; use crate::plugins::connectors::handle_responses::process_response; use crate::plugins::connectors::make_requests::ResponseKey; + use crate::query_planner::fetch::Variables; use crate::services::router; use crate::services::router::body::RouterBody; use crate::Context; @@ -600,6 +609,34 @@ mod tests { selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let original_request = Arc::new(original_request); + let res = super::aggregate_responses(vec![ process_response( Ok(response1), @@ -608,6 +645,7 @@ mod tests { &Context::default(), None, &None, + original_request.clone(), ) .await .mapped_response, @@ -618,6 +656,7 @@ mod tests { &Context::default(), None, &None, + original_request, ) .await .mapped_response, @@ -700,6 +739,34 @@ mod tests { selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let original_request = Arc::new(original_request); + let res = super::aggregate_responses(vec![ process_response( Ok(response1), @@ -708,6 +775,7 @@ mod tests { &Context::default(), None, &None, + original_request.clone(), ) .await .mapped_response, @@ -718,6 +786,7 @@ mod tests { &Context::default(), None, &None, + original_request, ) .await .mapped_response, @@ -810,6 +879,35 @@ mod tests { selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + + let original_request = Arc::new(original_request); + let res = super::aggregate_responses(vec![ process_response( Ok(response1), @@ -818,6 +916,7 @@ mod tests { &Context::default(), None, &None, + original_request.clone(), ) .await .mapped_response, @@ -828,6 +927,7 @@ mod tests { &Context::default(), None, &None, + original_request, ) .await .mapped_response, @@ -942,6 +1042,34 @@ mod tests { selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let original_request = Arc::new(original_request); + let res = super::aggregate_responses(vec![ process_response( Ok(response_plaintext), @@ -950,6 +1078,7 @@ mod tests { &Context::default(), None, &None, + original_request.clone(), ) .await .mapped_response, @@ -960,6 +1089,7 @@ mod tests { &Context::default(), None, &None, + original_request.clone(), ) .await .mapped_response, @@ -970,6 +1100,7 @@ mod tests { &Context::default(), None, &None, + original_request.clone(), ) .await .mapped_response, @@ -980,6 +1111,7 @@ mod tests { &Context::default(), None, &None, + original_request, ) .await .mapped_response, @@ -1168,6 +1300,33 @@ mod tests { selection: Arc::new(JSONSelection::parse("$status").unwrap()), }; + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let res = super::aggregate_responses(vec![ process_response( Ok(response1), @@ -1176,6 +1335,7 @@ mod tests { &Context::default(), None, &None, + Arc::new(original_request), ) .await .mapped_response, diff --git a/apollo-router/src/plugins/connectors/make_requests.rs b/apollo-router/src/plugins/connectors/make_requests.rs index f8e204f594..5826f71b60 100644 --- a/apollo-router/src/plugins/connectors/make_requests.rs +++ b/apollo-router/src/plugins/connectors/make_requests.rs @@ -9,6 +9,7 @@ use apollo_federation::sources::connect::CustomConfiguration; use apollo_federation::sources::connect::EntityResolver; use apollo_federation::sources::connect::JSONSelection; use apollo_federation::sources::connect::Namespace; +use http::response::Parts; use parking_lot::Mutex; use serde_json_bytes::json; use serde_json_bytes::ByteString; @@ -44,6 +45,8 @@ impl RequestInputs { config: Option<&CustomConfiguration>, context: &Context, status: Option, + original_request: Arc, + response_parts: Option<&Parts>, ) -> IndexMap { let mut map = IndexMap::with_capacity_and_hasher(variables_used.len(), Default::default()); @@ -92,6 +95,45 @@ impl RequestInputs { } } + // Add headers from the original router request + if variables_used.contains(&Namespace::Request) { + let headers: Map = original_request + .supergraph_request + .headers() + .iter() + .map(|(key, value)| { + ( + key.as_str().into(), + value.to_str().unwrap_or_default().into(), + ) + }) + .collect(); + let request_object = json!({ + "headers": Value::Object(headers) + }); + map.insert(Namespace::Request.as_str().into(), request_object); + } + + // Add headers from the connectors response + if variables_used.contains(&Namespace::Response) { + if let Some(response_parts) = response_parts { + let headers: Map = response_parts + .headers + .iter() + .map(|(key, value)| { + ( + key.as_str().into(), + value.to_str().unwrap_or_default().into(), + ) + }) + .collect(); + let response_object = json!({ + "headers": Value::Object(headers) + }); + map.insert(Namespace::Response.as_str().into(), response_object); + } + } + map } } @@ -187,7 +229,7 @@ pub(crate) fn make_requests( connector, service_name, request_params, - &request, + request, debug, ) } @@ -197,10 +239,11 @@ fn request_params_to_requests( connector: Arc, service_name: &str, request_params: Vec, - original_request: &connect::Request, + original_request: connect::Request, debug: &Option>>, ) -> Result, MakeRequestError> { let mut results = vec![]; + let original_request = Arc::new(original_request); for response_key in request_params { let connector = connector.clone(); let (transport_request, mapping_problems) = make_request( @@ -210,8 +253,10 @@ fn request_params_to_requests( connector.config.as_ref(), &original_request.context, None, + original_request.clone(), + None, ), - original_request, + &original_request, debug, )?; @@ -222,6 +267,7 @@ fn request_params_to_requests( transport_request, key: response_key, mapping_problems, + original_request: original_request.clone(), }); } diff --git a/apollo-router/src/plugins/connectors/testdata/variables-subgraph.graphql b/apollo-router/src/plugins/connectors/testdata/variables-subgraph.graphql index f0745e80bb..9cd3003a38 100644 --- a/apollo-router/src/plugins/connectors/testdata/variables-subgraph.graphql +++ b/apollo-router/src/plugins/connectors/testdata/variables-subgraph.graphql @@ -1,9 +1,6 @@ extend schema @link(url: "https://specs.apollo.dev/federation/v2.10") - @link( - url: "https://specs.apollo.dev/connect/v0.1" - import: ["@connect", "@source"] - ) + @link(url: "https://specs.apollo.dev/connect/v0.1", import: ["@connect", "@source"]) @source( name: "v1" http: { @@ -20,7 +17,7 @@ type Query { @connect( source: "v1" http: { - POST: "/f?arg={$args.arg->slice(1)}&context={$context.value}&config={$config.value}" + POST: "/f?arg={$args.arg->slice(1)}&context={$context.value}&config={$config.value}&header={$request.headers.value}" headers: [ { name: "x-connect-context", value: "{$context.value}" } { name: "x-connect-config", value: "{$config.value}" } @@ -30,6 +27,7 @@ type Query { arg: $args.arg context: $context.value config: $config.value + request: $request.headers.value """ } selection: """ @@ -39,6 +37,8 @@ type Query { status: $status sibling: $("D") extra: $->echo({ arg: $args.arg, context: $context.value, config: $config.value, status: $status }) + request: $request.headers.value + response: $response.headers.value """ ) } @@ -50,6 +50,8 @@ type T { status: Int sibling: String extra: JSON + request: String + response: String f(arg: String): U @connect( source: "v1" diff --git a/apollo-router/src/plugins/connectors/testdata/variables.graphql b/apollo-router/src/plugins/connectors/testdata/variables.graphql index eba9f46791..80d7782bc2 100644 --- a/apollo-router/src/plugins/connectors/testdata/variables.graphql +++ b/apollo-router/src/plugins/connectors/testdata/variables.graphql @@ -61,7 +61,7 @@ enum link__Purpose { type Query @join__type(graph: CONNECTORS) { - f(arg: String!): T @join__directive(graphs: [CONNECTORS], name: "connect", args: {source: "v1", http: {POST: "/f?arg={$args.arg->slice(1)}&context={$context.value}&config={$config.value}", headers: [{name: "x-connect-context", value: "{$context.value}"}, {name: "x-connect-config", value: "{$config.value}"}, {name: "x-connect-arg", value: "{$args.arg->last}"}], body: "arg: $args.arg\ncontext: $context.value\nconfig: $config.value"}, selection: "arg: $args.arg\ncontext: $context.value\nconfig: $config.value\nstatus: $status\nsibling: $(\"D\")\nextra: $->echo({ arg: $args.arg, context: $context.value, config: $config.value, status: $status })"}) + f(arg: String!): T @join__directive(graphs: [CONNECTORS], name: "connect", args: {source: "v1", http: {POST: "/f?arg={$args.arg->slice(1)}&context={$context.value}&config={$config.value}&header={$request.headers.value}", headers: [{name: "x-connect-context", value: "{$context.value}"}, {name: "x-connect-config", value: "{$config.value}"}, {name: "x-connect-arg", value: "{$args.arg->last}"}], body: "arg: $args.arg\ncontext: $context.value\nconfig: $config.value\nrequest: $request.headers.value"}, selection: "arg: $args.arg\ncontext: $context.value\nconfig: $config.value\nstatus: $status\nsibling: $(\"D\")\nextra: $->echo({ arg: $args.arg, context: $context.value, config: $config.value, status: $status })\nrequest: $request.headers.value\nresponse: $response.headers.value"}) } type T @@ -73,6 +73,8 @@ type T status: Int sibling: String extra: JSON + request: String + response: String f(arg: String): U @join__directive(graphs: [CONNECTORS], name: "connect", args: {source: "v1", http: {POST: "/f?arg={$args.arg->slice(2)}&context={$context.value}&config={$config.value}&sibling={$this.sibling}", headers: [{name: "x-connect-context", value: "{$context.value}"}, {name: "x-connect-config", value: "{$config.value}"}, {name: "x-connect-arg", value: "{$args.arg->first}"}, {name: "x-connect-sibling", value: "{$this.sibling}"}], body: "arg: $args.arg\ncontext: $context.value\nconfig: $config.value\nsibling: $this.sibling"}, selection: "arg: $args.arg\ncontext: $context.value\nconfig: $config.value\nsibling: $this.sibling\nstatus: $status"}) } diff --git a/apollo-router/src/plugins/connectors/tests/mod.rs b/apollo-router/src/plugins/connectors/tests/mod.rs index a6fa81ed8c..fa45373a9d 100644 --- a/apollo-router/src/plugins/connectors/tests/mod.rs +++ b/apollo-router/src/plugins/connectors/tests/mod.rs @@ -1461,7 +1461,11 @@ async fn test_variables() { .await; Mock::given(method("POST")) .and(path("/f")) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({}))) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({})) + .insert_header("value", "myothercoolheader"), + ) .mount(&mock_server) .await; let uri = mock_server.uri(); @@ -1469,7 +1473,7 @@ async fn test_variables() { let response = execute( &VARIABLES_SCHEMA.replace("http://localhost:4001/", &mock_server.uri()), &uri, - "{ f(arg: \"arg\") { arg context config sibling status extra f(arg: \"arg\") { arg context config sibling status } } }", + "{ f(arg: \"arg\") { arg context config sibling status extra request response f(arg: \"arg\") { arg context config sibling status } } }", Default::default(), Some(json!({ "preview_connectors": { @@ -1490,7 +1494,10 @@ async fn test_variables() { } } })), - |_| {}, + |request| { + let headers = request.router_request.headers_mut(); + headers.insert("value", "coolheader".parse().unwrap()); + }, ) .await; @@ -1509,6 +1516,8 @@ async fn test_variables() { "config": "C", "status": 200 }, + "request": "coolheader", + "response": "myothercoolheader", "f": { "arg": "arg", "context": "B", @@ -1528,13 +1537,13 @@ async fn test_variables() { Matcher::new() .method("POST") .path("/f") - .query("arg=rg&context=B&config=C") + .query("arg=rg&context=B&config=C&header=coolheader") .header("x-source-context".into(), "B".try_into().unwrap()) .header("x-source-config".into(), "C".try_into().unwrap()) .header("x-connect-arg".into(), "g".try_into().unwrap()) .header("x-connect-context".into(), "B".try_into().unwrap()) .header("x-connect-config".into(), "C".try_into().unwrap()) - .body(serde_json::json!({ "arg": "arg", "context": "B", "config": "C" })) + .body(serde_json::json!({ "arg": "arg", "context": "B", "config": "C", "request": "coolheader" })) , Matcher::new() .method("POST") diff --git a/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs b/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs index 8116a66b3c..22fd86603e 100644 --- a/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs +++ b/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs @@ -297,6 +297,8 @@ mod tests { use std::sync::Arc; use apollo_compiler::name; + use apollo_compiler::ExecutableDocument; + use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -313,11 +315,13 @@ mod tests { use super::ConnectorSelector; use super::ConnectorSource; use super::MappingProblems; + use crate::graphql; use crate::plugins::connectors::handle_responses::MappedResponse; use crate::plugins::connectors::make_requests::ResponseKey; use crate::plugins::connectors::mapping::Problem; use crate::plugins::telemetry::config_new::selectors::ResponseStatus; use crate::plugins::telemetry::config_new::Selector; + use crate::query_planner::fetch::Variables; use crate::services::connector::request_service::transport; use crate::services::connector::request_service::Request; use crate::services::connector::request_service::Response; @@ -394,6 +398,34 @@ mod tests { http_request: http::Request, mapping_problems: Vec, ) -> Request { + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let original_request = Arc::new(original_request); + Request { context: context(), connector: Arc::new(connector()), @@ -404,6 +436,7 @@ mod tests { }), key: response_key(), mapping_problems, + original_request, } } diff --git a/apollo-router/src/plugins/telemetry/config_new/events.rs b/apollo-router/src/plugins/telemetry/config_new/events.rs index 7ea219bf2a..601f9f03d4 100644 --- a/apollo-router/src/plugins/telemetry/config_new/events.rs +++ b/apollo-router/src/plugins/telemetry/config_new/events.rs @@ -841,6 +841,8 @@ mod tests { use std::str::FromStr; use apollo_compiler::name; + use apollo_compiler::ExecutableDocument; + use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -862,6 +864,7 @@ mod tests { use crate::plugins::connectors::make_requests::ResponseKey; use crate::plugins::telemetry::Telemetry; use crate::plugins::test::PluginTestHarness; + use crate::query_planner::fetch::Variables; use crate::services::connector::request_service::transport; use crate::services::connector::request_service::Request; use crate::services::connector::request_service::Response; @@ -1238,6 +1241,36 @@ mod tests { inputs: Default::default(), selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; + + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./") + .unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let original_request = Arc::new(original_request); + let connector_request = Request { context: context.clone(), connector: Arc::new(connector.clone()), @@ -1245,6 +1278,7 @@ mod tests { transport_request, key: response_key.clone(), mapping_problems: vec![], + original_request, }; test_harness .call_connector_request_service(connector_request, |request| Response { @@ -1320,6 +1354,36 @@ mod tests { inputs: Default::default(), selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; + + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./") + .unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let original_request = Arc::new(original_request); + let connector_request = Request { context: context.clone(), connector: Arc::new(connector.clone()), @@ -1327,6 +1391,7 @@ mod tests { transport_request, key: response_key.clone(), mapping_problems: vec![], + original_request, }; test_harness .call_connector_request_service(connector_request, |request| Response { diff --git a/apollo-router/src/plugins/telemetry/config_new/instruments.rs b/apollo-router/src/plugins/telemetry/config_new/instruments.rs index c47d11ac87..1a97a6c729 100644 --- a/apollo-router/src/plugins/telemetry/config_new/instruments.rs +++ b/apollo-router/src/plugins/telemetry/config_new/instruments.rs @@ -2623,7 +2623,9 @@ mod tests { use apollo_compiler::ast::NamedType; use apollo_compiler::executable::SelectionSet; use apollo_compiler::name; + use apollo_compiler::ExecutableDocument; use apollo_compiler::Name; + use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -2665,6 +2667,7 @@ mod tests { use crate::plugins::telemetry::APOLLO_PRIVATE_QUERY_DEPTH; use crate::plugins::telemetry::APOLLO_PRIVATE_QUERY_HEIGHT; use crate::plugins::telemetry::APOLLO_PRIVATE_QUERY_ROOT_FIELDS; + use crate::query_planner::fetch::Variables; use crate::services::connector::request_service::transport; use crate::services::connector::request_service::Request; use crate::services::connector::request_service::Response; @@ -3284,6 +3287,38 @@ mod tests { JSONSelection::parse("$.data").unwrap(), ), }; + let schema = Arc::new( + Schema::parse_and_validate( + "type Query { a: A } type A { f: String }", + "./", + ) + .unwrap(), + ); + + let original_request = + crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let original_request = Arc::new(original_request); let request = Request { context: Context::default(), connector: Arc::new(connector), @@ -3291,6 +3326,7 @@ mod tests { transport_request, key: response_key.clone(), mapping_problems, + original_request, }; connector_instruments = Some({ let connector_instruments = config diff --git a/apollo-router/src/plugins/telemetry/fmt_layer.rs b/apollo-router/src/plugins/telemetry/fmt_layer.rs index 1a091d2e4e..602730c9a6 100644 --- a/apollo-router/src/plugins/telemetry/fmt_layer.rs +++ b/apollo-router/src/plugins/telemetry/fmt_layer.rs @@ -266,6 +266,8 @@ mod tests { use std::sync::MutexGuard; use apollo_compiler::name; + use apollo_compiler::ExecutableDocument; + use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -296,6 +298,7 @@ mod tests { use crate::plugins::telemetry::config_new::logging::TextFormat; use crate::plugins::telemetry::dynamic_attribute::SpanDynAttribute; use crate::plugins::telemetry::otel; + use crate::query_planner::fetch::Variables; use crate::services::connector::request_service::transport; use crate::services::connector::request_service::Request; use crate::services::connector::request_service::Response; @@ -884,6 +887,34 @@ connector: inputs: Default::default(), selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./") + .unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(crate::context::Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let original_request = Arc::new(original_request); let connector_request = Request { context: context.clone(), connector: connector.clone(), @@ -907,6 +938,7 @@ connector: path: "@.id".to_string(), }, ], + original_request, }; let connector_events = event_config.new_connector_events(); connector_events.on_request(&connector_request); @@ -1132,6 +1164,34 @@ connector: inputs: Default::default(), selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; + let schema = Arc::new( + Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./") + .unwrap(), + ); + + let original_request = crate::services::connect::Request::builder() + .service_name("subgraph_Query_a_0".into()) + .context(crate::context::Context::default()) + .operation(Arc::new( + ExecutableDocument::parse_and_validate( + &schema, + "query { a { f } a2: a { f2: f } }".to_string(), + "./", + ) + .unwrap(), + )) + .variables(Variables { + variables: Default::default(), + inverted_paths: Default::default(), + contextual_arguments: Default::default(), + }) + .supergraph_request(Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), + )) + .build(); + let original_request = Arc::new(original_request); let connector_request = Request { context: context.clone(), connector: connector.clone(), @@ -1155,6 +1215,7 @@ connector: path: "@.id".to_string(), }, ], + original_request, }; let connector_events = event_config.new_connector_events(); connector_events.on_request(&connector_request); diff --git a/apollo-router/src/services/connect.rs b/apollo-router/src/services/connect.rs index 40b1f990d1..045c34abae 100644 --- a/apollo-router/src/services/connect.rs +++ b/apollo-router/src/services/connect.rs @@ -5,6 +5,7 @@ use std::sync::Arc; use apollo_compiler::validation::Valid; use apollo_compiler::ExecutableDocument; use static_assertions::assert_impl_all; +use std::fmt::Debug; use tower::BoxError; use crate::graphql; @@ -23,6 +24,18 @@ pub(crate) struct Request { pub(crate) variables: Variables, } +impl Debug for Request { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Request") + .field("service_name", &self.service_name) + .field("context", &self.context) + .field("operation", &self.operation) + .field("supergraph_request", &self.supergraph_request) + .field("variables", &self.variables.variables) + .finish() + } +} + assert_impl_all!(Response: Send); #[derive(Debug)] #[non_exhaustive] diff --git a/apollo-router/src/services/connector/request_service.rs b/apollo-router/src/services/connector/request_service.rs index 65643b91f0..c586ac60a6 100644 --- a/apollo-router/src/services/connector/request_service.rs +++ b/apollo-router/src/services/connector/request_service.rs @@ -36,6 +36,7 @@ use crate::plugins::telemetry::config_new::connector::events::ConnectorEventRequ use crate::plugins::telemetry::config_new::events::log_event; use crate::plugins::telemetry::config_new::events::EventLevel; use crate::plugins::telemetry::consts::CONNECT_REQUEST_SPAN_NAME; +use crate::services::connect; use crate::services::connector::request_service::transport::http::HttpRequest; use crate::services::connector::request_service::transport::http::HttpResponse; use crate::services::http::HttpClientServiceFactory; @@ -76,6 +77,8 @@ pub(crate) struct Request { /// Mapping problems encountered when creating the transport request pub(crate) mapping_problems: Vec, + + pub(crate) original_request: Arc, } /// Response type for a connector @@ -318,6 +321,7 @@ impl tower::Service for ConnectorRequestService { &request.context, debug_request, &debug, + request.original_request, ) .await) }) From 160efd9a1e318280fc571ea2c27138e3c8c204e0 Mon Sep 17 00:00:00 2001 From: Andrew McGivery Date: Thu, 23 Jan 2025 16:32:35 -0500 Subject: [PATCH 2/8] Update snapshots --- ...ders__invalid_namespace_in_header_variables.graphql.snap | 6 +++--- ...n_tests@invalid_namespace_in_body_selection.graphql.snap | 4 ++-- ...invalid_namespace_in_url_template_variables.graphql.snap | 4 ++-- ...ion_tests@uri_templates__this_on_root_types.graphql.snap | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@headers__invalid_namespace_in_header_variables.graphql.snap b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@headers__invalid_namespace_in_header_variables.graphql.snap index 4168982f21..1e885dffa2 100644 --- a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@headers__invalid_namespace_in_header_variables.graphql.snap +++ b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@headers__invalid_namespace_in_header_variables.graphql.snap @@ -1,6 +1,6 @@ --- source: apollo-federation/src/sources/connect/validation/mod.rs -expression: "format!(\"{:#?}\", errors)" +expression: "format!(\"{:#?}\", result.errors)" input_file: apollo-federation/src/sources/connect/validation/test_data/headers/invalid_namespace_in_header_variables.graphql --- [ @@ -34,14 +34,14 @@ input_file: apollo-federation/src/sources/connect/validation/test_data/headers/i }, Message { code: InvalidHeader, - message: "In `@connect(http.headers:)` on `Query.scalar`: $status is not valid here, must be one of $args, $config, $context", + message: "In `@connect(http.headers:)` on `Query.scalar`: $status is not valid here, must be one of $args, $config, $context, $request", locations: [ 25:62..25:69, ], }, Message { code: InvalidHeader, - message: "In `@connect(http.headers:)` on `Query.scalar`: $this is not valid here, must be one of $args, $config, $context", + message: "In `@connect(http.headers:)` on `Query.scalar`: $this is not valid here, must be one of $args, $config, $context, $request", locations: [ 26:47..26:52, ], diff --git a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@invalid_namespace_in_body_selection.graphql.snap b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@invalid_namespace_in_body_selection.graphql.snap index cf30789ed7..4d05f7e196 100644 --- a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@invalid_namespace_in_body_selection.graphql.snap +++ b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@invalid_namespace_in_body_selection.graphql.snap @@ -1,12 +1,12 @@ --- source: apollo-federation/src/sources/connect/validation/mod.rs -expression: "format!(\"{:#?}\", errors)" +expression: "format!(\"{:#?}\", result.errors)" input_file: apollo-federation/src/sources/connect/validation/test_data/invalid_namespace_in_body_selection.graphql --- [ Message { code: InvalidJsonSelection, - message: "In `@connect(http: {body:})` on `Mutation.createUser`: variable `$status` is not valid at this location, must be one of $args, $config, $context, $this", + message: "In `@connect(http: {body:})` on `Mutation.createUser`: variable `$status` is not valid at this location, must be one of $args, $config, $context, $request, $this", locations: [ 21:17..21:24, ], diff --git a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid_namespace_in_url_template_variables.graphql.snap b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid_namespace_in_url_template_variables.graphql.snap index aecca0fd8a..440affc31c 100644 --- a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid_namespace_in_url_template_variables.graphql.snap +++ b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid_namespace_in_url_template_variables.graphql.snap @@ -1,6 +1,6 @@ --- source: apollo-federation/src/sources/connect/validation/mod.rs -expression: "format!(\"{:#?}\", errors)" +expression: "format!(\"{:#?}\", result.errors)" input_file: apollo-federation/src/sources/connect/validation/test_data/uri_templates/invalid_namespace_in_url_template_variables.graphql --- [ @@ -13,7 +13,7 @@ input_file: apollo-federation/src/sources/connect/validation/test_data/uri_templ }, Message { code: InvalidUrl, - message: "In `GET` in `@connect(http:)` on `Query.invalid`: $status is not valid here, must be one of $args, $config, $context", + message: "In `GET` in `@connect(http:)` on `Query.invalid`: $status is not valid here, must be one of $args, $config, $context, $request", locations: [ 18:31..18:42, ], diff --git a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__this_on_root_types.graphql.snap b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__this_on_root_types.graphql.snap index 27bfc422dd..1775e27221 100644 --- a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__this_on_root_types.graphql.snap +++ b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__this_on_root_types.graphql.snap @@ -1,19 +1,19 @@ --- source: apollo-federation/src/sources/connect/validation/mod.rs -expression: "format!(\"{:#?}\", errors)" +expression: "format!(\"{:#?}\", result.errors)" input_file: apollo-federation/src/sources/connect/validation/test_data/uri_templates/this_on_root_types.graphql --- [ Message { code: InvalidUrl, - message: "In `GET` in `@connect(http:)` on `Query.requiresThis`: $this is not valid here, must be one of $args, $config, $context", + message: "In `GET` in `@connect(http:)` on `Query.requiresThis`: $this is not valid here, must be one of $args, $config, $context, $request", locations: [ 11:39..11:51, ], }, Message { code: InvalidUrl, - message: "In `GET` in `@connect(http:)` on `Mutation.requiresThis`: $this is not valid here, must be one of $args, $config, $context", + message: "In `GET` in `@connect(http:)` on `Mutation.requiresThis`: $this is not valid here, must be one of $args, $config, $context, $request", locations: [ 20:37..20:49, ], From b615d698f2a1eb9cd9d194ac22d454dbebddc31d Mon Sep 17 00:00:00 2001 From: Andrew McGivery Date: Thu, 23 Jan 2025 16:43:49 -0500 Subject: [PATCH 3/8] Fix lint issue --- apollo-router/src/services/connect.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/src/services/connect.rs b/apollo-router/src/services/connect.rs index 045c34abae..4053d5122a 100644 --- a/apollo-router/src/services/connect.rs +++ b/apollo-router/src/services/connect.rs @@ -1,11 +1,11 @@ //! Connect service request and response types. +use std::fmt::Debug; use std::sync::Arc; use apollo_compiler::validation::Valid; use apollo_compiler::ExecutableDocument; use static_assertions::assert_impl_all; -use std::fmt::Debug; use tower::BoxError; use crate::graphql; From 67fc106a7f9ed1bc4be2e63bc2e11b118331d803 Mon Sep 17 00:00:00 2001 From: Andrew McGivery Date: Thu, 23 Jan 2025 17:49:03 -0500 Subject: [PATCH 4/8] Swap to passing the supergraph_request instead of the connectors request --- .../plugins/connectors/handle_responses.rs | 190 ++++-------------- .../src/plugins/connectors/make_requests.rs | 10 +- .../config_new/connector/selectors.rs | 35 +--- .../plugins/telemetry/config_new/events.rs | 69 +------ .../telemetry/config_new/instruments.rs | 40 +--- .../src/plugins/telemetry/fmt_layer.rs | 65 +----- .../src/services/connector/request_service.rs | 6 +- 7 files changed, 74 insertions(+), 341 deletions(-) diff --git a/apollo-router/src/plugins/connectors/handle_responses.rs b/apollo-router/src/plugins/connectors/handle_responses.rs index b0f2c56e17..a3a56148dd 100644 --- a/apollo-router/src/plugins/connectors/handle_responses.rs +++ b/apollo-router/src/plugins/connectors/handle_responses.rs @@ -25,7 +25,6 @@ use crate::plugins::telemetry::config_new::events::log_event; use crate::plugins::telemetry::consts::OTEL_STATUS_CODE; use crate::plugins::telemetry::consts::OTEL_STATUS_CODE_ERROR; use crate::plugins::telemetry::consts::OTEL_STATUS_CODE_OK; -use crate::services::connect; use crate::services::connect::Response; use crate::services::connector; use crate::services::connector::request_service::transport::http::HttpResponse; @@ -77,7 +76,7 @@ impl RawResponse { connector: Arc, context: &Context, debug_context: &Option>>, - original_request: Arc, + supergraph_request: Arc>, ) -> connector::request_service::Response { let mapped_response = match self { RawResponse::Error { error, key } => MappedResponse::Error { error, key }, @@ -92,7 +91,7 @@ impl RawResponse { connector.config.as_ref(), context, Some(parts.status.as_u16()), - original_request, + supergraph_request, Some(&parts), ); @@ -301,7 +300,7 @@ pub(crate) async fn process_response( context: &Context, debug_request: Option, debug_context: &Option>>, - original_request: Arc, + supergraph_request: Arc>, ) -> connector::request_service::Response { match result { // This occurs when we short-circuit the request when over the limit @@ -351,7 +350,13 @@ pub(crate) async fn process_response( }; if is_success { Span::current().record(OTEL_STATUS_CODE, OTEL_STATUS_CODE_OK); - raw.map_response(result, connector, context, debug_context, original_request) + raw.map_response( + result, + connector, + context, + debug_context, + supergraph_request, + ) } else { Span::current().record(OTEL_STATUS_CODE, OTEL_STATUS_CODE_ERROR); raw.map_error(result, connector, context, debug_context) @@ -544,8 +549,6 @@ mod tests { use std::sync::Arc; use apollo_compiler::name; - use apollo_compiler::ExecutableDocument; - use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -559,7 +562,6 @@ mod tests { use crate::graphql; use crate::plugins::connectors::handle_responses::process_response; use crate::plugins::connectors::make_requests::ResponseKey; - use crate::query_planner::fetch::Variables; use crate::services::router; use crate::services::router::body::RouterBody; use crate::Context; @@ -609,33 +611,11 @@ mod tests { selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), - ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - let original_request = Arc::new(original_request); + ); let res = super::aggregate_responses(vec![ process_response( @@ -645,7 +625,7 @@ mod tests { &Context::default(), None, &None, - original_request.clone(), + supergraph_request.clone(), ) .await .mapped_response, @@ -656,7 +636,7 @@ mod tests { &Context::default(), None, &None, - original_request, + supergraph_request, ) .await .mapped_response, @@ -739,33 +719,11 @@ mod tests { selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), - ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - let original_request = Arc::new(original_request); + ); let res = super::aggregate_responses(vec![ process_response( @@ -775,7 +733,7 @@ mod tests { &Context::default(), None, &None, - original_request.clone(), + supergraph_request.clone(), ) .await .mapped_response, @@ -786,7 +744,7 @@ mod tests { &Context::default(), None, &None, - original_request, + supergraph_request, ) .await .mapped_response, @@ -879,34 +837,11 @@ mod tests { selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), - ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - - let original_request = Arc::new(original_request); + ); let res = super::aggregate_responses(vec![ process_response( @@ -916,7 +851,7 @@ mod tests { &Context::default(), None, &None, - original_request.clone(), + supergraph_request.clone(), ) .await .mapped_response, @@ -927,7 +862,7 @@ mod tests { &Context::default(), None, &None, - original_request, + supergraph_request, ) .await .mapped_response, @@ -1042,33 +977,11 @@ mod tests { selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), - ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - let original_request = Arc::new(original_request); + ); let res = super::aggregate_responses(vec![ process_response( @@ -1078,7 +991,7 @@ mod tests { &Context::default(), None, &None, - original_request.clone(), + supergraph_request.clone(), ) .await .mapped_response, @@ -1089,7 +1002,7 @@ mod tests { &Context::default(), None, &None, - original_request.clone(), + supergraph_request.clone(), ) .await .mapped_response, @@ -1100,7 +1013,7 @@ mod tests { &Context::default(), None, &None, - original_request.clone(), + supergraph_request.clone(), ) .await .mapped_response, @@ -1111,7 +1024,7 @@ mod tests { &Context::default(), None, &None, - original_request, + supergraph_request, ) .await .mapped_response, @@ -1300,32 +1213,11 @@ mod tests { selection: Arc::new(JSONSelection::parse("$status").unwrap()), }; - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), - ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); + ); let res = super::aggregate_responses(vec![ process_response( @@ -1335,7 +1227,7 @@ mod tests { &Context::default(), None, &None, - Arc::new(original_request), + supergraph_request, ) .await .mapped_response, diff --git a/apollo-router/src/plugins/connectors/make_requests.rs b/apollo-router/src/plugins/connectors/make_requests.rs index 5826f71b60..65831d920f 100644 --- a/apollo-router/src/plugins/connectors/make_requests.rs +++ b/apollo-router/src/plugins/connectors/make_requests.rs @@ -45,7 +45,7 @@ impl RequestInputs { config: Option<&CustomConfiguration>, context: &Context, status: Option, - original_request: Arc, + supergraph_request: Arc>, response_parts: Option<&Parts>, ) -> IndexMap { let mut map = IndexMap::with_capacity_and_hasher(variables_used.len(), Default::default()); @@ -97,8 +97,7 @@ impl RequestInputs { // Add headers from the original router request if variables_used.contains(&Namespace::Request) { - let headers: Map = original_request - .supergraph_request + let headers: Map = supergraph_request .headers() .iter() .map(|(key, value)| { @@ -243,7 +242,6 @@ fn request_params_to_requests( debug: &Option>>, ) -> Result, MakeRequestError> { let mut results = vec![]; - let original_request = Arc::new(original_request); for response_key in request_params { let connector = connector.clone(); let (transport_request, mapping_problems) = make_request( @@ -253,7 +251,7 @@ fn request_params_to_requests( connector.config.as_ref(), &original_request.context, None, - original_request.clone(), + original_request.supergraph_request.clone(), None, ), &original_request, @@ -267,7 +265,7 @@ fn request_params_to_requests( transport_request, key: response_key, mapping_problems, - original_request: original_request.clone(), + supergraph_request: original_request.supergraph_request.clone(), }); } diff --git a/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs b/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs index 22fd86603e..55ef0a5a58 100644 --- a/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs +++ b/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs @@ -297,8 +297,6 @@ mod tests { use std::sync::Arc; use apollo_compiler::name; - use apollo_compiler::ExecutableDocument; - use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -321,7 +319,6 @@ mod tests { use crate::plugins::connectors::mapping::Problem; use crate::plugins::telemetry::config_new::selectors::ResponseStatus; use crate::plugins::telemetry::config_new::Selector; - use crate::query_planner::fetch::Variables; use crate::services::connector::request_service::transport; use crate::services::connector::request_service::Request; use crate::services::connector::request_service::Response; @@ -398,33 +395,11 @@ mod tests { http_request: http::Request, mapping_problems: Vec, ) -> Request { - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./").unwrap(), - ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - let original_request = Arc::new(original_request); + ); Request { context: context(), @@ -436,7 +411,7 @@ mod tests { }), key: response_key(), mapping_problems, - original_request, + supergraph_request, } } diff --git a/apollo-router/src/plugins/telemetry/config_new/events.rs b/apollo-router/src/plugins/telemetry/config_new/events.rs index 601f9f03d4..a75116ea3b 100644 --- a/apollo-router/src/plugins/telemetry/config_new/events.rs +++ b/apollo-router/src/plugins/telemetry/config_new/events.rs @@ -841,8 +841,6 @@ mod tests { use std::str::FromStr; use apollo_compiler::name; - use apollo_compiler::ExecutableDocument; - use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -864,7 +862,6 @@ mod tests { use crate::plugins::connectors::make_requests::ResponseKey; use crate::plugins::telemetry::Telemetry; use crate::plugins::test::PluginTestHarness; - use crate::query_planner::fetch::Variables; use crate::services::connector::request_service::transport; use crate::services::connector::request_service::Request; use crate::services::connector::request_service::Response; @@ -1241,36 +1238,11 @@ mod tests { inputs: Default::default(), selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; - - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./") + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) - .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - let original_request = Arc::new(original_request); - let connector_request = Request { context: context.clone(), connector: Arc::new(connector.clone()), @@ -1278,7 +1250,7 @@ mod tests { transport_request, key: response_key.clone(), mapping_problems: vec![], - original_request, + supergraph_request, }; test_harness .call_connector_request_service(connector_request, |request| Response { @@ -1354,36 +1326,11 @@ mod tests { inputs: Default::default(), selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; - - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./") + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) - .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - let original_request = Arc::new(original_request); - let connector_request = Request { context: context.clone(), connector: Arc::new(connector.clone()), @@ -1391,7 +1338,7 @@ mod tests { transport_request, key: response_key.clone(), mapping_problems: vec![], - original_request, + supergraph_request, }; test_harness .call_connector_request_service(connector_request, |request| Response { diff --git a/apollo-router/src/plugins/telemetry/config_new/instruments.rs b/apollo-router/src/plugins/telemetry/config_new/instruments.rs index 1a97a6c729..7a7b7706d8 100644 --- a/apollo-router/src/plugins/telemetry/config_new/instruments.rs +++ b/apollo-router/src/plugins/telemetry/config_new/instruments.rs @@ -2623,9 +2623,7 @@ mod tests { use apollo_compiler::ast::NamedType; use apollo_compiler::executable::SelectionSet; use apollo_compiler::name; - use apollo_compiler::ExecutableDocument; use apollo_compiler::Name; - use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -2667,7 +2665,6 @@ mod tests { use crate::plugins::telemetry::APOLLO_PRIVATE_QUERY_DEPTH; use crate::plugins::telemetry::APOLLO_PRIVATE_QUERY_HEIGHT; use crate::plugins::telemetry::APOLLO_PRIVATE_QUERY_ROOT_FIELDS; - use crate::query_planner::fetch::Variables; use crate::services::connector::request_service::transport; use crate::services::connector::request_service::Request; use crate::services::connector::request_service::Response; @@ -3287,38 +3284,11 @@ mod tests { JSONSelection::parse("$.data").unwrap(), ), }; - let schema = Arc::new( - Schema::parse_and_validate( - "type Query { a: A } type A { f: String }", - "./", - ) - .unwrap(), + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) + .unwrap(), ); - - let original_request = - crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) - .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - let original_request = Arc::new(original_request); let request = Request { context: Context::default(), connector: Arc::new(connector), @@ -3326,7 +3296,7 @@ mod tests { transport_request, key: response_key.clone(), mapping_problems, - original_request, + supergraph_request, }; connector_instruments = Some({ let connector_instruments = config diff --git a/apollo-router/src/plugins/telemetry/fmt_layer.rs b/apollo-router/src/plugins/telemetry/fmt_layer.rs index 602730c9a6..a81a183faa 100644 --- a/apollo-router/src/plugins/telemetry/fmt_layer.rs +++ b/apollo-router/src/plugins/telemetry/fmt_layer.rs @@ -266,8 +266,6 @@ mod tests { use std::sync::MutexGuard; use apollo_compiler::name; - use apollo_compiler::ExecutableDocument; - use apollo_compiler::Schema; use apollo_federation::sources::connect::ConnectId; use apollo_federation::sources::connect::ConnectSpec; use apollo_federation::sources::connect::Connector; @@ -298,7 +296,6 @@ mod tests { use crate::plugins::telemetry::config_new::logging::TextFormat; use crate::plugins::telemetry::dynamic_attribute::SpanDynAttribute; use crate::plugins::telemetry::otel; - use crate::query_planner::fetch::Variables; use crate::services::connector::request_service::transport; use crate::services::connector::request_service::Request; use crate::services::connector::request_service::Response; @@ -887,34 +884,11 @@ connector: inputs: Default::default(), selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./") + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(crate::context::Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) - .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - let original_request = Arc::new(original_request); let connector_request = Request { context: context.clone(), connector: connector.clone(), @@ -938,7 +912,7 @@ connector: path: "@.id".to_string(), }, ], - original_request, + supergraph_request, }; let connector_events = event_config.new_connector_events(); connector_events.on_request(&connector_request); @@ -1164,34 +1138,11 @@ connector: inputs: Default::default(), selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; - let schema = Arc::new( - Schema::parse_and_validate("type Query { a: A } type A { f: String }", "./") + let supergraph_request = Arc::new( + http::Request::builder() + .body(graphql::Request::builder().build()) .unwrap(), ); - - let original_request = crate::services::connect::Request::builder() - .service_name("subgraph_Query_a_0".into()) - .context(crate::context::Context::default()) - .operation(Arc::new( - ExecutableDocument::parse_and_validate( - &schema, - "query { a { f } a2: a { f2: f } }".to_string(), - "./", - ) - .unwrap(), - )) - .variables(Variables { - variables: Default::default(), - inverted_paths: Default::default(), - contextual_arguments: Default::default(), - }) - .supergraph_request(Arc::new( - http::Request::builder() - .body(graphql::Request::builder().build()) - .unwrap(), - )) - .build(); - let original_request = Arc::new(original_request); let connector_request = Request { context: context.clone(), connector: connector.clone(), @@ -1215,7 +1166,7 @@ connector: path: "@.id".to_string(), }, ], - original_request, + supergraph_request, }; let connector_events = event_config.new_connector_events(); connector_events.on_request(&connector_request); diff --git a/apollo-router/src/services/connector/request_service.rs b/apollo-router/src/services/connector/request_service.rs index c586ac60a6..7beec5e57f 100644 --- a/apollo-router/src/services/connector/request_service.rs +++ b/apollo-router/src/services/connector/request_service.rs @@ -36,7 +36,6 @@ use crate::plugins::telemetry::config_new::connector::events::ConnectorEventRequ use crate::plugins::telemetry::config_new::events::log_event; use crate::plugins::telemetry::config_new::events::EventLevel; use crate::plugins::telemetry::consts::CONNECT_REQUEST_SPAN_NAME; -use crate::services::connect; use crate::services::connector::request_service::transport::http::HttpRequest; use crate::services::connector::request_service::transport::http::HttpResponse; use crate::services::http::HttpClientServiceFactory; @@ -78,7 +77,8 @@ pub(crate) struct Request { /// Mapping problems encountered when creating the transport request pub(crate) mapping_problems: Vec, - pub(crate) original_request: Arc, + /// The original supergraph request from the supergraph service + pub(crate) supergraph_request: Arc>, } /// Response type for a connector @@ -321,7 +321,7 @@ impl tower::Service for ConnectorRequestService { &request.context, debug_request, &debug, - request.original_request, + request.supergraph_request, ) .await) }) From 4628af27f88ee2437b220c0379600e3f20fe3026 Mon Sep 17 00:00:00 2001 From: Andrew McGivery Date: Fri, 24 Jan 2025 11:50:22 -0500 Subject: [PATCH 5/8] Only add headers in memory that are actually referenced by selections --- .../sources/connect/json_selection/parser.rs | 26 +++++++++ .../src/sources/connect/models.rs | 32 +++++++++++ .../plugins/connectors/handle_responses.rs | 11 ++++ .../src/plugins/connectors/make_requests.rs | 55 +++++++++++++++---- .../src/plugins/connectors/tracing.rs | 2 + .../config_new/connector/selectors.rs | 2 + .../plugins/telemetry/config_new/events.rs | 4 ++ .../telemetry/config_new/instruments.rs | 4 ++ .../src/plugins/telemetry/fmt_layer.rs | 4 ++ 9 files changed, 129 insertions(+), 11 deletions(-) diff --git a/apollo-federation/src/sources/connect/json_selection/parser.rs b/apollo-federation/src/sources/connect/json_selection/parser.rs index 2007feb728..be463d2106 100644 --- a/apollo-federation/src/sources/connect/json_selection/parser.rs +++ b/apollo-federation/src/sources/connect/json_selection/parser.rs @@ -237,6 +237,32 @@ impl JSONSelection { .flat_map(|var_path| var_path.variable_reference()) .map(|var_ref| var_ref.namespace.namespace) } + + /// Get any headers referenced in the variable references by looking at both Request and Response namespaces. + pub fn header_references(&self) -> impl Iterator + '_ { + self.external_var_paths() + .into_iter() + .flat_map(|var_path| var_path.variable_reference()) + .filter_map(|var_ref| { + if var_ref.namespace.namespace != Namespace::Request + && var_ref.namespace.namespace != Namespace::Response + { + return None; + } + + // We only care if the path references starts with "headers" + if !var_ref + .path + .first() + .map_or(false, |path| path.part == "headers") + { + return None; + } + + // Grab the name of the header from the path + var_ref.path.get(1).map(|path| path.part.to_string()) + }) + } } impl ExternalVarPaths for JSONSelection { diff --git a/apollo-federation/src/sources/connect/models.rs b/apollo-federation/src/sources/connect/models.rs index a6161ecd6e..d16dcd7992 100644 --- a/apollo-federation/src/sources/connect/models.rs +++ b/apollo-federation/src/sources/connect/models.rs @@ -64,6 +64,11 @@ pub struct Connector { pub request_variables: HashSet, pub response_variables: HashSet, + + /// The request headers referenced in the connectors request mapping + pub request_headers: HashSet, + /// The request or response headers referenced in the connectors response mapping + pub response_headers: HashSet, } pub type CustomConfiguration = Arc>; @@ -159,7 +164,9 @@ impl Connector { }; let request_variables = transport.variables().collect(); + let request_headers = transport.header_references().collect(); let response_variables = connect.selection.external_variables().collect(); + let response_headers = connect.selection.header_references().collect(); let connector = Connector { id: id.clone(), @@ -171,6 +178,8 @@ impl Connector { spec, request_variables, response_variables, + request_headers, + response_headers, }; Ok((id, connector)) @@ -320,6 +329,29 @@ impl HttpJsonTransport { .flat_map(PathSelection::variable_reference) }) } + + /// Get any headers referenced in the variable references by looking at both Request and Response namespaces. + fn header_references(&self) -> impl Iterator + '_ { + self.variable_references().filter_map(|var_ref| { + if var_ref.namespace.namespace != Namespace::Request + && var_ref.namespace.namespace != Namespace::Response + { + return None; + } + + // We only care if the path references starts with "headers" + if !var_ref + .path + .first() + .map_or(false, |path| path.part == "headers") + { + return None; + } + + // Grab the name of the header from the path + var_ref.path.get(1).map(|path| path.part.to_string()) + }) + } } /// The HTTP arguments needed for a connect request diff --git a/apollo-router/src/plugins/connectors/handle_responses.rs b/apollo-router/src/plugins/connectors/handle_responses.rs index a3a56148dd..c68b0b81f1 100644 --- a/apollo-router/src/plugins/connectors/handle_responses.rs +++ b/apollo-router/src/plugins/connectors/handle_responses.rs @@ -88,6 +88,7 @@ impl RawResponse { } => { let inputs = key.inputs().merge( &connector.response_variables, + &connector.response_headers, connector.config.as_ref(), context, Some(parts.status.as_u16()), @@ -591,6 +592,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }); let response1: http::Response = http::Response::builder() @@ -699,6 +702,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }); let response1: http::Response = http::Response::builder() @@ -813,6 +818,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }); let response1: http::Response = http::Response::builder() @@ -937,6 +944,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }); let response_plaintext: http::Response = http::Response::builder() @@ -1201,6 +1210,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: selection.external_variables().collect(), + request_headers: Default::default(), + response_headers: Default::default(), }); let response1: http::Response = http::Response::builder() diff --git a/apollo-router/src/plugins/connectors/make_requests.rs b/apollo-router/src/plugins/connectors/make_requests.rs index 65831d920f..a435fa63a4 100644 --- a/apollo-router/src/plugins/connectors/make_requests.rs +++ b/apollo-router/src/plugins/connectors/make_requests.rs @@ -39,9 +39,11 @@ impl RequestInputs { /// Creates a map for use in JSONSelection::apply_with_vars. It only clones /// values into the map if the variable namespaces (`$args`, `$this`, etc.) /// are actually referenced in the expressions for URLs, headers, body, or selection. + #[allow(clippy::too_many_arguments)] pub(crate) fn merge( &self, variables_used: &HashSet, + headers_used: &HashSet, config: Option<&CustomConfiguration>, context: &Context, status: Option, @@ -95,16 +97,21 @@ impl RequestInputs { } } - // Add headers from the original router request + // Add headers from the original router request. + // Only include headers that are actually referenced to save on passing around unused headers in memory. if variables_used.contains(&Namespace::Request) { let headers: Map = supergraph_request .headers() .iter() - .map(|(key, value)| { - ( - key.as_str().into(), - value.to_str().unwrap_or_default().into(), - ) + .filter_map(|(key, value)| { + if headers_used.contains::(&key.as_str().into()) { + return Some(( + key.as_str().into(), + value.to_str().unwrap_or_default().into(), + )); + } + + None }) .collect(); let request_object = json!({ @@ -114,16 +121,21 @@ impl RequestInputs { } // Add headers from the connectors response + // Only include headers that are actually referenced to save on passing around unused headers in memory. if variables_used.contains(&Namespace::Response) { if let Some(response_parts) = response_parts { let headers: Map = response_parts .headers .iter() - .map(|(key, value)| { - ( - key.as_str().into(), - value.to_str().unwrap_or_default().into(), - ) + .filter_map(|(key, value)| { + if headers_used.contains::(&key.as_str().into()) { + return Some(( + key.as_str().into(), + value.to_str().unwrap_or_default().into(), + )); + } + + None }) .collect(); let response_object = json!({ @@ -248,6 +260,7 @@ fn request_params_to_requests( &connector.transport, response_key.inputs().merge( &connector.request_variables, + &connector.request_headers, connector.config.as_ref(), &original_request.context, None, @@ -680,6 +693,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; assert_debug_snapshot!(super::root_fields(Arc::new(connector), &req), @r###" @@ -811,6 +826,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; assert_debug_snapshot!(super::root_fields(Arc::new(connector), &req), @r###" @@ -970,6 +987,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; assert_debug_snapshot!(super::root_fields(Arc::new(connector), &req), @r###" @@ -1199,6 +1218,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; assert_debug_snapshot!(super::entities_from_request(Arc::new(connector), &req).unwrap(), @r###" @@ -1517,6 +1538,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; assert_debug_snapshot!(super::entities_from_request(Arc::new(connector), &req).unwrap(), @r###" @@ -1816,6 +1839,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; assert_debug_snapshot!(super::entities_from_request(Arc::new(connector), &req).unwrap(), @r###" @@ -2035,6 +2060,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; assert_debug_snapshot!(super::entities_with_fields_from_request(Arc::new(connector), &req).unwrap(), @r###" @@ -2313,6 +2340,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; assert_debug_snapshot!(super::entities_with_fields_from_request(Arc::new(connector), &req).unwrap(), @r###" @@ -2588,6 +2617,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; assert_debug_snapshot!(super::entities_with_fields_from_request(Arc::new(connector), &req).unwrap(), @r###" @@ -2728,6 +2759,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; let requests: Vec<_> = super::make_requests( diff --git a/apollo-router/src/plugins/connectors/tracing.rs b/apollo-router/src/plugins/connectors/tracing.rs index 3615c11cf4..13c73fba3c 100644 --- a/apollo-router/src/plugins/connectors/tracing.rs +++ b/apollo-router/src/plugins/connectors/tracing.rs @@ -87,6 +87,8 @@ mod tests { max_requests: None, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; let connectors = Connectors { diff --git a/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs b/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs index 55ef0a5a58..d4341e12d5 100644 --- a/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs +++ b/apollo-router/src/plugins/telemetry/config_new/connector/selectors.rs @@ -363,6 +363,8 @@ mod tests { spec: ConnectSpec::V0_1, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), } } diff --git a/apollo-router/src/plugins/telemetry/config_new/events.rs b/apollo-router/src/plugins/telemetry/config_new/events.rs index a75116ea3b..8a957cc7fa 100644 --- a/apollo-router/src/plugins/telemetry/config_new/events.rs +++ b/apollo-router/src/plugins/telemetry/config_new/events.rs @@ -1232,6 +1232,8 @@ mod tests { spec: ConnectSpec::V0_1, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; let response_key = ResponseKey::RootField { name: "hello".to_string(), @@ -1320,6 +1322,8 @@ mod tests { spec: ConnectSpec::V0_1, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; let response_key = ResponseKey::RootField { name: "hello".to_string(), diff --git a/apollo-router/src/plugins/telemetry/config_new/instruments.rs b/apollo-router/src/plugins/telemetry/config_new/instruments.rs index 7a7b7706d8..1fadf91bd9 100644 --- a/apollo-router/src/plugins/telemetry/config_new/instruments.rs +++ b/apollo-router/src/plugins/telemetry/config_new/instruments.rs @@ -3276,6 +3276,8 @@ mod tests { spec: ConnectSpec::V0_1, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; let response_key = ResponseKey::RootField { name: "hello".to_string(), @@ -3344,6 +3346,8 @@ mod tests { spec: ConnectSpec::V0_1, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }; let response_key = ResponseKey::RootField { name: "hello".to_string(), diff --git a/apollo-router/src/plugins/telemetry/fmt_layer.rs b/apollo-router/src/plugins/telemetry/fmt_layer.rs index a81a183faa..bf22506b37 100644 --- a/apollo-router/src/plugins/telemetry/fmt_layer.rs +++ b/apollo-router/src/plugins/telemetry/fmt_layer.rs @@ -878,6 +878,8 @@ connector: spec: ConnectSpec::V0_1, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }); let response_key = ResponseKey::RootField { name: "hello".to_string(), @@ -1132,6 +1134,8 @@ connector: spec: ConnectSpec::V0_1, request_variables: Default::default(), response_variables: Default::default(), + request_headers: Default::default(), + response_headers: Default::default(), }); let response_key = ResponseKey::RootField { name: "hello".to_string(), From 21083eae0be3aa7d64596b403f1fb7deb8d90d67 Mon Sep 17 00:00:00 2001 From: Andrew McGivery Date: Fri, 24 Jan 2025 12:07:36 -0500 Subject: [PATCH 6/8] Fix build errors post-merge --- apollo-federation/src/sources/connect/json_selection/parser.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apollo-federation/src/sources/connect/json_selection/parser.rs b/apollo-federation/src/sources/connect/json_selection/parser.rs index 404e0c8a29..fd91641e59 100644 --- a/apollo-federation/src/sources/connect/json_selection/parser.rs +++ b/apollo-federation/src/sources/connect/json_selection/parser.rs @@ -32,6 +32,7 @@ use super::location::WithRange; use crate::sources::connect::variable::VariableNamespace; use crate::sources::connect::variable::VariablePathPart; use crate::sources::connect::variable::VariableReference; +use crate::sources::connect::Namespace; // ParseResult is the internal type returned by most ::parse methods, as it is // convenient to use with nom's combinators. The top-level JSONSelection::parse @@ -245,7 +246,7 @@ impl JSONSelection { self.external_var_paths() .into_iter() .flat_map(|var_path| var_path.variable_reference()) - .filter_map(|var_ref| { + .filter_map(|var_ref: VariableReference<'_, Namespace>| { if var_ref.namespace.namespace != Namespace::Request && var_ref.namespace.namespace != Namespace::Response { From a642a40d1fcc303a52962db07ac0678583170ce9 Mon Sep 17 00:00:00 2001 From: Andrew McGivery Date: Fri, 24 Jan 2025 12:14:35 -0500 Subject: [PATCH 7/8] Update test snapshots --- ...it_expand_supergraph@carryover.graphql-2.snap | 6 ++++++ ...nd_supergraph@interface-object.graphql-2.snap | 6 ++++++ .../it_expand_supergraph@keys.graphql-2.snap | 16 ++++++++++++++++ ...xpand_supergraph@nested_inputs.graphql-2.snap | 2 ++ ...and_supergraph@normalize_names.graphql-2.snap | 4 ++++ ...it_expand_supergraph@realistic.graphql-2.snap | 8 ++++++++ ...pand_supergraph@sibling_fields.graphql-2.snap | 4 ++++ .../it_expand_supergraph@simple.graphql-2.snap | 6 ++++++ ..._expand_supergraph@steelthread.graphql-2.snap | 6 ++++++ ...nd_supergraph@types_used_twice.graphql-2.snap | 2 ++ apollo-federation/src/sources/connect/models.rs | 8 ++++++-- ...id_namespace_in_header_variables.graphql.snap | 2 +- ...emplates__invalid-path-parameter.graphql.snap | 2 +- ...espace_in_url_template_variables.graphql.snap | 2 +- 14 files changed, 69 insertions(+), 5 deletions(-) diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@carryover.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@carryover.graphql-2.snap index 83cf15a887..be4d749610 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@carryover.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@carryover.graphql-2.snap @@ -166,6 +166,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ca spec: V0_1, request_variables: {}, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "one_Query_t_0": Connector { id: ConnectId { @@ -380,6 +382,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ca $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "one_T_r_0": Connector { id: ConnectId { @@ -520,5 +524,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ca $this, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@interface-object.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@interface-object.graphql-2.snap index edd2b57f5f..fad26e4cdd 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@interface-object.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@interface-object.graphql-2.snap @@ -147,6 +147,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/in $this, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_Query_itfs_0": Connector { id: ConnectId { @@ -240,6 +242,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/in spec: V0_1, request_variables: {}, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_Query_itf_0": Connector { id: ConnectId { @@ -396,5 +400,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/in $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@keys.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@keys.graphql-2.snap index 04f4c88382..3c37e970cb 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@keys.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@keys.graphql-2.snap @@ -155,6 +155,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ke $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "one_Query_t2_0": Connector { id: ConnectId { @@ -367,6 +369,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ke $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "one_Query_unselected_0": Connector { id: ConnectId { @@ -519,6 +523,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ke $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "one_T_r1_0": Connector { id: ConnectId { @@ -659,6 +665,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ke $this, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "one_T_r2_0": Connector { id: ConnectId { @@ -859,6 +867,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ke $this, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "one_T_r3_0": Connector { id: ConnectId { @@ -1043,6 +1053,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ke response_variables: { $this, }, + request_headers: {}, + response_headers: {}, }, "one_T_r4_0": Connector { id: ConnectId { @@ -1200,6 +1212,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ke $this, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "one_T_r5_0": Connector { id: ConnectId { @@ -1401,5 +1415,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ke response_variables: { $this, }, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@nested_inputs.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@nested_inputs.graphql-2.snap index bd83daf098..19ad3df6d0 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@nested_inputs.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@nested_inputs.graphql-2.snap @@ -262,5 +262,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ne $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@normalize_names.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@normalize_names.graphql-2.snap index 6318b65e64..991eac5ed8 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@normalize_names.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@normalize_names.graphql-2.snap @@ -83,6 +83,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/no spec: V0_1, request_variables: {}, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors-subgraph_Query_user_0": Connector { id: ConnectId { @@ -227,5 +229,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/no $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@realistic.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@realistic.graphql-2.snap index 91d300e6d0..1cea7f9292 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@realistic.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@realistic.graphql-2.snap @@ -301,6 +301,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/re $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_Query_filterUsersByEmailDomain_0": Connector { id: ConnectId { @@ -468,6 +470,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/re $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_Query_usersByCompany_0": Connector { id: ConnectId { @@ -679,6 +683,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/re $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_Query_user_0": Connector { id: ConnectId { @@ -1039,5 +1045,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/re $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@sibling_fields.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@sibling_fields.graphql-2.snap index 8fc72357a1..3ec4b276cc 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@sibling_fields.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@sibling_fields.graphql-2.snap @@ -100,6 +100,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/si spec: V0_1, request_variables: {}, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_T_b_0": Connector { id: ConnectId { @@ -243,5 +245,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/si $this, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@simple.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@simple.graphql-2.snap index 0323ff7192..45755b3616 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@simple.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@simple.graphql-2.snap @@ -83,6 +83,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/si spec: V0_1, request_variables: {}, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_Query_user_0": Connector { id: ConnectId { @@ -227,6 +229,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/si $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_User_d_1": Connector { id: ConnectId { @@ -423,5 +427,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/si $this, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@steelthread.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@steelthread.graphql-2.snap index be3b10be8b..cb8875ac51 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@steelthread.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@steelthread.graphql-2.snap @@ -94,6 +94,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/st spec: V0_1, request_variables: {}, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_Query_user_0": Connector { id: ConnectId { @@ -248,6 +250,8 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/st $args, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, "connectors_User_d_1": Connector { id: ConnectId { @@ -395,5 +399,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/st $this, }, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@types_used_twice.graphql-2.snap b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@types_used_twice.graphql-2.snap index 7514920800..22f6303e0b 100644 --- a/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@types_used_twice.graphql-2.snap +++ b/apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@types_used_twice.graphql-2.snap @@ -143,5 +143,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/ty spec: V0_1, request_variables: {}, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } diff --git a/apollo-federation/src/sources/connect/models.rs b/apollo-federation/src/sources/connect/models.rs index d16dcd7992..d5af0e56cc 100644 --- a/apollo-federation/src/sources/connect/models.rs +++ b/apollo-federation/src/sources/connect/models.rs @@ -615,7 +615,7 @@ mod tests { let connectors = Connector::from_schema(subgraph.schema.schema(), "connectors", ConnectSpec::V0_1) .unwrap(); - assert_debug_snapshot!(&connectors, @r###" + assert_debug_snapshot!(&connectors, @r#" { ConnectId { label: "connectors.json http: GET /users", @@ -736,6 +736,8 @@ mod tests { spec: V0_1, request_variables: {}, response_variables: {}, + request_headers: {}, + response_headers: {}, }, ConnectId { label: "connectors.json http: GET /posts", @@ -868,8 +870,10 @@ mod tests { spec: V0_1, request_variables: {}, response_variables: {}, + request_headers: {}, + response_headers: {}, }, } - "###); + "#); } } diff --git a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@headers__invalid_namespace_in_header_variables.graphql.snap b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@headers__invalid_namespace_in_header_variables.graphql.snap index 0b877d1c13..60102c5e3e 100644 --- a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@headers__invalid_namespace_in_header_variables.graphql.snap +++ b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@headers__invalid_namespace_in_header_variables.graphql.snap @@ -27,7 +27,7 @@ input_file: apollo-federation/src/sources/connect/validation/test_data/headers/i }, Message { code: InvalidHeader, - message: "In `@connect(http.headers:)` on `Query.scalar`: unknown variable `$foo`, must be one of $args, $config, $context", + message: "In `@connect(http.headers:)` on `Query.scalar`: unknown variable `$foo`, must be one of $args, $config, $context, $request", locations: [ 24:49..24:57, ], diff --git a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid-path-parameter.graphql.snap b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid-path-parameter.graphql.snap index b3ad291d4c..4c5e1da162 100644 --- a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid-path-parameter.graphql.snap +++ b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid-path-parameter.graphql.snap @@ -6,7 +6,7 @@ input_file: apollo-federation/src/sources/connect/validation/test_data/uri_templ [ Message { code: InvalidUrl, - message: "In `GET` in `@connect(http:)` on `Query.resources`: unknown variable `$blah`, must be one of $args, $config, $context", + message: "In `GET` in `@connect(http:)` on `Query.resources`: unknown variable `$blah`, must be one of $args, $config, $context, $request", locations: [ 12:23..12:28, ], diff --git a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid_namespace_in_url_template_variables.graphql.snap b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid_namespace_in_url_template_variables.graphql.snap index 8d2bbab60c..e790a59c24 100644 --- a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid_namespace_in_url_template_variables.graphql.snap +++ b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@uri_templates__invalid_namespace_in_url_template_variables.graphql.snap @@ -6,7 +6,7 @@ input_file: apollo-federation/src/sources/connect/validation/test_data/uri_templ [ Message { code: InvalidUrl, - message: "In `GET` in `@connect(http:)` on `Query.unknown`: unknown variable `$foo`, must be one of $args, $config, $context", + message: "In `GET` in `@connect(http:)` on `Query.unknown`: unknown variable `$foo`, must be one of $args, $config, $context, $request", locations: [ 11:31..11:39, ], From 37f83b7f60170043c19f913d361963e84cefdd01 Mon Sep 17 00:00:00 2001 From: Andrew McGivery Date: Fri, 24 Jan 2025 15:26:59 -0500 Subject: [PATCH 8/8] Simplified headers_used check --- apollo-router/src/plugins/connectors/make_requests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/plugins/connectors/make_requests.rs b/apollo-router/src/plugins/connectors/make_requests.rs index a435fa63a4..39590469ce 100644 --- a/apollo-router/src/plugins/connectors/make_requests.rs +++ b/apollo-router/src/plugins/connectors/make_requests.rs @@ -104,7 +104,7 @@ impl RequestInputs { .headers() .iter() .filter_map(|(key, value)| { - if headers_used.contains::(&key.as_str().into()) { + if headers_used.contains(key.as_str()) { return Some(( key.as_str().into(), value.to_str().unwrap_or_default().into(), @@ -128,7 +128,7 @@ impl RequestInputs { .headers .iter() .filter_map(|(key, value)| { - if headers_used.contains::(&key.as_str().into()) { + if headers_used.contains(key.as_str()) { return Some(( key.as_str().into(), value.to_str().unwrap_or_default().into(),