You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/07/28 09:25:45 UTC
(datafusion) branch main updated: Add name filter to metrics (#23719)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 1b32517ace Add name filter to metrics (#23719)
1b32517ace is described below
commit 1b32517ace9bca3dbc0f65072798aeceeb9ea076
Author: Gabriel <45...@users.noreply.github.com>
AuthorDate: Tue Jul 28 11:07:03 2026 +0200
Add name filter to metrics (#23719)
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes #123` indicates that this PR will close issue #123.
-->
- Closes #.
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
Precedes https://github.com/apache/datafusion/pull/23720.
While displaying a plan with metrics, allow filtering them by name, not
just by category or type.
This is very useful when writing snapshot tests where only a specific
metrics needs to be asserted, without other unrelated metrics polluting
the snapshot assertion.
Regardless of what happens with
https://github.com/apache/datafusion/pull/23720, I think this PR is
still worth it, as it allows creating some really nice `insta` tests
asserting runtime properties reliably by just cherry picking the runtime
metrics relevant for that specific test. This is relevant not only
within DataFusion codebase, but also for other people's codebases using
DataFusion and relying on `insta` for snapshot testing, See an example
of this here:
https://github.com/apache/datafusion/pull/23720/changes#diff-8281405e117428077c19c07afbbe59fed303a3460096e958f761d34f0899b618
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->
While displaying metrics, allows filtering them by name
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->
Yes, by a new small test.
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->
People using the metrics display API will be able to filter metrics by
name.
---
datafusion/physical-expr-common/src/metrics/mod.rs | 40 ++++++++++
datafusion/physical-plan/src/display.rs | 87 ++++++++++++++++++++++
2 files changed, 127 insertions(+)
diff --git a/datafusion/physical-expr-common/src/metrics/mod.rs b/datafusion/physical-expr-common/src/metrics/mod.rs
index d6048a0fcd..146c039c75 100644
--- a/datafusion/physical-expr-common/src/metrics/mod.rs
+++ b/datafusion/physical-expr-common/src/metrics/mod.rs
@@ -418,6 +418,21 @@ impl MetricsSet {
.collect::<Vec<_>>();
Self { metrics }
}
+
+ /// Returns a new `MetricsSet` filtered by metric name.
+ /// Only metrics with the names appearing the list will be kept.
+ pub fn filter_by_names(self, names: &[String]) -> Self {
+ if names.is_empty() {
+ return Self { metrics: vec![] };
+ }
+
+ let metrics = self
+ .metrics
+ .into_iter()
+ .filter(|metric| names.iter().any(|name| name == metric.value().name()))
+ .collect::<Vec<_>>();
+ Self { metrics }
+ }
}
impl Display for MetricsSet {
@@ -966,4 +981,29 @@ mod tests {
metric_names(&metrics)
);
}
+
+ #[test]
+ fn test_filter_by_names() {
+ let metrics = ExecutionPlanMetricsSet::new();
+ MetricBuilder::new(&metrics).output_rows(0);
+ MetricBuilder::new(&metrics).counter("custom_counter", 0);
+
+ assert!(
+ metrics
+ .clone_inner()
+ .filter_by_names(&[])
+ .iter()
+ .next()
+ .is_none()
+ );
+
+ let names = vec!["output_rows".to_string()];
+ let filtered = metrics.clone_inner().filter_by_names(&names);
+
+ assert_eq!(filtered.iter().count(), 1);
+ assert_eq!(
+ filtered.iter().next().unwrap().value().name(),
+ "output_rows"
+ );
+ }
}
diff --git a/datafusion/physical-plan/src/display.rs b/datafusion/physical-plan/src/display.rs
index 6a4d09057b..34493a5f51 100644
--- a/datafusion/physical-plan/src/display.rs
+++ b/datafusion/physical-plan/src/display.rs
@@ -129,6 +129,9 @@ pub struct DisplayableExecutionPlan<'a> {
/// Optional filter by semantic category (rows / bytes / timing).
/// `None` means show all categories; `Some(vec![])` means plan-only.
metric_categories: Option<Vec<MetricCategory>>,
+ /// Optional filter by metric names. Only metric names in this list
+ /// will be rendered.
+ metric_names: Option<Vec<String>>,
// (TreeRender) Maximum total width of the rendered tree
tree_maximum_render_width: usize,
/// Optional summary totals (currently only used by `pgjson`) — the total
@@ -159,6 +162,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: false,
metric_types: Self::default_metric_types(),
metric_categories: None,
+ metric_names: None,
tree_maximum_render_width: 240,
summary: None,
}
@@ -175,6 +179,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: false,
metric_types: Self::default_metric_types(),
metric_categories: None,
+ metric_names: None,
tree_maximum_render_width: 240,
summary: None,
}
@@ -191,6 +196,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: false,
metric_types: Self::default_metric_types(),
metric_categories: None,
+ metric_names: None,
tree_maximum_render_width: 240,
summary: None,
}
@@ -234,6 +240,18 @@ impl<'a> DisplayableExecutionPlan<'a> {
self
}
+ /// Specify which metric names to include.
+ ///
+ /// - An empty vector means plan-only — suppress all metrics.
+ /// - `vec!["metric_1"]` means show only the metric named `metric_1`.
+ ///
+ /// Name filtering is intersected with other types of filters, like metric
+ /// category and metric type.
+ pub fn set_metric_names(mut self, metric_names: Vec<String>) -> Self {
+ self.metric_names = Some(metric_names);
+ self
+ }
+
/// Set the maximum render width for the tree format
pub fn set_tree_maximum_render_width(mut self, width: usize) -> Self {
self.tree_maximum_render_width = width;
@@ -279,6 +297,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: bool,
metric_types: Vec<MetricType>,
metric_categories: Option<Vec<MetricCategory>>,
+ metric_names: Option<Vec<String>>,
}
impl fmt::Display for Wrapper<'_> {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
@@ -291,6 +310,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: self.show_schema,
metric_types: &self.metric_types,
metric_categories: self.metric_categories.as_deref(),
+ metric_names: self.metric_names.as_deref(),
};
accept(self.plan, &mut visitor)
}
@@ -303,6 +323,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: self.show_schema,
metric_types: self.metric_types.clone(),
metric_categories: self.metric_categories.clone(),
+ metric_names: self.metric_names.clone(),
}
}
@@ -324,6 +345,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_statistics: bool,
metric_types: Vec<MetricType>,
metric_categories: Option<Vec<MetricCategory>>,
+ metric_names: Option<Vec<String>>,
}
impl fmt::Display for Wrapper<'_> {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
@@ -336,6 +358,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_statistics: self.show_statistics,
metric_types: &self.metric_types,
metric_categories: self.metric_categories.as_deref(),
+ metric_names: self.metric_names.as_deref(),
graphviz_builder: GraphvizBuilder::default(),
parents: Vec::new(),
};
@@ -355,6 +378,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_statistics: self.show_statistics,
metric_types: self.metric_types.clone(),
metric_categories: self.metric_categories.clone(),
+ metric_names: self.metric_names.clone(),
}
}
@@ -403,6 +427,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: bool,
metric_types: Vec<MetricType>,
metric_categories: Option<Vec<MetricCategory>>,
+ metric_names: Option<Vec<String>>,
summary: Option<AnalyzeSummary>,
}
impl fmt::Display for Wrapper<'_> {
@@ -413,6 +438,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: self.show_schema,
metric_types: &self.metric_types,
metric_categories: self.metric_categories.as_deref(),
+ metric_names: self.metric_names.as_deref(),
objects: HashMap::new(),
parent_ids: Vec::new(),
next_id: 0,
@@ -446,6 +472,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: self.show_schema,
metric_types: self.metric_types.clone(),
metric_categories: self.metric_categories.clone(),
+ metric_names: self.metric_names.clone(),
summary: self.summary,
}
}
@@ -460,6 +487,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: bool,
metric_types: Vec<MetricType>,
metric_categories: Option<Vec<MetricCategory>>,
+ metric_names: Option<Vec<String>>,
}
impl fmt::Display for Wrapper<'_> {
@@ -473,6 +501,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: self.show_schema,
metric_types: &self.metric_types,
metric_categories: self.metric_categories.as_deref(),
+ metric_names: self.metric_names.as_deref(),
};
visitor.pre_visit(self.plan)?;
Ok(())
@@ -486,6 +515,7 @@ impl<'a> DisplayableExecutionPlan<'a> {
show_schema: self.show_schema,
metric_types: self.metric_types.clone(),
metric_categories: self.metric_categories.clone(),
+ metric_names: self.metric_names.clone(),
}
}
@@ -544,6 +574,8 @@ struct IndentVisitor<'a, 'b> {
metric_types: &'a [MetricType],
/// Optional filter by semantic category (rows / bytes / timing).
metric_categories: Option<&'a [MetricCategory]>,
+ /// Optional filter by metric name.
+ metric_names: Option<&'a [String]>,
}
impl ExecutionPlanVisitor for IndentVisitor<'_, '_> {
@@ -563,6 +595,9 @@ impl ExecutionPlanVisitor for IndentVisitor<'_, '_> {
if let Some(cats) = self.metric_categories {
metrics = metrics.filter_by_categories(cats);
}
+ if let Some(names) = self.metric_names {
+ metrics = metrics.filter_by_names(names);
+ }
write!(self.f, ", metrics=[{metrics}]")?;
} else {
write!(self.f, ", metrics=[]")?;
@@ -574,6 +609,9 @@ impl ExecutionPlanVisitor for IndentVisitor<'_, '_> {
if let Some(cats) = self.metric_categories {
metrics = metrics.filter_by_categories(cats);
}
+ if let Some(names) = self.metric_names {
+ metrics = metrics.filter_by_names(names);
+ }
write!(self.f, ", metrics=[{metrics}]")?;
} else {
write!(self.f, ", metrics=[]")?;
@@ -616,6 +654,8 @@ struct GraphvizVisitor<'a, 'b> {
metric_types: &'a [MetricType],
/// Optional filter by semantic category
metric_categories: Option<&'a [MetricCategory]>,
+ /// Optional filter by metric name.
+ metric_names: Option<&'a [String]>,
graphviz_builder: GraphvizBuilder,
/// Used to record parent node ids when visiting a plan.
@@ -660,6 +700,9 @@ impl ExecutionPlanVisitor for GraphvizVisitor<'_, '_> {
if let Some(cats) = self.metric_categories {
metrics = metrics.filter_by_categories(cats);
}
+ if let Some(names) = self.metric_names {
+ metrics = metrics.filter_by_names(names);
+ }
format!("metrics=[{metrics}]")
} else {
"metrics=[]".to_string()
@@ -671,6 +714,9 @@ impl ExecutionPlanVisitor for GraphvizVisitor<'_, '_> {
if let Some(cats) = self.metric_categories {
metrics = metrics.filter_by_categories(cats);
}
+ if let Some(names) = self.metric_names {
+ metrics = metrics.filter_by_names(names);
+ }
format!("metrics=[{metrics}]")
} else {
"metrics=[]".to_string()
@@ -729,6 +775,7 @@ struct PgJsonExecutionPlanVisitor<'a> {
show_schema: bool,
metric_types: &'a [MetricType],
metric_categories: Option<&'a [MetricCategory]>,
+ metric_names: Option<&'a [String]>,
objects: HashMap<u32, serde_json::Value>,
parent_ids: Vec<u32>,
next_id: u32,
@@ -813,6 +860,12 @@ impl PgJsonExecutionPlanVisitor<'_> {
metrics
};
+ let metrics = if let Some(names) = self.metric_names {
+ metrics.filter_by_names(names)
+ } else {
+ metrics
+ };
+
// Build the Extras bucket, while extracting PG-canonical keys to the
// top level.
let mut extras = serde_json::Map::new();
@@ -1701,6 +1754,40 @@ mod tests {
assert_eq!(root["Actual Rows"].as_u64(), Some(42));
assert_eq!(root["Actual Total Time"].as_f64(), Some(5.0));
assert_eq!(root["Extras"]["output_batches"].as_u64(), Some(7));
+
+ let metric_names = vec!["output_rows".to_string()];
+ for rendered in [
+ DisplayableExecutionPlan::with_metrics(plan.as_ref())
+ .set_metric_names(metric_names.clone())
+ .indent(false)
+ .to_string(),
+ DisplayableExecutionPlan::with_full_metrics(plan.as_ref())
+ .set_metric_names(metric_names.clone())
+ .indent(false)
+ .to_string(),
+ DisplayableExecutionPlan::with_metrics(plan.as_ref())
+ .set_metric_names(metric_names.clone())
+ .graphviz()
+ .to_string(),
+ DisplayableExecutionPlan::with_full_metrics(plan.as_ref())
+ .set_metric_names(metric_names.clone())
+ .graphviz()
+ .to_string(),
+ ] {
+ assert!(rendered.contains("output_rows"));
+ assert!(!rendered.contains("elapsed_compute"));
+ assert!(!rendered.contains("output_batches"));
+ }
+
+ let out = DisplayableExecutionPlan::with_metrics(plan.as_ref())
+ .set_metric_names(metric_names)
+ .pgjson(false)
+ .to_string();
+ let value: serde_json::Value = serde_json::from_str(&out).unwrap();
+ let root = value[0].get("Plan").expect("plan");
+ assert_eq!(root["Actual Rows"].as_u64(), Some(42));
+ assert!(root.get("Actual Total Time").is_none());
+ assert!(root.get("Extras").is_none());
}
#[test]
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org