experimental support for query batching (#3837)

Things needing attention - [x] Configuration to match the design - [x] Decide if cloning `parts` is acceptable (in review) It isn't, so not cloned but re-built. - [x] Decide if cloning context for each `SupergraphRequest` is acceptable (in review) - [x] Should we do special handling if `@defer` or `subscription` detected in batch? - [x] Metrics - [x] Find someone happy to test from an appropriately configured Apollo Client and verify it works - [x] Modify Apollo telemetry to create separate root traces for each batch entry Fixes #126  --- **Checklist** Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review. - [x] Changes are compatible[^1] - [x] Documentation[^2] completed - [x] Performance impact assessed and acceptable - Tests added and passing[^3] - [x] Unit Tests - [x] Integration Tests - [x] Manual Tests **Exceptions** Manual testing was performed with `curl` and [apollo client](https://www.apollographql.com/docs/react/api/link/apollo-link-batch-http/) **Notes** [^1]: It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. [^2]: Configuration is an important part of many changes. Where applicable please try to document configuration examples. [^3]: Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. --------- Co-authored-by: Edward Huang <[email protected]> Co-authored-by: Geoffroy Couprie <[email protected]> Co-authored-by: Maria Elisabeth Schreiber <[email protected]>
apollographql · Sep 27, 2023 · aeb5ffe · aeb5ffe
1 parent bca9d86
commit aeb5ffe
Show file tree

Hide file tree

Showing 24 changed files with 3,649 additions and 292 deletions.
diff --git a/.changesets/exp_garypen_126_query_batching.md b/.changesets/exp_garypen_126_query_batching.md
@@ -0,0 +1,21 @@
+### query batching prototype ([Issue #126](https://github.com/apollographql/router/issues/126))
+
+An experimental implementation of query batching which adds support for client request batching to the Apollo Router.
+
+If you’re using Apollo Client, you can leverage the in-built support for batching to reduce the number of individual requests sent to the Apollo Router.
+
+Once [configured](https://www.apollographql.com/docs/react/api/link/apollo-link-batch-http/), Apollo Client will automatically combine multiple operations into a single HTTP request. The number of operations within a batch is client configurable, including the maximum number of operations in a batch and the maximum duration to wait for operations to accumulate before sending the batch request. 
+
+The Apollo Router must be configured to receive batch requests, otherwise it rejects them. When processing a batch request, the router deserializes and processes each operation of a batch independently, and it responds to the client only after all operations of the batch have been completed.
+
+```yaml
+experimental_batching:
+  enabled: true
+  mode: batch_http_link
+```
+
+All operations within a batch will execute concurrently with respect to each other.
+
+Do not attempt to use subscriptions or `@defer` queries within a batch as they are not supported.
+
+By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/3837
diff --git a/apollo-router/feature_discussions.json b/apollo-router/feature_discussions.json
@@ -3,7 +3,8 @@
     "experimental_retry": "https://github.com/apollographql/router/discussions/2241",
     "experimental_response_trace_id": "https://github.com/apollographql/router/discussions/2147",
     "experimental_logging": "https://github.com/apollographql/router/discussions/1961",
-    "experimental_http_max_request_bytes": "https://github.com/apollographql/router/discussions/3220"
+    "experimental_http_max_request_bytes": "https://github.com/apollographql/router/discussions/3220",
+    "experimental_batching": "https://github.com/apollographql/router/discussions/3840"
   },
   "preview": {
     "preview_directives": "https://github.com/apollographql/router/discussions/3754"

diff --git a/apollo-router/src/configuration/metrics.rs b/apollo-router/src/configuration/metrics.rs
@@ -313,6 +313,12 @@ impl Metrics {
             opt.tracing.zipkin,
             "$.tracing.zipkin[?(@.endpoint)]"
         );
+        log_usage_metrics!(
+            value.apollo.router.config.batching,
+            "$.experimental_batching[?(@.enabled == true)]",
+            opt.mode,
+            "$.mode"
+        );
     }
 }
 

diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs
@@ -185,6 +185,10 @@ pub struct Configuration {
 
     #[serde(default, skip_serializing, skip_deserializing)]
     pub(crate) notify: Notify<String, graphql::Response>,
+
+    /// Batching configuration.
+    #[serde(default)]
+    pub(crate) experimental_batching: Batching,
 }
 
 impl PartialEq for Configuration {
@@ -234,6 +238,7 @@ impl<'de> serde::Deserialize<'de> for Configuration {
             limits: Limits,
             experimental_chaos: Chaos,
             experimental_graphql_validation_mode: GraphQLValidationMode,
+            experimental_batching: Batching,
         }
         let ad_hoc: AdHocConfiguration = serde::Deserialize::deserialize(deserializer)?;
 
@@ -252,6 +257,7 @@ impl<'de> serde::Deserialize<'de> for Configuration {
             .chaos(ad_hoc.experimental_chaos)
             .uplink(ad_hoc.uplink)
             .graphql_validation_mode(ad_hoc.experimental_graphql_validation_mode)
+            .experimental_batching(ad_hoc.experimental_batching)
             .build()
             .map_err(|e| serde::de::Error::custom(e.to_string()))
     }
@@ -288,6 +294,7 @@ impl Configuration {
         chaos: Option<Chaos>,
         uplink: Option<UplinkConfig>,
         graphql_validation_mode: Option<GraphQLValidationMode>,
+        experimental_batching: Option<Batching>,
     ) -> Result<Self, ConfigurationError> {
         #[cfg(not(test))]
         let notify_queue_cap = match apollo_plugins.get(APOLLO_SUBSCRIPTION_PLUGIN_NAME) {
@@ -322,6 +329,7 @@ impl Configuration {
             },
             tls: tls.unwrap_or_default(),
             uplink,
+            experimental_batching: experimental_batching.unwrap_or_default(),
             #[cfg(test)]
             notify: notify.unwrap_or_default(),
             #[cfg(not(test))]
@@ -360,6 +368,7 @@ impl Configuration {
         chaos: Option<Chaos>,
         uplink: Option<UplinkConfig>,
         graphql_validation_mode: Option<GraphQLValidationMode>,
+        experimental_batching: Option<Batching>,
     ) -> Result<Self, ConfigurationError> {
         let configuration = Self {
             validated_yaml: Default::default(),
@@ -382,6 +391,7 @@ impl Configuration {
             apq: apq.unwrap_or_default(),
             preview_persisted_queries: persisted_query.unwrap_or_default(),
             uplink,
+            experimental_batching: experimental_batching.unwrap_or_default(),
         };
 
         configuration.validate()
@@ -1273,3 +1283,23 @@ fn default_graphql_path() -> String {
 fn default_graphql_introspection() -> bool {
     false
 }
+
+#[derive(Clone, Debug, Default, Error, Display, Serialize, Deserialize, JsonSchema)]
+#[serde(deny_unknown_fields, rename_all = "snake_case")]
+pub(crate) enum BatchingMode {
+    /// batch_http_link
+    #[default]
+    BatchHttpLink,
+}
+
+/// Configuration for Batching
+#[derive(Debug, Clone, Default, Deserialize, Serialize, JsonSchema)]
+#[serde(deny_unknown_fields)]
+pub(crate) struct Batching {
+    /// Activates Batching (disabled by default)
+    #[serde(default)]
+    pub(crate) enabled: bool,
+
+    /// Batching mode
+    pub(crate) mode: BatchingMode,
+}
diff --git a/.../snapshots/apollo_router__configuration__metrics__test__metrics@batching.router.yaml.snap b/.../snapshots/apollo_router__configuration__metrics__test__metrics@batching.router.yaml.snap
@@ -0,0 +1,8 @@
+---
+source: apollo-router/src/configuration/metrics.rs
+expression: "&metrics.metrics"
+---
+value.apollo.router.config.batching:
+  - 1
+  - opt__mode__: batch_http_link
+
diff --git a/...r/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/...r/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap
@@ -1009,6 +1009,37 @@ expression: "&schema"
       },
       "additionalProperties": false
     },
+    "experimental_batching": {
+      "description": "Batching configuration.",
+      "default": {
+        "enabled": false,
+        "mode": "batch_http_link"
+      },
+      "type": "object",
+      "required": [
+        "mode"
+      ],
+      "properties": {
+        "enabled": {
+          "description": "Activates Batching (disabled by default)",
+          "default": false,
+          "type": "boolean"
+        },
+        "mode": {
+          "description": "Batching mode",
+          "oneOf": [
+            {
+              "description": "batch_http_link",
+              "type": "string",
+              "enum": [
+                "batch_http_link"
+              ]
+            }
+          ]
+        }
+      },
+      "additionalProperties": false
+    },
     "experimental_chaos": {
       "description": "Configuration for chaos testing, trying to reproduce bugs that require uncommon conditions. You probably don’t want this in production!",
       "default": {

diff --git a/apollo-router/src/configuration/testdata/metrics/batching.router.yaml b/apollo-router/src/configuration/testdata/metrics/batching.router.yaml
@@ -0,0 +1,3 @@
+experimental_batching:
+  enabled: true
+  mode: batch_http_link
diff --git a/apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs b/apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs
@@ -209,11 +209,12 @@ impl Exporter {
         })
     }
 
-    fn extract_root_trace(
+    fn extract_root_traces(
         &mut self,
         span: &LightSpanData,
         child_nodes: Vec<TreeData>,
-    ) -> Result<Box<proto::reports::Trace>, Error> {
+    ) -> Result<Vec<proto::reports::Trace>, Error> {
+        let mut results: Vec<proto::reports::Trace> = vec![];
         let http = extract_http_data(span);
         let mut root_trace = proto::reports::Trace {
             start_time: Some(span.start_time.into()),
@@ -236,17 +237,19 @@ impl Exporter {
                     client_version,
                     duration_ns,
                 } => {
-                    if http.method != Method::Unknown as i32 {
-                        let root_http = root_trace
-                            .http
-                            .as_mut()
-                            .expect("http was extracted earlier, qed");
-                        root_http.request_headers = http.request_headers;
-                        root_http.response_headers = http.response_headers;
+                    for trace in results.iter_mut() {
+                        if http.method != Method::Unknown as i32 {
+                            let root_http = trace
+                                .http
+                                .as_mut()
+                                .expect("http was extracted earlier, qed");
+                            root_http.request_headers = http.request_headers.clone();
+                            root_http.response_headers = http.response_headers.clone();
+                        }
+                        trace.client_name = client_name.clone().unwrap_or_default();
+                        trace.client_version = client_version.clone().unwrap_or_default();
+                        trace.duration_ns = duration_ns;
                     }
-                    root_trace.client_name = client_name.unwrap_or_default();
-                    root_trace.client_version = client_version.unwrap_or_default();
-                    root_trace.duration_ns = duration_ns;
                 }
                 TreeData::Supergraph {
                     operation_signature,
@@ -259,6 +262,7 @@ impl Exporter {
                         variables_json,
                         operation_name,
                     });
+                    results.push(root_trace.clone());
                 }
                 TreeData::Execution(operation_type) => {
                     if operation_type == OperationKind::Subscription.as_apollo_operation_type() {
@@ -282,21 +286,17 @@ impl Exporter {
             }
         }
 
-        Ok(Box::new(root_trace))
+        Ok(results)
     }
 
-    fn extract_trace(&mut self, span: LightSpanData) -> Result<Box<proto::reports::Trace>, Error> {
-        self.extract_data_from_spans(&span)?
-            .pop()
-            .and_then(|node| {
-                match node {
-                    TreeData::Request(trace) | TreeData::SubscriptionEvent(trace) => {
-                        Some(trace)
-                    }
-                    _ => None
-                }
-            })
-            .expect("root trace must exist because it is constructed on the request or subscription_event span, qed")
+    fn extract_traces(&mut self, span: LightSpanData) -> Result<Vec<proto::reports::Trace>, Error> {
+        let mut results = vec![];
+        for node in self.extract_data_from_spans(&span)? {
+            if let TreeData::Request(trace) | TreeData::SubscriptionEvent(trace) = node {
+                results.push(*trace?)
+            }
+        }
+        Ok(results)
     }
 
     fn extract_data_from_spans(&mut self, span: &LightSpanData) -> Result<Vec<TreeData>, Error> {
@@ -417,11 +417,11 @@ impl Exporter {
                 });
                 child_nodes
             }
-            _ if span.attributes.get(&APOLLO_PRIVATE_REQUEST).is_some() => {
-                vec![TreeData::Request(
-                    self.extract_root_trace(span, child_nodes),
-                )]
-            }
+            _ if span.attributes.get(&APOLLO_PRIVATE_REQUEST).is_some() => self
+                .extract_root_traces(span, child_nodes)?
+                .into_iter()
+                .map(|node| TreeData::Request(Ok(Box::new(node))))
+                .collect(),
             ROUTER_SPAN_NAME => {
                 child_nodes.push(TreeData::Router {
                     http: Box::new(extract_http_data(span)),
@@ -550,9 +550,10 @@ impl Exporter {
                         .to_string(),
                 ));
 
-                vec![TreeData::SubscriptionEvent(
-                    self.extract_root_trace(span, child_nodes),
-                )]
+                self.extract_root_traces(span, child_nodes)?
+                    .into_iter()
+                    .map(|node| TreeData::SubscriptionEvent(Ok(Box::new(node))))
+                    .collect()
             }
             _ => child_nodes,
         })
@@ -705,12 +706,14 @@ impl SpanExporter for Exporter {
             if span.attributes.get(&APOLLO_PRIVATE_REQUEST).is_some()
                 || span.name == SUBSCRIPTION_EVENT_SPAN_NAME
             {
-                match self.extract_trace(span.into()) {
-                    Ok(mut trace) => {
-                        let mut operation_signature = Default::default();
-                        std::mem::swap(&mut trace.signature, &mut operation_signature);
-                        if !operation_signature.is_empty() {
-                            traces.push((operation_signature, *trace));
+                match self.extract_traces(span.into()) {
+                    Ok(extracted_traces) => {
+                        for mut trace in extracted_traces {
+                            let mut operation_signature = Default::default();
+                            std::mem::swap(&mut trace.signature, &mut operation_signature);
+                            if !operation_signature.is_empty() {
+                                traces.push((operation_signature, trace));
+                            }
                         }
                     }
                     Err(Error::MultipleErrors(errors)) => {