From bfc9679acd83313c23cd00e222072250f2facb40 Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Tue, 27 Oct 2026 15:24:29 +0000 Subject: [PATCH 1/2] Accept `&str` in metrics arguments --- foundations-macros/src/metrics/mod.rs | 107 +++++++++++++++++++++- foundations-macros/src/metrics/parsing.rs | 26 +++++- foundations/src/telemetry/metrics/mod.rs | 6 +- foundations/tests/metrics.rs | 28 ++++++ 4 files changed, 159 insertions(+), 8 deletions(-) diff --git a/foundations-macros/src/metrics/mod.rs b/foundations-macros/src/metrics/mod.rs index 9d58095f..d6debc55 100644 --- a/foundations-macros/src/metrics/mod.rs +++ b/foundations-macros/src/metrics/mod.rs @@ -79,6 +79,7 @@ struct ArgAttrs { enum ArgMode { ByValue(Type), Clone(Type), + ToOwned(Type), Into(Type), } @@ -92,9 +93,8 @@ impl FnArg { } = self; let ty = match &self.mode { - ArgMode::ByValue(ty) => ty, - ArgMode::Clone(ty) => ty, - ArgMode::Into(ty) => ty, + ArgMode::ByValue(ty) | ArgMode::Clone(ty) | ArgMode::Into(ty) => quote! { #ty }, + ArgMode::ToOwned(ty) => quote! { <#ty as ::std::borrow::ToOwned>::Owned }, }; quote! { #serde_as #serde #ident #colon_token #ty } @@ -121,6 +121,9 @@ impl FnArg { ArgMode::Clone(_) => quote! { #ident #colon_token ::std::clone::Clone::clone(#ident) }, + ArgMode::ToOwned(_) => quote! { + #ident #colon_token ::std::borrow::ToOwned::to_owned(#ident) + }, ArgMode::Into(_) => quote! { #ident #colon_token ::std::convert::Into::into(#ident) }, @@ -287,7 +290,10 @@ fn label_set_struct(foundations: &Path, fn_: &ItemFn) -> Option ty, + ArgMode::ByValue(ty) + | ArgMode::Clone(ty) + | ArgMode::ToOwned(ty) + | ArgMode::Into(ty) => ty, }; quote! { @@ -305,7 +311,10 @@ fn label_set_struct(foundations: &Path, fn_: &ItemFn) -> Option ty, + ArgMode::ByValue(ty) + | ArgMode::Clone(ty) + | ArgMode::ToOwned(ty) + | ArgMode::Into(ty) => ty, }; quote! { @@ -900,6 +909,94 @@ mod tests { assert_eq!(actual, expected); } + #[test] + fn expand_str_label() { + let attr = parse_attr! { + #[metrics] + }; + + let src = parse_quote! { + pub mod oxy { + /// Total number of connection errors + pub fn connections_errors_total(endpoint: &str) -> Counter; + } + }; + + let actual = expand_from_parsed(attr, src).to_string(); + + let expected = code_str! { + pub mod oxy { + use super::*; + + #[allow(non_camel_case_types)] + struct __oxy_Metrics { + connections_errors_total: + ::foundations::telemetry::metrics::Family< + connections_errors_total, + Counter, + >, + } + + #[allow(non_camel_case_types)] + #[derive( + ::std::clone::Clone, + ::std::cmp::Eq, + ::std::hash::Hash, + ::std::cmp::PartialEq, + ::foundations::reexports_for_macros::serde::Serialize, + )] + #[serde(crate = ":: foundations :: reexports_for_macros :: serde")] + struct connections_errors_total { + endpoint: ::Owned, + } + + #[allow(non_upper_case_globals)] + static __oxy_Metrics: ::std::sync::LazyLock<__oxy_Metrics> = + ::std::sync::LazyLock::new(|| { + __oxy_Metrics { + connections_errors_total: { + let metric = ::std::default::Default::default(); + + ::foundations::telemetry::metrics::internal::register_metric( + ::std::stringify!(oxy), + ::std::stringify!(connections_errors_total), + ::std::concat!( + ::std::stringify!(oxy), + "_", + ::std::stringify!(connections_errors_total), + ), + str::trim(" Total number of connection errors"), + ::std::clone::Clone::clone(&metric), + false, + true, + ); + + metric + }, + } + }); + + #[doc = " Total number of connection errors"] + #[must_use] + pub fn connections_errors_total( + endpoint: &str, + ) -> Counter { + let __args = connections_errors_total { + endpoint: ::std::borrow::ToOwned::to_owned(endpoint), + }; + ::std::clone::Clone::clone( + &::foundations::telemetry::metrics::Family::get_or_create( + &__oxy_Metrics.connections_errors_total, + &__args, + ) + ) + } + } + }; + + assert_eq!(actual, expected); + } + #[test] fn expand_ctor() { let attr = parse_attr! { diff --git a/foundations-macros/src/metrics/parsing.rs b/foundations-macros/src/metrics/parsing.rs index 16a88a38..5c62901f 100644 --- a/foundations-macros/src/metrics/parsing.rs +++ b/foundations-macros/src/metrics/parsing.rs @@ -221,7 +221,8 @@ impl Parse for FnArg { let attrs = parse_attrs(input.call(Attribute::parse_outer)?)?; - /// If the type is a reference with an eluded lifetime, then the argument should be cloned. + /// If the type is a reference with an elided lifetime, then the argument should be cloned, + /// or converted with `ToOwned` if the pointee is `str` or a slice. /// If the type is `impl Into`, then the Into trait should be used. /// Otherwise, use the value directly. fn arg_mode(ty: &Type) -> Result { @@ -271,7 +272,11 @@ impl Parse for FnArg { Ok(match ty { Type::Reference(ref_) if ref_.lifetime.is_none() => { - ArgMode::Clone((*ref_.elem).clone()) + if is_unsized(&ref_.elem) { + ArgMode::ToOwned((*ref_.elem).clone()) + } else { + ArgMode::Clone((*ref_.elem).clone()) + } } Type::ImplTrait(impl_) => ArgMode::Into(as_into_target(impl_)?.clone()), ty => ArgMode::ByValue(ty.clone()), @@ -292,3 +297,20 @@ impl Parse for FnArg { }) } } + +fn is_unsized(ty: &Type) -> bool { + match ty { + Type::Paren(paren) => is_unsized(&paren.elem), + Type::Slice(_) => true, + Type::Path(path) + if path.qself.is_none() + && path.path.leading_colon.is_none() + && path.path.segments.len() == 1 => + { + let segment = &path.path.segments[0]; + + segment.ident == "str" && matches!(segment.arguments, PathArguments::None) + } + _ => false, + } +} diff --git a/foundations/src/telemetry/metrics/mod.rs b/foundations/src/telemetry/metrics/mod.rs index aea3b3f9..86005b54 100644 --- a/foundations/src/telemetry/metrics/mod.rs +++ b/foundations/src/telemetry/metrics/mod.rs @@ -301,6 +301,10 @@ fn report_nonfatal_collect_error(err: &dyn Display) { /// # Labels /// Arguments of the bodyless functions become labels for that metric. /// +/// References with an elided lifetime are converted to owned values: `&str` and `&[T]` with +/// [`ToOwned`], any other `&T` with [`Clone`]. `impl Into` arguments are converted with +/// [`Into`]. All other types, including `&'static` references, are stored as is. +/// /// For ordinary labeled metrics, the generated accessor returns an owned metric handle after /// looking up the label set in its [`Family`]. When the same labels are reused, call the accessor /// once and stash or cache that handle (cloning it when another owner needs a copy) instead of @@ -460,7 +464,7 @@ fn report_nonfatal_collect_error(err: &dyn Display) { /// pub mod my_app_metrics { /// /// Number of active client connections /// pub fn client_connections_active( -/// // Labels with an anonymous reference type will get cloned. +/// // Labels with an elided-lifetime reference are cloned (`&str` and `&[T]` use `ToOwned`). /// endpoint: &Arc, /// protocol: labels::L4Protocol, /// ip_version: labels::IpVersion, diff --git a/foundations/tests/metrics.rs b/foundations/tests/metrics.rs index e9b96b03..b13c2197 100644 --- a/foundations/tests/metrics.rs +++ b/foundations/tests/metrics.rs @@ -18,6 +18,11 @@ mod regular { pub fn dynamic(label: &'static str) -> Counter; } +#[metrics] +mod str_labels { + pub fn requests(path: &str) -> Counter; +} + #[metrics(unprefixed)] mod library { pub fn calls() -> Counter; @@ -64,6 +69,29 @@ fn metrics_unprefixed() { assert!(!metrics.contains("\nlibrary_optional")); } +#[test] +fn str_labels_are_owned_with_to_owned() { + let path = "/metrics".to_owned(); + + str_labels::requests(&path).inc_by(2); + str_labels::requests("/health").inc(); + + let settings = MetricsSettings { + service_name_format: ServiceNameFormat::MetricPrefix, + report_optional: false, + }; + let metrics = collect_text(&settings); + + assert!( + metrics.contains("undefined_str_labels_requests{path=\"/metrics\"} 2\n"), + "collected output was: {metrics}" + ); + assert!( + metrics.contains("undefined_str_labels_requests{path=\"/health\"} 1\n"), + "collected output was: {metrics}" + ); +} + #[test] fn encode_error_is_skipped() { encode_error::valid().inc(); From a77b01b290102223f3fcab108cb71d51189a27b0 Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Wed, 28 Oct 2026 17:00:57 +0000 Subject: [PATCH 2/2] ToOwned everything --- foundations-macros/src/metrics/mod.rs | 24 +++++++--------------- foundations-macros/src/metrics/parsing.rs | 25 ++--------------------- 2 files changed, 9 insertions(+), 40 deletions(-) diff --git a/foundations-macros/src/metrics/mod.rs b/foundations-macros/src/metrics/mod.rs index d6debc55..1a7d590e 100644 --- a/foundations-macros/src/metrics/mod.rs +++ b/foundations-macros/src/metrics/mod.rs @@ -78,7 +78,6 @@ struct ArgAttrs { enum ArgMode { ByValue(Type), - Clone(Type), ToOwned(Type), Into(Type), } @@ -93,7 +92,7 @@ impl FnArg { } = self; let ty = match &self.mode { - ArgMode::ByValue(ty) | ArgMode::Clone(ty) | ArgMode::Into(ty) => quote! { #ty }, + ArgMode::ByValue(ty) | ArgMode::Into(ty) => quote! { #ty }, ArgMode::ToOwned(ty) => quote! { <#ty as ::std::borrow::ToOwned>::Owned }, }; @@ -118,9 +117,6 @@ impl FnArg { match &self.mode { ArgMode::ByValue(_) => quote! { #ident }, - ArgMode::Clone(_) => quote! { - #ident #colon_token ::std::clone::Clone::clone(#ident) - }, ArgMode::ToOwned(_) => quote! { #ident #colon_token ::std::borrow::ToOwned::to_owned(#ident) }, @@ -290,10 +286,7 @@ fn label_set_struct(foundations: &Path, fn_: &ItemFn) -> Option ty, + ArgMode::ByValue(ty) | ArgMode::ToOwned(ty) | ArgMode::Into(ty) => ty, }; quote! { @@ -311,10 +304,7 @@ fn label_set_struct(foundations: &Path, fn_: &ItemFn) -> Option ty, + ArgMode::ByValue(ty) | ArgMode::ToOwned(ty) | ArgMode::Into(ty) => ty, }; quote! { @@ -849,7 +839,7 @@ mod tests { #[serde(crate = ":: foundations :: reexports_for_macros :: serde")] struct connections_errors_total { #[serde(rename = "endpoint_name")] - endpoint: Arc, + endpoint: as ::std::borrow::ToOwned>::Owned, #[serde_as(as = "DisplayFromStr")] kind: Kind, message: &'static str, @@ -891,7 +881,7 @@ mod tests { error: impl Into, ) -> Counter { let __args = connections_errors_total { - endpoint: ::std::clone::Clone::clone(endpoint), + endpoint: ::std::borrow::ToOwned::to_owned(endpoint), kind, message, error: ::std::convert::Into::into(error), @@ -1313,12 +1303,12 @@ mod tests { .expect("explicit-lifetime shared reference must parse"); let arg = &parsed.fns[0].args[0]; - assert!(matches!(&arg.mode, ArgMode::Clone(Type::Path(_)))); + assert!(matches!(&arg.mode, ArgMode::ToOwned(Type::Path(_)))); assert_eq!(arg.ty.to_token_stream().to_string(), "& 'static Kind"); assert_eq!(arg.to_struct_member().to_string(), "kind : Kind"); assert_eq!( arg.to_initializer().to_string(), - "kind : :: std :: clone :: Clone :: clone (kind)" + "kind : :: std :: clone :: ToOwned :: to_owned (kind)" ); } diff --git a/foundations-macros/src/metrics/parsing.rs b/foundations-macros/src/metrics/parsing.rs index 5c62901f..8d23f47c 100644 --- a/foundations-macros/src/metrics/parsing.rs +++ b/foundations-macros/src/metrics/parsing.rs @@ -174,7 +174,7 @@ impl Parse for ItemFn { return error(&reference, LOW_CARDINALITY_MUT_REFERENCE_ERROR); } - arg.mode = ArgMode::Clone((*reference.elem).clone()); + arg.mode = ArgMode::ToOwned((*reference.elem).clone()); } } } @@ -272,11 +272,7 @@ impl Parse for FnArg { Ok(match ty { Type::Reference(ref_) if ref_.lifetime.is_none() => { - if is_unsized(&ref_.elem) { - ArgMode::ToOwned((*ref_.elem).clone()) - } else { - ArgMode::Clone((*ref_.elem).clone()) - } + ArgMode::ToOwned((*ref_.elem).clone()) } Type::ImplTrait(impl_) => ArgMode::Into(as_into_target(impl_)?.clone()), ty => ArgMode::ByValue(ty.clone()), @@ -297,20 +293,3 @@ impl Parse for FnArg { }) } } - -fn is_unsized(ty: &Type) -> bool { - match ty { - Type::Paren(paren) => is_unsized(&paren.elem), - Type::Slice(_) => true, - Type::Path(path) - if path.qself.is_none() - && path.path.leading_colon.is_none() - && path.path.segments.len() == 1 => - { - let segment = &path.path.segments[0]; - - segment.ident == "str" && matches!(segment.arguments, PathArguments::None) - } - _ => false, - } -}