If subgraph batching, do not log the full response when failing to no…

…tify a waiter (apollographql#6150)

.changesets/fix_garypen_log_less_error_for_subgraph_batching.md

@@ -0,0 +1,7 @@
+### If subgraph batching, do not log response data for notification failure ([PR #6150](https://github.com/apollographql/router/pull/6150))
+
+A subgraph response may contain a lot of data and/or PII data.
+
+For a subgraph batching operation, we should not log out the entire subgraph response when failing to notify a waiting batch participant.
+
+By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/6150

apollo-router/src/services/subgraph_service.rs

@@ -1047,6 +1047,8 @@ pub(crate) async fn notify_batch_query(
         Err(e) => {
             for tx in senders {
                 // Try to notify all waiters. If we can't notify an individual sender, then log an error
+                // which, unlike failing to notify on success (see below), contains the the entire error
+                // response.
                 if let Err(log_error) = tx.send(Err(Box::new(e.clone()))).map_err(|error| {
                     FetchError::SubrequestBatchingError {
                         service: service.clone(),
@@ -1076,13 +1078,15 @@ pub(crate) async fn notify_batch_query(
             // graphql_response, so zip_eq shouldn't panic.
             // Use the tx to send a graphql_response message to each waiter.
             for (response, sender) in rs.into_iter().zip_eq(senders) {
-                if let Err(log_error) =
-                    sender
-                        .send(Ok(response))
-                        .map_err(|error| FetchError::SubrequestBatchingError {
-                            service: service.to_string(),
-                            reason: format!("tx send failed: {error:?}"),
-                        })
+                if let Err(log_error) = sender
+                    .send(Ok(response))
+                    // If we fail to notify the waiter that our request succeeded, do not log
+                    // out the entire response since this may be substantial and/or contain
+                    // PII data. Simply log that the send failed.
+                    .map_err(|_error| FetchError::SubrequestBatchingError {
+                        service: service.to_string(),
+                        reason: "tx send failed".to_string(),
+                    })
                 {
                     tracing::error!(service, error=%log_error, "failed to notify sender that batch processing succeeded");
                 }