add documentation and tests for metric_doc macro

Author: aaron-ang

Cargo.lock

@@ -42,3 +42,6 @@ name = "datafusion_doc"
 
 [dependencies]
 inventory = "0.3"
+
+[dev-dependencies]
+datafusion-macros = { workspace = true }

datafusion/doc/Cargo.toml

@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Integration tests for the `#[metric_doc]` macro.
+//!
+//! These tests verify:
+//! 1. Cross-file usage: metrics structs defined in one file, exec in another
+//! 2. Multiple metrics groups: one exec referencing multiple metrics structs
+
+mod test_helpers;
+
+use datafusion_doc::metric_doc_sections::{DocumentedExec, DocumentedMetrics};
+use test_helpers::separate_exec::UserDefinedExec;
+use test_helpers::separate_metrics::{MetricsGroupA, MetricsGroupB};
+
+/// Test that metrics structs in a separate file correctly implement DocumentedMetrics
+#[test]
+fn test_cross_file_metrics_have_documented_metrics_trait() {
+    // MetricsGroupA should implement DocumentedMetrics
+    let doc_a = MetricsGroupA::metric_doc();
+    assert_eq!(doc_a.name, "MetricsGroupA");
+    assert!(doc_a.doc.contains("First group of metrics"));
+    assert_eq!(doc_a.fields.len(), 2);
+
+    // MetricsGroupB should implement DocumentedMetrics
+    let doc_b = MetricsGroupB::metric_doc();
+    assert_eq!(doc_b.name, "MetricsGroupB");
+    assert!(doc_b.doc.contains("Second group of metrics"));
+    assert_eq!(doc_b.fields.len(), 2);
+}
+
+/// Test that an exec with multiple metrics groups correctly implements DocumentedExec
+#[test]
+fn test_exec_with_multiple_metrics_groups() {
+    let exec_doc = UserDefinedExec::exec_doc();
+
+    // Verify exec documentation
+    assert_eq!(exec_doc.name, "UserDefinedExec");
+    assert!(exec_doc.doc.contains("user-defined execution plan"));
+
+    // Verify that both metrics groups are linked
+    assert_eq!(
+        exec_doc.metrics.len(),
+        2,
+        "Expected 2 metrics groups, got {}",
+        exec_doc.metrics.len()
+    );
+
+    // Verify the metrics are the correct ones (order should match declaration order)
+    let metric_names: Vec<&str> = exec_doc.metrics.iter().map(|m| m.name).collect();
+    assert_eq!(
+        metric_names[0], "MetricsGroupA",
+        "Expected MetricsGroupA in metrics, got {}",
+        metric_names[0]
+    );
+    assert_eq!(
+        metric_names[1], "MetricsGroupB",
+        "Expected MetricsGroupB in metrics, got {}",
+        metric_names[1]
+    );
+}
+
+/// Test that field documentation is correctly extracted from metrics structs
+#[test]
+fn test_metrics_field_documentation() {
+    let doc_a = MetricsGroupA::metric_doc();
+
+    // Check that field docs are extracted
+    let field_names: Vec<&str> = doc_a.fields.iter().map(|f| f.name).collect();
+    assert_eq!(field_names[0], "phase_a_time");
+    assert_eq!(field_names[1], "phase_a_rows");
+
+    // Check that field descriptions are captured
+    let time_field = doc_a.fields.iter().find(|f| f.name == "phase_a_time");
+    assert_eq!(
+        time_field.unwrap().doc.trim(),
+        "Time spent executing phase A"
+    );
+}

datafusion/doc/tests/metric_doc_tests.rs

@@ -0,0 +1,21 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Test helper modules for metric_doc macro tests.
+
+pub mod separate_exec;
+pub mod separate_metrics;

datafusion/doc/tests/test_helpers/mod.rs

@@ -0,0 +1,35 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Exec struct that references metrics from a different file.
+//! This demonstrates cross-file usage and multiple metrics groups.
+
+#![allow(dead_code)]
+
+use datafusion_macros::metric_doc;
+
+// Import metrics from the separate file
+use super::separate_metrics::{MetricsGroupA, MetricsGroupB};
+
+/// A user-defined execution plan that demonstrates:
+/// 1. Referencing metrics from a different file
+/// 2. Having multiple metrics groups
+#[metric_doc(MetricsGroupA, MetricsGroupB)]
+pub struct UserDefinedExec {
+    /// Some internal state
+    pub state: String,
+}

datafusion/doc/tests/test_helpers/separate_exec.rs

@@ -0,0 +1,43 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Metrics structs defined in a separate file from the exec.
+//! This demonstrates that metric_doc works across file boundaries.
+
+#![allow(dead_code)]
+
+use datafusion_macros::metric_doc;
+
+/// First group of metrics for UserDefinedExec.
+/// Tracks phase A execution statistics.
+#[metric_doc]
+pub struct MetricsGroupA {
+    /// Time spent executing phase A
+    pub phase_a_time: u64,
+    /// Number of rows processed in phase A
+    pub phase_a_rows: usize,
+}
+
+/// Second group of metrics for UserDefinedExec.
+/// Tracks phase B execution statistics.
+#[metric_doc]
+pub struct MetricsGroupB {
+    /// Time spent executing phase B
+    pub phase_b_time: u64,
+    /// Number of rows processed in phase B
+    pub phase_b_rows: usize,
+}

datafusion/doc/tests/test_helpers/separate_metrics.rs

@@ -15,14 +15,15 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use std::sync::Arc;
-
-use datafusion::prelude::SessionContext;
-use datafusion_execution::TaskContextProvider;
-use datafusion_ffi::execution::FFI_TaskContextProvider;
-use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
-use datafusion_proto::logical_plan::DefaultLogicalExtensionCodec;
+#[cfg(feature = "integration-tests")]
+use {
+    datafusion::prelude::SessionContext, datafusion_execution::TaskContextProvider,
+    datafusion_ffi::execution::FFI_TaskContextProvider,
+    datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec,
+    datafusion_proto::logical_plan::DefaultLogicalExtensionCodec, std::sync::Arc,
+};
 
+#[cfg(feature = "integration-tests")]
 pub fn ctx_and_codec() -> (Arc<SessionContext>, FFI_LogicalExtensionCodec) {
     let ctx = Arc::new(SessionContext::default());
     let task_ctx_provider = Arc::clone(&ctx) as Arc<dyn TaskContextProvider>;

datafusion/ffi/tests/utils/mod.rs

@@ -110,9 +110,72 @@ pub fn user_doc(args: TokenStream, input: TokenStream) -> TokenStream {
 }
 
 /// Helper attribute to register metrics structs or execs for documentation generation.
-/// Usage:
-/// - `#[metric_doc(common)]` on `*_Metrics` structs (derives `DocumentedMetrics`)
-/// - `#[metric_doc(M1, M2)]` on `*_Exec` structs to link metrics (derives `DocumentedExec`)
+///
+/// # Usage
+///
+/// ## On Metrics Structs
+///
+/// Use `#[metric_doc]` (no arguments) for operator-specific metrics:
+/// ```ignore
+/// #[metric_doc]
+/// struct FilterExecMetrics {
+///     /// Common metrics for most operators
+///     baseline_metrics: BaselineMetrics,
+///     /// Selectivity of the filter
+///     selectivity: RatioMetrics,
+/// }
+/// ```
+///
+/// Use `#[metric_doc(common)]` for reusable metrics shared across operators:
+/// ```ignore
+/// #[metric_doc(common)]
+/// pub struct BaselineMetrics {
+///     /// Amount of time the operator was actively using the CPU
+///     elapsed_compute: Time,
+///     /// Total output rows
+///     output_rows: Count,
+/// }
+/// ```
+///
+/// ## On Exec Structs
+///
+/// Reference a single metrics struct:
+/// ```ignore
+/// #[metric_doc(FilterExecMetrics)]
+/// pub struct FilterExec { /* ... */ }
+/// ```
+///
+/// Reference multiple metrics structs (for operators with multiple metric groups):
+/// ```ignore
+/// #[metric_doc(MetricsGroupA, MetricsGroupB)]
+/// pub struct UserDefinedExec { /* ... */ }
+/// ```
+///
+/// ## Cross-File Usage
+///
+/// Metrics structs can be defined in different files/modules from the exec.
+/// The macro resolves references at compile time, so metrics can live anywhere
+/// as long as they are in scope where the exec is defined:
+/// ```ignore
+/// // In metrics/groups.rs
+/// #[metric_doc]
+/// pub struct MetricsGroupA {
+///     /// Time spent in phase A
+///     phase_a_time: Time,
+/// }
+///
+/// #[metric_doc]
+/// pub struct MetricsGroupB {
+///     /// Time spent in phase B
+///     phase_b_time: Time,
+/// }
+///
+/// // In exec/user_defined.rs
+/// use crate::metrics::groups::{MetricsGroupA, MetricsGroupB};
+///
+/// #[metric_doc(MetricsGroupA, MetricsGroupB)]
+/// pub struct UserDefinedExec { /* ... */ }
+/// ```
 #[proc_macro_attribute]
 pub fn metric_doc(args: TokenStream, input: TokenStream) -> TokenStream {
     metric_doc_impl::metric_doc(&args, input)