fix(criterion-compat): build walltime URIs like instrumented URIs

Walltime mode rebuilt the benchmark URI in the analysis phase from
`current_file`/`macro_group`, which are only set by
`criterion_group!`/`criterion_main!`. Bypassing the macros with a
custom main produced URIs like `::::my_group::my_bench`.

Build the URI once in `BenchmarkGroup` (skipping empty segments and
deriving the file from the caller location when the macros are
bypassed) and thread it through `BenchmarkId::codspeed_uri`, so
walltime reports the same URI as the instrumented modes.

Also derive the caller file per `benchmark_group` call instead of
filling it once on `Criterion`, so benchmark functions living in
different files don't inherit a stale file from a previous call.

Fixes COD-2324

Author: not-matthias

crates/criterion_compat/Cargo.toml

@@ -70,5 +70,5 @@ name = "test_benches"
 harness = false
 
 [[bench]]
-name = "repro_custom_main"
+name = "custom_main"
 harness = false

…rion_compat/benches/repro_custom_main.rs …/criterion_compat/benches/custom_main.rscrates/criterion_compat/benches/repro_custom_main.rs renamed to crates/criterion_compat/benches/custom_main.rs crates/criterion_compat/benches/repro_custom_main.rs renamed to crates/criterion_compat/benches/custom_main.rs

@@ -1,5 +1,5 @@
-/// Reproducer for COD-2324: URI formatting issues when users bypass criterion_group!/criterion_main!
-/// and use a custom main function (like the rtmalloc project does).
+/// Test bench for custom main patterns (COD-2324).
+/// Verifies URI formatting when users bypass criterion_group!/criterion_main!.
 use codspeed_criterion_compat::{criterion_group, BenchmarkId, Criterion};
 
 fn bench_with_group(c: &mut Criterion) {

crates/criterion_compat/criterion_fork/src/analysis/mod.rs

@@ -265,11 +265,9 @@ pub(crate) fn common<M: Measurement, T: ?Sized>(
 mod codspeed {
     use crate::{measurement::Measurement, report::BenchmarkId, Criterion};
 
-    /// WARNING: Keep URI generation in sync with `codspeed-criterion-compat::compat::group::run_bench`
-    pub fn create_uri_and_name<M: Measurement>(
-        id: &BenchmarkId,
-        c: &Criterion<M>,
-    ) -> (String, String) {
+    /// The URI is built once in `BenchmarkGroup::build_codspeed_uri` and threaded through
+    /// `BenchmarkId::codspeed_uri`, so that all runner modes report the same URI.
+    pub fn create_uri_and_name(id: &BenchmarkId) -> (String, String) {
         let mut bench_name = id.group_id.clone();
         if let Some(ref function_name) = id.function_id {
             bench_name.push_str(&format!("::{}", function_name));
@@ -278,15 +276,7 @@ mod codspeed {
             bench_name.push_str(&format!("[{}]", parameter));
         }
 
-        let git_relative_file_path = codspeed::utils::get_git_relative_path(&c.current_file);
-        let uri = format!(
-            "{}::{}::{}",
-            git_relative_file_path.to_string_lossy(),
-            &c.macro_group,
-            bench_name
-        );
-
-        (uri, bench_name)
+        (id.codspeed_uri.clone(), bench_name)
     }
 
     pub(crate) fn collect_walltime_results<M: Measurement>(
@@ -295,7 +285,7 @@ mod codspeed {
         iters: &[f64],
         times: &[f64],
     ) {
-        let (uri, bench_name) = create_uri_and_name(id, c);
+        let (uri, bench_name) = create_uri_and_name(id);
 
         if let Err(error) =
             ::codspeed::instrument_hooks::InstrumentHooks::instance().set_executed_benchmark(&uri)
@@ -320,6 +310,42 @@ mod codspeed {
             max_time_ns,
         );
     }
+
+    #[cfg(test)]
+    mod tests {
+        use super::*;
+
+        /// COD-2324: when `criterion_group!`/`criterion_main!` are bypassed, the URI threaded
+        /// through `BenchmarkId::codspeed_uri` must be used as-is, without empty
+        /// file/macro_group segments producing `::::my_group::my_bench`.
+        #[test]
+        fn create_uri_uses_threaded_codspeed_uri() {
+            let id = BenchmarkId::new(
+                "my_group".into(),
+                Some("my_bench".into()),
+                None,
+                None,
+                "my_group::my_bench".into(),
+            );
+            let (uri, bench_name) = create_uri_and_name(&id);
+            assert_eq!(uri, "my_group::my_bench");
+            assert_eq!(bench_name, "my_group::my_bench");
+        }
+
+        #[test]
+        fn create_uri_with_file_and_parameter() {
+            let id = BenchmarkId::new(
+                "my_group".into(),
+                Some("parameterized".into()),
+                Some("42".into()),
+                None,
+                "benches/custom_main.rs::my_group::parameterized[42]".into(),
+            );
+            let (uri, bench_name) = create_uri_and_name(&id);
+            assert_eq!(uri, "benches/custom_main.rs::my_group::parameterized[42]");
+            assert_eq!(bench_name, "my_group::parameterized[42]");
+        }
+    }
 }
 
 fn base_dir_exists(id: &BenchmarkId, baseline: &str, output_directory: &Path) -> bool {

crates/criterion_compat/criterion_fork/src/benchmark_group.rs

@@ -7,6 +7,7 @@ use crate::report::Report;
 use crate::report::ReportContext;
 use crate::routine::{Function, Routine};
 use crate::{Bencher, Criterion, Mode, PlotConfiguration, SamplingMode, Throughput};
+use codspeed::utils::get_git_relative_path;
 use std::time::Duration;
 
 /// Structure used to group together a set of related benchmarks, along with custom configuration
@@ -82,6 +83,8 @@ pub struct BenchmarkGroup<'a, M: Measurement> {
     any_matched: bool,
     partial_config: PartialBenchmarkConfig,
     throughput: Option<Throughput>,
+    /// The file the benchmark group was created in, for CodSpeed URI generation.
+    codspeed_current_file: String,
 }
 impl<'a, M: Measurement> BenchmarkGroup<'a, M> {
     /// Changes the size of the sample for this benchmark
@@ -235,14 +238,19 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> {
         self
     }
 
-    pub(crate) fn new(criterion: &mut Criterion<M>, group_name: String) -> BenchmarkGroup<'_, M> {
+    pub(crate) fn new(
+        criterion: &mut Criterion<M>,
+        group_name: String,
+        codspeed_current_file: String,
+    ) -> BenchmarkGroup<'_, M> {
         BenchmarkGroup {
             criterion,
             group_name,
             all_ids: vec![],
             any_matched: false,
             partial_config: PartialBenchmarkConfig::default(),
             throughput: None,
+            codspeed_current_file,
         }
     }
 
@@ -270,6 +278,36 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> {
         self
     }
 
+    fn build_codspeed_uri(&self, id: &BenchmarkId) -> String {
+        let mut uri = String::new();
+
+        if !self.codspeed_current_file.is_empty() {
+            let git_relative_file_path = get_git_relative_path(&self.codspeed_current_file);
+            let file_str = git_relative_file_path.to_string_lossy();
+            if !file_str.is_empty() {
+                uri.push_str(&file_str);
+            }
+        }
+        if !self.criterion.macro_group.is_empty() {
+            if !uri.is_empty() {
+                uri.push_str("::");
+            }
+            uri.push_str(&self.criterion.macro_group);
+        }
+        if !uri.is_empty() {
+            uri.push_str("::");
+        }
+        uri.push_str(&self.group_name);
+        if let Some(function_name) = &id.function_name {
+            uri = format!("{uri}::{function_name}");
+        }
+        if let Some(parameter) = &id.parameter {
+            uri = format!("{uri}[{parameter}]");
+        }
+
+        uri
+    }
+
     fn run_bench<F, I>(&mut self, id: BenchmarkId, input: &I, f: F)
     where
         F: FnMut(&mut Bencher<'_, M>, &I),
@@ -281,11 +319,13 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> {
             plot_config: self.partial_config.plot_config.clone(),
         };
 
+        let codspeed_uri = self.build_codspeed_uri(&id);
         let mut id = InternalBenchmarkId::new(
             self.group_name.clone(),
             id.function_name,
             id.parameter,
             self.throughput.clone(),
+            codspeed_uri,
         );
 
         assert!(

crates/criterion_compat/criterion_fork/src/connection.rs

@@ -293,6 +293,7 @@ pub struct RawBenchmarkId {
     function_id: Option<String>,
     value_str: Option<String>,
     throughput: Vec<Throughput>,
+    codspeed_uri: String,
 }
 impl From<&InternalBenchmarkId> for RawBenchmarkId {
     fn from(other: &InternalBenchmarkId) -> RawBenchmarkId {
@@ -301,6 +302,7 @@ impl From<&InternalBenchmarkId> for RawBenchmarkId {
             function_id: other.function_id.clone(),
             value_str: other.value_str.clone(),
             throughput: other.throughput.iter().cloned().collect(),
+            codspeed_uri: other.codspeed_uri.clone(),
         }
     }
 }

crates/criterion_compat/criterion_fork/src/html/mod.rs

@@ -460,7 +460,7 @@ impl Report for Html {
 
             if samples_with_function.len() > 1 {
                 let subgroup_id =
-                    BenchmarkId::new(group_id.clone(), Some(function_id.clone()), None, None);
+                    BenchmarkId::new(group_id.clone(), Some(function_id.clone()), None, None, String::new());
 
                 self.generate_summary(
                     &subgroup_id,
@@ -481,7 +481,7 @@ impl Report for Html {
 
             if samples_with_value.len() > 1 {
                 let subgroup_id =
-                    BenchmarkId::new(group_id.clone(), None, Some(value_str.clone()), None);
+                    BenchmarkId::new(group_id.clone(), None, Some(value_str.clone()), None, String::new());
 
                 self.generate_summary(&subgroup_id, &samples_with_value, context, formatter, false);
             }
@@ -509,7 +509,7 @@ impl Report for Html {
         all_data.sort_by_key(|(id, _)| id.function_id.as_ref());
 
         self.generate_summary(
-            &BenchmarkId::new(group_id, None, None, None),
+            &BenchmarkId::new(group_id, None, None, None, String::new()),
             &all_data,
             context,
             formatter,

crates/criterion_compat/criterion_fork/src/lib.rs

@@ -398,6 +398,26 @@ mod codspeed {
         pub fn set_macro_group(&mut self, macro_group: impl Into<String>) {
             self.macro_group = macro_group.into();
         }
+
+        /// Returns the file the benchmark lives in, for CodSpeed URI generation.
+        ///
+        /// Uses `current_file` when set explicitly (via `set_current_file`), otherwise derives
+        /// it from the caller location. Derived per call so that benchmark functions living in
+        /// different files don't inherit a stale file from a previous `benchmark_group` call.
+        #[track_caller]
+        pub(crate) fn codspeed_current_file(&self) -> String {
+            if !self.current_file.is_empty() {
+                return self.current_file.clone();
+            }
+            let caller = std::panic::Location::caller();
+            match std::env::var("CODSPEED_CARGO_WORKSPACE_ROOT") {
+                Ok(workspace_root) => std::path::PathBuf::from(workspace_root)
+                    .join(caller.file())
+                    .to_string_lossy()
+                    .into_owned(),
+                Err(_) => caller.file().to_string(),
+            }
+        }
     }
 }
 
@@ -1201,7 +1221,9 @@ https://bheisler.github.io/criterion.rs/book/faq.html
     /// ```
     /// # Panics:
     /// Panics if the group name is empty
+    #[track_caller]
     pub fn benchmark_group<S: Into<String>>(&mut self, group_name: S) -> BenchmarkGroup<'_, M> {
+        let codspeed_current_file = self.codspeed_current_file();
         let group_name = group_name.into();
         assert!(!group_name.is_empty(), "Group name must not be empty.");
 
@@ -1210,7 +1232,7 @@ https://bheisler.github.io/criterion.rs/book/faq.html
                 .unwrap();
         }
 
-        BenchmarkGroup::new(self, group_name)
+        BenchmarkGroup::new(self, group_name, codspeed_current_file)
     }
 }
 impl<M> Criterion<M>
@@ -1238,6 +1260,7 @@ where
     /// criterion_group!(benches, bench);
     /// criterion_main!(benches);
     /// ```
+    #[track_caller]
     pub fn bench_function<F>(&mut self, id: &str, f: F) -> &mut Criterion<M>
     where
         F: FnMut(&mut Bencher<'_, M>),
@@ -1270,6 +1293,7 @@ where
     /// criterion_group!(benches, bench);
     /// criterion_main!(benches);
     /// ```
+    #[track_caller]
     pub fn bench_with_input<F, I>(&mut self, id: BenchmarkId, input: &I, f: F) -> &mut Criterion<M>
     where
         F: FnMut(&mut Bencher<'_, M>, &I),

crates/criterion_compat/criterion_fork/src/macros.rs

@@ -69,7 +69,6 @@ macro_rules! criterion_group {
                 .configure_from_args();
             $(
                 if std::env::var("CODSPEED_ENV").is_ok() {
-                    criterion.set_current_file($crate::abs_file!());
                     criterion.set_macro_group(format!("{}::{}", stringify!($name), stringify!($target)));
                 }
                 $target(&mut criterion);

crates/criterion_compat/criterion_fork/src/report.rs

@@ -67,6 +67,8 @@ pub struct BenchmarkId {
     pub function_id: Option<String>,
     pub value_str: Option<String>,
     pub throughput: Option<Throughput>,
+    #[serde(default)]
+    pub codspeed_uri: String,
     full_id: String,
     directory_name: String,
     title: String,
@@ -114,6 +116,7 @@ impl BenchmarkId {
         function_id: Option<String>,
         value_str: Option<String>,
         throughput: Option<Throughput>,
+        codspeed_uri: String,
     ) -> BenchmarkId {
         let full_id = match (&function_id, &value_str) {
             (Some(func), Some(val)) => format!("{}/{}/{}", group_id, func, val),
@@ -153,6 +156,7 @@ impl BenchmarkId {
             function_id,
             value_str,
             throughput,
+            codspeed_uri,
             full_id,
             directory_name,
             title,
@@ -823,6 +827,7 @@ mod test {
             Some("function".to_owned()),
             Some("value".to_owned()),
             None,
+            String::new(),
         );
         let mut directories = HashSet::new();
         directories.insert(existing_id.as_directory_name().to_owned());
@@ -840,7 +845,7 @@ mod test {
     #[test]
     fn test_benchmark_id_make_long_directory_name_unique() {
         let long_name = (0..MAX_DIRECTORY_NAME_LEN).map(|_| 'a').collect::<String>();
-        let existing_id = BenchmarkId::new(long_name, None, None, None);
+        let existing_id = BenchmarkId::new(long_name, None, None, None, String::new());
         let mut directories = HashSet::new();
         directories.insert(existing_id.as_directory_name().to_owned());
 

crates/criterion_compat/src/compat/criterion.rs

@@ -122,24 +122,27 @@ impl<M: Measurement> Criterion<M> {
 
     #[track_caller]
     pub fn benchmark_group<S: Into<String>>(&mut self, group_name: S) -> BenchmarkGroup<M> {
-        self.fill_missing_file_from_caller();
-        BenchmarkGroup::<M>::new(self, group_name.into())
+        let current_file = self.codspeed_current_file();
+        BenchmarkGroup::<M>::new(self, group_name.into(), current_file)
     }
 
-    /// When `current_file` wasn't set by `criterion_group!`, derive it from the caller location.
+    /// Returns the file the benchmark lives in, for CodSpeed URI generation.
+    ///
+    /// Uses `current_file` when set explicitly (via `set_current_file`), otherwise derives
+    /// it from the caller location. Derived per call so that benchmark functions living in
+    /// different files don't inherit a stale file from a previous `benchmark_group` call.
     #[track_caller]
-    fn fill_missing_file_from_caller(&mut self) {
+    fn codspeed_current_file(&self) -> String {
         if !self.current_file.is_empty() {
-            return;
+            return self.current_file.clone();
         }
         let caller = std::panic::Location::caller();
-        if let Ok(workspace_root) = std::env::var("CODSPEED_CARGO_WORKSPACE_ROOT") {
-            self.current_file = std::path::PathBuf::from(workspace_root)
+        match std::env::var("CODSPEED_CARGO_WORKSPACE_ROOT") {
+            Ok(workspace_root) => std::path::PathBuf::from(workspace_root)
                 .join(caller.file())
                 .to_string_lossy()
-                .into_owned();
-        } else {
-            self.current_file = caller.file().to_string();
+                .into_owned(),
+            Err(_) => caller.file().to_string(),
         }
     }
 }