Skip to content

Commit

Permalink
refactor(telemetry): more readable code to redact errors going to apo…
Browse files Browse the repository at this point in the history
…llo studio (#3030)

related to
#3011 (comment)

---------

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
  • Loading branch information
bnjjj authored and garypen committed May 10, 2023
1 parent c854055 commit d02d73a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changesets/maint_bnjjj_follow_up_3011.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
### Refactor the way we're redacting errors for Apollo telemetry

Preprocess errors in a more readable way.

By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/3030
34 changes: 19 additions & 15 deletions apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,10 +544,8 @@ fn extract_ftv1_trace(
) -> Option<Result<Box<proto::reports::Trace>, Error>> {
if let Value::String(s) = v {
if let Some(mut t) = decode_ftv1_trace(s.as_str()) {
if error_config.redact || !error_config.send {
if let Some(root) = &mut t.root {
redact_node_errors(root, !error_config.send);
}
if let Some(root) = &mut t.root {
preprocess_errors(root, error_config);
}
return Some(Ok(Box::new(t)));
}
Expand All @@ -556,19 +554,19 @@ fn extract_ftv1_trace(
None
}

fn redact_node_errors(t: &mut proto::reports::trace::Node, to_delete: bool) {
if to_delete {
t.error = Vec::new();
} else {
fn preprocess_errors(t: &mut proto::reports::trace::Node, error_config: &ErrorConfiguration) {
if error_config.send {
t.error.iter_mut().for_each(|err| {
err.message = String::from("<redacted>");
err.location = Vec::new();
err.json = String::new();
});
} else {
t.error = Vec::new();
}
t.child
.iter_mut()
.for_each(|n| redact_node_errors(n, to_delete));
.for_each(|n| preprocess_errors(n, error_config));
}

pub(crate) fn decode_ftv1_trace(string: &str) -> Option<proto::reports::Trace> {
Expand Down Expand Up @@ -775,7 +773,7 @@ mod test {
use crate::plugins::telemetry::apollo_exporter::proto::reports::trace::query_plan_node::{DeferNodePrimary, DeferredNode, ResponsePathElement};
use crate::plugins::telemetry::apollo_exporter::proto::reports::trace::{QueryPlanNode, Node, Error};
use crate::plugins::telemetry::apollo_exporter::proto::reports::trace::query_plan_node::response_path_element::Id;
use crate::plugins::telemetry::tracing::apollo_telemetry::{ChildNodes, extract_ftv1_trace, extract_i64, extract_json, extract_path, extract_string, TreeData, redact_node_errors};
use crate::plugins::telemetry::tracing::apollo_telemetry::{ChildNodes, extract_ftv1_trace, extract_i64, extract_json, extract_path, extract_string, TreeData, preprocess_errors};

fn elements(tree_data: Vec<TreeData>) -> Vec<&'static str> {
let mut elements = Vec::new();
Expand Down Expand Up @@ -940,7 +938,7 @@ mod test {
}

#[test]
fn test_redact_node_errors() {
fn test_preprocess_errors() {
let sub_node = Node {
error: vec![Error {
message: "this is my error".to_string(),
Expand Down Expand Up @@ -968,8 +966,11 @@ mod test {
..Default::default()
};
node.child.push(sub_node);

redact_node_errors(&mut node, false);
let error_config = ErrorConfiguration {
send: true,
redact: true,
};
preprocess_errors(&mut node, &error_config);
assert!(node.error[0].json.is_empty());
assert!(node.error[0].location.is_empty());
assert_eq!(node.error[0].message.as_str(), "<redacted>");
Expand Down Expand Up @@ -1014,8 +1015,11 @@ mod test {
..Default::default()
};
node.child.push(sub_node);

redact_node_errors(&mut node, true);
let error_config = ErrorConfiguration {
send: false,
redact: true,
};
preprocess_errors(&mut node, &error_config);
assert!(node.error.is_empty());
assert!(node.child[0].error.is_empty());
}
Expand Down

0 comments on commit d02d73a

Please sign in to comment.