feat: add metric_doc macro to existing metric and exec structs

Author: aaron-ang

Cargo.lock

@@ -185,6 +185,9 @@ sysinfo = "0.37.2"
 test-utils = { path = "../../test-utils" }
 tokio = { workspace = true, features = ["rt-multi-thread", "parking_lot", "fs"] }
 
+[build-dependencies]
+toml = "0.9"
+
 [package.metadata.cargo-machete]
 ignored = ["datafusion-doc", "datafusion-macros", "dashmap"]
 

datafusion/core/Cargo.toml

@@ -19,6 +19,7 @@ use std::env;
 use std::fs;
 use std::io::{Read, Write};
 use std::path::Path;
+use toml::value::Table;
 
 fn main() {
     let out_dir = env::var("OUT_DIR").unwrap();
@@ -34,10 +35,13 @@ fn main() {
 
     // Read Cargo.toml to check dependencies
     let cargo_toml_path = Path::new(&core_crate_dir).join("Cargo.toml");
-    let mut cargo_toml_content = String::new();
-    if let Ok(mut file) = fs::File::open(&cargo_toml_path) {
-        file.read_to_string(&mut cargo_toml_content).unwrap();
-    }
+    let cargo_toml_content =
+        fs::read_to_string(&cargo_toml_path).expect("core Cargo.toml must be readable");
+    let cargo_manifest: toml::Value =
+        toml::from_str(&cargo_toml_content).expect("core Cargo.toml must be valid TOML");
+    let dependencies = manifest_table(&cargo_manifest, "dependencies");
+    let dev_dependencies = manifest_table(&cargo_manifest, "dev-dependencies");
+    let features = manifest_table(&cargo_manifest, "features");
 
     if let Ok(entries) = fs::read_dir(datafusion_dir) {
         for entry in entries.flatten() {
@@ -53,11 +57,9 @@ fn main() {
                         continue;
                     }
 
-                    // Skip if not a dependency in Cargo.toml
-                    // This is a rough check
-                    if !cargo_toml_content.contains(&format!("{crate_name} ="))
-                        && !cargo_toml_content.contains(&format!("\"{crate_name}\""))
-                    {
+                    let is_dependency = dependencies.contains_key(&crate_name)
+                        || dev_dependencies.contains_key(&crate_name);
+                    if !is_dependency {
                         continue;
                     }
 
@@ -81,12 +83,9 @@ fn main() {
     for krate in crates_with_metrics {
         let krate_snake = krate.replace("-", "_");
 
-        let is_optional = cargo_toml_content
-            .lines()
-            .any(|line| line.contains(&krate) && line.contains("optional = true"));
-
-        if is_optional {
-            writeln!(f, "    #[cfg(feature = \"{krate_snake}\")]").unwrap();
+        if is_optional_dependency(&dependencies, &krate) {
+            let feature = dependency_feature(&features, &krate);
+            writeln!(f, "    #[cfg(feature = \"{feature}\")]").unwrap();
         }
 
         writeln!(f, "    {{").unwrap();
@@ -100,6 +99,39 @@ fn main() {
     println!("cargo:rerun-if-changed=Cargo.toml");
 }
 
+fn manifest_table(manifest: &toml::Value, key: &str) -> Table {
+    manifest
+        .get(key)
+        .and_then(|value| value.as_table())
+        .cloned()
+        .unwrap_or_default()
+}
+
+fn is_optional_dependency(dependencies: &Table, crate_name: &str) -> bool {
+    dependencies
+        .get(crate_name)
+        .and_then(|dep| dep.as_table())
+        .and_then(|dep| dep.get("optional"))
+        .and_then(|value| value.as_bool())
+        .unwrap_or(false)
+}
+
+fn dependency_feature(features: &Table, crate_name: &str) -> String {
+    let dep_feature = format!("dep:{crate_name}");
+    for (feature, deps) in features {
+        if let Some(array) = deps.as_array() {
+            for dep in array {
+                if let Some(dep_str) = dep.as_str() {
+                    if dep_str == crate_name || dep_str == dep_feature {
+                        return feature.clone();
+                    }
+                }
+            }
+        }
+    }
+    crate_name.to_string()
+}
+
 fn has_metric_doc(dir: &Path) -> bool {
     if let Ok(entries) = fs::read_dir(dir) {
         for entry in entries.flatten() {

datafusion/core/build.rs

@@ -18,8 +18,6 @@
 //! Print metrics documentation collected via `DocumentedMetrics`/`DocumentedExec`.
 //! Called from doc generation scripts to refresh `docs/source/user-guide/metrics.md`.
 
-use std::collections::HashSet;
-
 use datafusion_expr::metric_doc_sections::{
     ExecDoc, MetricDoc, MetricDocPosition, exec_docs, metric_docs,
 };
@@ -40,32 +38,25 @@ fn main() -> std::io::Result<()> {
         .filter(|m| m.position == MetricDocPosition::Common)
         .collect();
 
-    // Collect names of common metric types for filtering embedded fields
-    let common_metric_names: HashSet<&str> = common.iter().map(|m| m.name).collect();
-
     if !common.is_empty() {
         content.push_str("## Common Metrics\n\n");
         for metric in common {
-            render_metric_doc(&mut content, metric, 3, &common_metric_names);
+            render_metric_doc(&mut content, metric, 3);
         }
     }
 
     if !execs.is_empty() {
         content.push_str("## Operator-specific Metrics\n\n");
         for exec in execs {
-            render_exec_doc(&mut content, exec, &common_metric_names);
+            render_exec_doc(&mut content, exec);
         }
     }
 
     println!("{content}");
     Ok(())
 }
 
-fn render_exec_doc(
-    out: &mut String,
-    exec: &ExecDoc,
-    common_metric_names: &HashSet<&str>,
-) {
+fn render_exec_doc(out: &mut String, exec: &ExecDoc) {
     out.push_str(&heading(3, exec.name));
     out.push_str("\n\n");
 
@@ -86,20 +77,17 @@ fn render_exec_doc(
     metrics.sort_by(|a, b| a.name.cmp(b.name));
 
     if metrics.is_empty() {
-        out.push_str("_No operator-specific metrics documented._\n\n");
+        out.push_str(
+            "_No operator-specific metrics documented (see Common Metrics)._\n\n",
+        );
     } else {
         for metric in metrics {
-            render_metric_doc(out, metric, 4, common_metric_names);
+            render_metric_doc(out, metric, 4);
         }
     }
 }
 
-fn render_metric_doc(
-    out: &mut String,
-    metric: &MetricDoc,
-    heading_level: usize,
-    common_metric_names: &HashSet<&str>,
-) {
+fn render_metric_doc(out: &mut String, metric: &MetricDoc, heading_level: usize) {
     out.push_str(&heading(heading_level, metric.name));
     out.push_str("\n\n");
 
@@ -110,21 +98,9 @@ fn render_metric_doc(
         out.push_str("\n\n");
     }
 
-    // Filter out fields whose type is a common metric (documented separately)
-    let fields: Vec<_> = metric
-        .fields
-        .iter()
-        .filter(|field| !common_metric_names.contains(field.type_name))
-        .collect();
-
-    if fields.is_empty() {
-        out.push_str("_No metrics documented._\n\n");
-        return;
-    }
-
     out.push_str("| Metric | Description |\n");
     out.push_str("| --- | --- |\n");
-    for field in fields {
+    for field in metric.fields {
         out.push_str(&format!("| {} | {} |\n", field.name, sanitize(field.doc)));
     }
     out.push('\n');

datafusion/core/src/bin/print_metric_docs.rs

@@ -37,9 +37,11 @@ bytes = { workspace = true }
 datafusion-common = { workspace = true, features = ["object_store", "parquet"] }
 datafusion-common-runtime = { workspace = true }
 datafusion-datasource = { workspace = true }
+datafusion-doc = { workspace = true }
 datafusion-execution = { workspace = true }
 datafusion-expr = { workspace = true }
 datafusion-functions-aggregate-common = { workspace = true }
+datafusion-macros = { workspace = true }
 datafusion-physical-expr = { workspace = true }
 datafusion-physical-expr-adapter = { workspace = true }
 datafusion-physical-expr-common = { workspace = true }

datafusion/datasource-parquet/Cargo.toml

@@ -15,6 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
+use datafusion_macros::metric_doc;
 use datafusion_physical_plan::metrics::{
     Count, ExecutionPlanMetricsSet, MetricBuilder, MetricType, PruningMetrics,
     RatioMergeStrategy, RatioMetrics, Time,
@@ -26,6 +27,7 @@ use datafusion_physical_plan::metrics::{
 /// through [`ParquetFileReaderFactory`].
 ///
 /// [`ParquetFileReaderFactory`]: super::ParquetFileReaderFactory
+#[metric_doc]
 #[derive(Debug, Clone)]
 pub struct ParquetFileMetrics {
     /// Number of file **ranges** pruned or matched by partition or file level statistics.

datafusion/datasource-parquet/src/metrics.rs

@@ -49,8 +49,10 @@ bzip2 = { workspace = true, optional = true }
 chrono = { workspace = true }
 datafusion-common = { workspace = true, features = ["object_store"] }
 datafusion-common-runtime = { workspace = true }
+datafusion-doc = { workspace = true }
 datafusion-execution = { workspace = true }
 datafusion-expr = { workspace = true }
+datafusion-macros = { workspace = true }
 datafusion-physical-expr = { workspace = true }
 datafusion-physical-expr-adapter = { workspace = true }
 datafusion-physical-expr-common = { workspace = true }

datafusion/datasource/Cargo.toml

@@ -32,6 +32,7 @@ use crate::file_scan_config::FileScanConfig;
 use arrow::datatypes::SchemaRef;
 use datafusion_common::error::Result;
 use datafusion_execution::RecordBatchStream;
+use datafusion_macros::metric_doc;
 use datafusion_physical_plan::metrics::{
     BaselineMetrics, Count, ExecutionPlanMetricsSet, MetricBuilder, Time,
 };
@@ -360,6 +361,7 @@ impl StartableTime {
 /// as other operators.
 ///
 /// [`FileStream`]: <https://github.com/apache/datafusion/blob/main/datafusion/datasource/src/file_stream.rs>
+#[metric_doc]
 pub struct FileStreamMetrics {
     /// Wall clock time elapsed for file opening.
     ///

datafusion/datasource/src/file_stream.rs

@@ -36,9 +36,11 @@ use datafusion_physical_plan::{
 use itertools::Itertools;
 
 use crate::file_scan_config::FileScanConfig;
+use crate::file_stream::FileStreamMetrics;
 use datafusion_common::config::ConfigOptions;
 use datafusion_common::{Constraints, Result, Statistics};
 use datafusion_execution::{SendableRecordBatchStream, TaskContext};
+use datafusion_macros::metric_doc;
 use datafusion_physical_expr::{EquivalenceProperties, Partitioning, PhysicalExpr};
 use datafusion_physical_expr_common::sort_expr::{LexOrdering, PhysicalSortExpr};
 use datafusion_physical_plan::SortOrderPushdownResult;
@@ -224,6 +226,7 @@ pub trait DataSource: Send + Sync + Debug {
 /// the [`FileSource`] trait.
 ///
 /// [`FileSource`]: crate::file::FileSource
+#[metric_doc(FileStreamMetrics)]
 #[derive(Clone, Debug)]
 pub struct DataSourceExec {
     /// The source of the data -- for example, `FileScanConfig` or `MemorySourceConfig`

datafusion/datasource/src/source.rs

@@ -178,6 +178,7 @@ impl Drop for BaselineMetrics {
 
 /// Helper for creating and tracking spill-related metrics for
 /// each operator
+#[metric_doc(common)]
 #[derive(Debug, Clone)]
 pub struct SpillMetrics {
     /// count of spills during the execution of the operator
@@ -202,6 +203,7 @@ impl SpillMetrics {
 }
 
 /// Metrics for tracking batch splitting activity
+#[metric_doc(common)]
 #[derive(Debug, Clone)]
 pub struct SplitMetrics {
     /// Number of times an input [`RecordBatch`] was split