From 6ce2de01faf3d7ccaac9a68536d952fc95c5982b Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Wed, 12 Apr 2023 14:21:13 +0200 Subject: [PATCH 01/14] Allow specifying result labels for enum variants The target use case is when a service uses a single enumeration with all the possible errors, but some "errors" should not be counted as failures regarding metric collection (for example, all the 4xx errors in a HTTP handler usually mean that the server succeeded in rejecting a bad request). --- autometrics-macros/src/error_labels.rs | 156 +++++++++++++++++++++++++ autometrics-macros/src/lib.rs | 13 +++ autometrics/src/labels.rs | 22 +++- autometrics/src/lib.rs | 3 + 4 files changed, 192 insertions(+), 2 deletions(-) create mode 100644 autometrics-macros/src/error_labels.rs diff --git a/autometrics-macros/src/error_labels.rs b/autometrics-macros/src/error_labels.rs new file mode 100644 index 00000000..a25c93cb --- /dev/null +++ b/autometrics-macros/src/error_labels.rs @@ -0,0 +1,156 @@ +//! The definition of the ErrorLabels derive macro, that allows to specify +//! inside an enumeration whether variants should be considered as errors or +//! successes as far as the automatic metrics are concerned. +//! +//! For example, this would allow you to put all the client-side errors in a +//! HTTP webserver (4**) as successes, since it means the handler function +//! _successfully_ rejected a bad request, and that should not affect the SLO or +//! the success rate of the function in the metrics. +//! +//! ```rust,ignore +//! #[derive(ErrorLabels)] +//! enum ServiceError { +//! // By default, the variant will be labeled as an error, +//! // so you do not need to decorate every variant +//! Database, +//! // It is possible to mention it as well of course. +//! // Only "error" and "ok" are accepted values +//! #[label(result = "error")] +//! Network, +//! #[label(result = "ok")] +//! Authentication, +//! #[label(result = "ok")] +//! Authorization, +//! } +//! ``` + +use proc_macro2::TokenStream; +use quote::quote; +use syn::{ + parse_quote, punctuated::Punctuated, token::Comma, Attribute, Data, DataEnum, DeriveInput, + Error, Lit, LitStr, Result, Variant, +}; + +// These labels must match autometrics::ERROR_KEY and autometrics::OK_KEY, +// to avoid a dependency loop just for 2 constants we recreate these here. +const OK_KEY: &str = "ok"; +const ERROR_KEY: &str = "error"; +const RESULT_KEY: &str = "result"; +const ATTR_LABEL: &str = "label"; +const ACCEPTED_LABELS: [&str; 2] = [ERROR_KEY, OK_KEY]; + +/// Entry point of the ErrorLabels macro +pub(crate) fn expand(input: DeriveInput) -> Result { + let Data::Enum(DataEnum { + variants, + ..}) = &input.data else + { + return Err(Error::new_spanned( + input, + "ErrorLabels only works with 'Enum's.", + )) + }; + let enum_name = &input.ident; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + let match_clauses_for_labels = match_label_clauses(variants)?; + + Ok(quote! { + #[automatically_derived] + impl #impl_generics ::autometrics::__private::GetErrorLabelFromEnum for #enum_name #ty_generics #where_clause { + fn __autometrics_get_error_label(&self) -> &'static str { + match self { + #(#match_clauses_for_labels)* + } + } + } + }) +} + +/// Build the list of match clauses for the generated code. +fn match_label_clauses(variants: &Punctuated) -> Result> { + variants + .iter() + .map(|variant| { + let variant_name = &variant.ident; + if let Some(key) = extract_label_attribute(&variant.attrs)? { + Ok(quote! { + #variant_name => #key, + }) + } else { + Ok(quote! { + #variant_name => #ERROR_KEY, + }) + } + }) + .collect() +} + +/// Extract the wanted label from the annotation in the variant, if present. +/// The function looks for `#[label(result = "ok")]` kind of labels. +/// +/// ## Error cases +/// +/// The function will error out with the smallest possible span when: +/// +/// - The attribute on a variant is not a "list" type (so `#[label]` is not allowed), +/// - The key in the key value pair is not "result", as it's the only supported keyword +/// for now (so `#[label(non_existing_label = "ok")]` is not allowed), +/// - The value for the "result" label is not in the autometrics supported set (so +/// `#[label(result = "random label that will break queries")]` is not allowed) +fn extract_label_attribute(attrs: &[Attribute]) -> Result> { + attrs + .iter() + .find_map(|att| match att.parse_meta() { + Ok(meta) => match &meta { + syn::Meta::List(list) => { + // Ignore attribute if it's not `label(...)` + if list.path.segments.len() != 1 || list.path.segments[0].ident != ATTR_LABEL { + return None; + } + + // Only lists are allowed + let Some(syn::NestedMeta::Meta(syn::Meta::NameValue(pair))) = list.nested.first() else { + return Some(Err(Error::new_spanned( + meta, + format!("Only `{ATTR_LABEL}({RESULT_KEY} = \"RES\")` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"), + ))) + }; + + // Inside list, only 'result = ...' are allowed + if pair.path.segments.len() != 1 || pair.path.segments[0].ident != RESULT_KEY { + return Some(Err(Error::new_spanned( + pair, + format!("Only `{RESULT_KEY} = \"RES\"` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"), + ))); + } + + // Inside 'result = val', 'val' must be a string literal + let Lit::Str(ref lit_str) = pair.lit else { + return Some(Err(Error::new_spanned( + &pair.lit, + format!("Only {OK_KEY:?} or {ERROR_KEY:?}, as string literals, are accepted as result values"), + ))); + }; + + // Inside 'result = val', 'val' must be one of the allowed string literals + if !ACCEPTED_LABELS.contains(&lit_str.value().as_str()) { + return Some(Err(Error::new_spanned( + lit_str, + format!("Only {OK_KEY:?} or {ERROR_KEY:?} are accepted as result values"), + ))); + } + + Some(Ok(lit_str.clone())) + }, + _ => Some(Err(Error::new_spanned( + meta, + format!("Only `{ATTR_LABEL}({RESULT_KEY} = \"RES\")` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"), + ))), + }, + Err(e) => Some(Err(Error::new_spanned( + att, + format!("could not parse the meta attribute: {e}"), + ))), + }) + .transpose() +} diff --git a/autometrics-macros/src/lib.rs b/autometrics-macros/src/lib.rs index f2f6713a..9afaac87 100644 --- a/autometrics-macros/src/lib.rs +++ b/autometrics-macros/src/lib.rs @@ -5,6 +5,7 @@ use quote::quote; use std::env; use syn::{parse_macro_input, ImplItem, ItemFn, ItemImpl, Result}; +mod error_labels; mod parse; const COUNTER_NAME_PROMETHEUS: &str = "function_calls_count"; @@ -34,6 +35,18 @@ pub fn autometrics( output.into() } +// TODO: macro needs tests: +// - about generating code that actually compiles, and +// - about correct overriding of the result labels in the enums, and +// - about attribute validation +#[proc_macro_derive(ErrorLabels, attributes(label))] +pub fn error_labels(input: proc_macro::TokenStream) -> proc_macro::TokenStream { + let input = parse_macro_input!(input as syn::DeriveInput); + error_labels::expand(input) + .unwrap_or_else(syn::Error::into_compile_error) + .into() +} + /// Add autometrics instrumentation to a single function fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result { let sig = item.sig; diff --git a/autometrics/src/labels.rs b/autometrics/src/labels.rs index 1f6959be..3be7aec1 100644 --- a/autometrics/src/labels.rs +++ b/autometrics/src/labels.rs @@ -134,11 +134,14 @@ impl GetLabelsFromResult for Result { fn __autometrics_get_labels(&self) -> Option { match self { Ok(ok) => Some((OK_KEY, ok.__autometrics_static_str())), - Err(err) => Some((ERROR_KEY, err.__autometrics_static_str())), + // TODO: get from err whether it should be an error or not. + Err(err) => Some(( + err.__autometrics_get_error_label(), + err.__autometrics_static_str(), + )), } } } - pub enum LabelArray { Three([Label; 3]), Four([Label; 4]), @@ -221,3 +224,18 @@ pub trait GetStaticStr { } } impl_trait_for_types!(GetStaticStr); + +pub trait GetErrorLabel { + fn __autometrics_get_error_label(&self) -> &'static str { + ERROR_KEY + } +} +impl_trait_for_types!(GetErrorLabel); + +pub trait GetErrorLabelFromEnum { + fn __autometrics_get_error_label(&self) -> &'static str { + ERROR_KEY + } +} + +// TODO: add an attribute macro that will derive GetErrorLabelFromEnum for the decorated enum. diff --git a/autometrics/src/lib.rs b/autometrics/src/lib.rs index d786b8da..16bc8fd2 100644 --- a/autometrics/src/lib.rs +++ b/autometrics/src/lib.rs @@ -134,6 +134,9 @@ mod tracker; /// pub use autometrics_macros::autometrics; +// TODO: write documentation +pub use autometrics_macros::ErrorLabels; + // Optional exports #[cfg(feature = "prometheus-exporter")] pub use self::prometheus_exporter::*; From 2f913a66d260db51088f082f69941309e318843b Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Wed, 12 Apr 2023 14:50:47 +0200 Subject: [PATCH 02/14] Add variant matchers to the calls --- autometrics-macros/src/error_labels.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/autometrics-macros/src/error_labels.rs b/autometrics-macros/src/error_labels.rs index a25c93cb..e36aa4c1 100644 --- a/autometrics-macros/src/error_labels.rs +++ b/autometrics-macros/src/error_labels.rs @@ -72,13 +72,18 @@ fn match_label_clauses(variants: &Punctuated) -> Result = match variant.fields { + syn::Fields::Named(_) => Some(quote! { (_) }), + syn::Fields::Unnamed(_) => Some(quote! { (_) }), + syn::Fields::Unit => None, + }; if let Some(key) = extract_label_attribute(&variant.attrs)? { Ok(quote! { - #variant_name => #key, + #variant_name #variant_matcher => #key, }) } else { Ok(quote! { - #variant_name => #ERROR_KEY, + #variant_name #variant_matcher => #ERROR_KEY, }) } }) From 3352b7c823faaae1843fa81e17adb303f12fbcf9 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Wed, 12 Apr 2023 17:18:40 +0200 Subject: [PATCH 03/14] Add tests for ErrorLabels using TryBuild This allowed to discover a bug in the code generated, which has also been fixed in the commit. --- autometrics-macros/.gitignore | 1 + autometrics-macros/Cargo.toml | 4 ++ autometrics-macros/src/error_labels.rs | 37 ++++++++++--------- autometrics-macros/tests/error_labels.rs | 8 ++++ .../error_labels/fail/wrong_attribute.rs | 15 ++++++++ .../error_labels/fail/wrong_attribute.stderr | 5 +++ .../error_labels/fail/wrong_kv_attribute.rs | 15 ++++++++ .../fail/wrong_kv_attribute.stderr | 5 +++ .../error_labels/fail/wrong_result_name.rs | 15 ++++++++ .../fail/wrong_result_name.stderr | 5 +++ .../tests/error_labels/fail/wrong_variant.rs | 15 ++++++++ .../error_labels/fail/wrong_variant.stderr | 5 +++ .../tests/error_labels/pass/macro.rs | 32 ++++++++++++++++ autometrics/src/labels.rs | 2 - 14 files changed, 145 insertions(+), 19 deletions(-) create mode 100644 autometrics-macros/.gitignore create mode 100644 autometrics-macros/tests/error_labels.rs create mode 100644 autometrics-macros/tests/error_labels/fail/wrong_attribute.rs create mode 100644 autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr create mode 100644 autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs create mode 100644 autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr create mode 100644 autometrics-macros/tests/error_labels/fail/wrong_result_name.rs create mode 100644 autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr create mode 100644 autometrics-macros/tests/error_labels/fail/wrong_variant.rs create mode 100644 autometrics-macros/tests/error_labels/fail/wrong_variant.stderr create mode 100644 autometrics-macros/tests/error_labels/pass/macro.rs diff --git a/autometrics-macros/.gitignore b/autometrics-macros/.gitignore new file mode 100644 index 00000000..e7123041 --- /dev/null +++ b/autometrics-macros/.gitignore @@ -0,0 +1 @@ +wip diff --git a/autometrics-macros/Cargo.toml b/autometrics-macros/Cargo.toml index 881163be..c186005c 100644 --- a/autometrics-macros/Cargo.toml +++ b/autometrics-macros/Cargo.toml @@ -18,3 +18,7 @@ percent-encoding = "2.2" proc-macro2 = "1" quote = "1" syn = { version = "1", features = ["full"] } + +[dev-dependencies] +trybuild = "1.0" +autometrics = { path = "../autometrics" } diff --git a/autometrics-macros/src/error_labels.rs b/autometrics-macros/src/error_labels.rs index e36aa4c1..bb4bffec 100644 --- a/autometrics-macros/src/error_labels.rs +++ b/autometrics-macros/src/error_labels.rs @@ -27,8 +27,8 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{ - parse_quote, punctuated::Punctuated, token::Comma, Attribute, Data, DataEnum, DeriveInput, - Error, Lit, LitStr, Result, Variant, + punctuated::Punctuated, token::Comma, Attribute, Data, DataEnum, DeriveInput, Error, Ident, + Lit, LitStr, Result, Variant, }; // These labels must match autometrics::ERROR_KEY and autometrics::OK_KEY, @@ -52,39 +52,42 @@ pub(crate) fn expand(input: DeriveInput) -> Result { }; let enum_name = &input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - let match_clauses_for_labels = match_label_clauses(variants)?; + let conditional_clauses_for_labels = conditional_label_clauses(variants, enum_name)?; Ok(quote! { #[automatically_derived] impl #impl_generics ::autometrics::__private::GetErrorLabelFromEnum for #enum_name #ty_generics #where_clause { fn __autometrics_get_error_label(&self) -> &'static str { - match self { - #(#match_clauses_for_labels)* - } + #(#conditional_clauses_for_labels)* + #ERROR_KEY } } }) } /// Build the list of match clauses for the generated code. -fn match_label_clauses(variants: &Punctuated) -> Result> { +fn conditional_label_clauses( + variants: &Punctuated, + enum_name: &Ident, +) -> Result> { variants .iter() .map(|variant| { let variant_name = &variant.ident; - let variant_matcher: Option = match variant.fields { - syn::Fields::Named(_) => Some(quote! { (_) }), - syn::Fields::Unnamed(_) => Some(quote! { (_) }), - syn::Fields::Unit => None, + let variant_matcher: TokenStream = match variant.fields { + syn::Fields::Named(_) => quote! { #variant_name {..} }, + syn::Fields::Unnamed(_) => quote! { #variant_name (_) }, + syn::Fields::Unit => quote! { #variant_name }, }; if let Some(key) = extract_label_attribute(&variant.attrs)? { - Ok(quote! { - #variant_name #variant_matcher => #key, - }) + Ok(quote! [ + if ::std::matches!(self, & #enum_name :: #variant_matcher) { + return #key + } + ]) } else { - Ok(quote! { - #variant_name #variant_matcher => #ERROR_KEY, - }) + // Let the code flow through the last value + Ok(quote! {}) } }) .collect() diff --git a/autometrics-macros/tests/error_labels.rs b/autometrics-macros/tests/error_labels.rs new file mode 100644 index 00000000..882bb8b6 --- /dev/null +++ b/autometrics-macros/tests/error_labels.rs @@ -0,0 +1,8 @@ +//! Tests for the ErrorLabels macro + +#[test] +fn harness() { + let t = trybuild::TestCases::new(); + t.pass("tests/error_labels/pass/*.rs"); + t.compile_fail("tests/error_labels/fail/*.rs") +} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_attribute.rs b/autometrics-macros/tests/error_labels/fail/wrong_attribute.rs new file mode 100644 index 00000000..8373dd30 --- /dev/null +++ b/autometrics-macros/tests/error_labels/fail/wrong_attribute.rs @@ -0,0 +1,15 @@ +use autometrics_macros::ErrorLabels; + +struct Inner {} + +#[derive(ErrorLabels)] +enum MyError { + Empty, + #[label] + ClientError { + inner: Inner, + }, + ServerError(u64), +} + +fn main() {} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr b/autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr new file mode 100644 index 00000000..4115bb59 --- /dev/null +++ b/autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr @@ -0,0 +1,5 @@ +error: Only `label(result = "RES")` (RES can be "ok" or "error") is supported + --> tests/error_labels/fail/wrong_attribute.rs:8:7 + | +8 | #[label] + | ^^^^^ diff --git a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs b/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs new file mode 100644 index 00000000..a1b2fd2d --- /dev/null +++ b/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs @@ -0,0 +1,15 @@ +use autometrics_macros::ErrorLabels; + +struct Inner {} + +#[derive(ErrorLabels)] +enum MyError { + Empty, + #[label = "error"] + ClientError { + inner: Inner, + }, + ServerError(u64), +} + +fn main() {} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr b/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr new file mode 100644 index 00000000..a03beca6 --- /dev/null +++ b/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr @@ -0,0 +1,5 @@ +error: Only `label(result = "RES")` (RES can be "ok" or "error") is supported + --> tests/error_labels/fail/wrong_kv_attribute.rs:8:7 + | +8 | #[label = "error"] + | ^^^^^^^^^^^^^^^ diff --git a/autometrics-macros/tests/error_labels/fail/wrong_result_name.rs b/autometrics-macros/tests/error_labels/fail/wrong_result_name.rs new file mode 100644 index 00000000..8d52fe2f --- /dev/null +++ b/autometrics-macros/tests/error_labels/fail/wrong_result_name.rs @@ -0,0 +1,15 @@ +use autometrics_macros::ErrorLabels; + +struct Inner {} + +#[derive(ErrorLabels)] +enum MyError { + Empty, + #[label(unknown = "ok")] + ClientError { + inner: Inner, + }, + ServerError(u64), +} + +fn main() {} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr b/autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr new file mode 100644 index 00000000..45c7571d --- /dev/null +++ b/autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr @@ -0,0 +1,5 @@ +error: Only `result = "RES"` (RES can be "ok" or "error") is supported + --> tests/error_labels/fail/wrong_result_name.rs:8:13 + | +8 | #[label(unknown = "ok")] + | ^^^^^^^^^^^^^^ diff --git a/autometrics-macros/tests/error_labels/fail/wrong_variant.rs b/autometrics-macros/tests/error_labels/fail/wrong_variant.rs new file mode 100644 index 00000000..2f97eede --- /dev/null +++ b/autometrics-macros/tests/error_labels/fail/wrong_variant.rs @@ -0,0 +1,15 @@ +use autometrics_macros::ErrorLabels; + +struct Inner {} + +#[derive(ErrorLabels)] +enum MyError { + Empty, + #[label(result = "not ok")] + ClientError { + inner: Inner, + }, + ServerError(u64), +} + +fn main() {} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_variant.stderr b/autometrics-macros/tests/error_labels/fail/wrong_variant.stderr new file mode 100644 index 00000000..fd45ea60 --- /dev/null +++ b/autometrics-macros/tests/error_labels/fail/wrong_variant.stderr @@ -0,0 +1,5 @@ +error: Only "ok" or "error" are accepted as result values + --> tests/error_labels/fail/wrong_variant.rs:8:22 + | +8 | #[label(result = "not ok")] + | ^^^^^^^^ diff --git a/autometrics-macros/tests/error_labels/pass/macro.rs b/autometrics-macros/tests/error_labels/pass/macro.rs new file mode 100644 index 00000000..5518dfba --- /dev/null +++ b/autometrics-macros/tests/error_labels/pass/macro.rs @@ -0,0 +1,32 @@ +//! This test uses interfaces not meant to be directly used. +//! +//! The goal here is to make sure that the macro has the effect we want. +//! autometrics (the library) is then responsible for orchestrating the +//! calls to `__autometrics_get_error_label` correctly when observing +//! function call results for the metrics. +use autometrics::__private::GetErrorLabelFromEnum; +use autometrics_macros::ErrorLabels; + +struct Inner {} + +#[derive(ErrorLabels)] +enum MyError { + #[label(result = "error")] + Empty, + #[label(result = "ok")] + ClientError { + inner: Inner, + }, + ServerError(u64), +} + +fn main() { + let err = MyError::ClientError { inner: Inner {} }; + assert_eq!(err.__autometrics_get_error_label(), "ok"); + + let err = MyError::Empty; + assert_eq!(err.__autometrics_get_error_label(), "error"); + + let err = MyError::ServerError(502); + assert_eq!(err.__autometrics_get_error_label(), "error"); +} diff --git a/autometrics/src/labels.rs b/autometrics/src/labels.rs index 3be7aec1..29185e92 100644 --- a/autometrics/src/labels.rs +++ b/autometrics/src/labels.rs @@ -237,5 +237,3 @@ pub trait GetErrorLabelFromEnum { ERROR_KEY } } - -// TODO: add an attribute macro that will derive GetErrorLabelFromEnum for the decorated enum. From 1a199d86041994fa17db9ee27e717e066ea08c43 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Thu, 13 Apr 2023 17:08:51 +0200 Subject: [PATCH 04/14] Add documentation and update tests --- autometrics-macros/src/error_labels.rs | 2 +- autometrics-macros/src/lib.rs | 4 -- .../error_labels/fail/wrong_attribute.rs | 3 ++ .../error_labels/fail/wrong_attribute.stderr | 8 ++-- .../error_labels/fail/wrong_kv_attribute.rs | 3 ++ .../fail/wrong_kv_attribute.stderr | 8 ++-- .../error_labels/fail/wrong_result_name.rs | 3 ++ .../fail/wrong_result_name.stderr | 8 ++-- .../tests/error_labels/fail/wrong_variant.rs | 4 ++ .../error_labels/fail/wrong_variant.stderr | 8 ++-- autometrics/src/labels.rs | 7 +++- autometrics/src/lib.rs | 39 ++++++++++++++++++- 12 files changed, 74 insertions(+), 23 deletions(-) diff --git a/autometrics-macros/src/error_labels.rs b/autometrics-macros/src/error_labels.rs index bb4bffec..6c29ff29 100644 --- a/autometrics-macros/src/error_labels.rs +++ b/autometrics-macros/src/error_labels.rs @@ -127,7 +127,7 @@ fn extract_label_attribute(attrs: &[Attribute]) -> Result> { // Inside list, only 'result = ...' are allowed if pair.path.segments.len() != 1 || pair.path.segments[0].ident != RESULT_KEY { return Some(Err(Error::new_spanned( - pair, + pair.path.clone(), format!("Only `{RESULT_KEY} = \"RES\"` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"), ))); } diff --git a/autometrics-macros/src/lib.rs b/autometrics-macros/src/lib.rs index 9afaac87..2938281f 100644 --- a/autometrics-macros/src/lib.rs +++ b/autometrics-macros/src/lib.rs @@ -35,10 +35,6 @@ pub fn autometrics( output.into() } -// TODO: macro needs tests: -// - about generating code that actually compiles, and -// - about correct overriding of the result labels in the enums, and -// - about attribute validation #[proc_macro_derive(ErrorLabels, attributes(label))] pub fn error_labels(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input = parse_macro_input!(input as syn::DeriveInput); diff --git a/autometrics-macros/tests/error_labels/fail/wrong_attribute.rs b/autometrics-macros/tests/error_labels/fail/wrong_attribute.rs index 8373dd30..388daa92 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_attribute.rs +++ b/autometrics-macros/tests/error_labels/fail/wrong_attribute.rs @@ -1,3 +1,6 @@ +// This test ensures that the macro fails with a readable +// error when the attribute given to one variant inside the +// enumeration is not in the correct form. use autometrics_macros::ErrorLabels; struct Inner {} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr b/autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr index 4115bb59..17a96a43 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr +++ b/autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr @@ -1,5 +1,5 @@ error: Only `label(result = "RES")` (RES can be "ok" or "error") is supported - --> tests/error_labels/fail/wrong_attribute.rs:8:7 - | -8 | #[label] - | ^^^^^ + --> tests/error_labels/fail/wrong_attribute.rs:11:7 + | +11 | #[label] + | ^^^^^ diff --git a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs b/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs index a1b2fd2d..a465cc87 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs +++ b/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs @@ -1,3 +1,6 @@ +// This test ensures that the macro fails with a readable +// error when the attribute given to one variant inside the +// enumeration is not in the correct form. use autometrics_macros::ErrorLabels; struct Inner {} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr b/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr index a03beca6..8caeda21 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr +++ b/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr @@ -1,5 +1,5 @@ error: Only `label(result = "RES")` (RES can be "ok" or "error") is supported - --> tests/error_labels/fail/wrong_kv_attribute.rs:8:7 - | -8 | #[label = "error"] - | ^^^^^^^^^^^^^^^ + --> tests/error_labels/fail/wrong_kv_attribute.rs:11:7 + | +11 | #[label = "error"] + | ^^^^^^^^^^^^^^^ diff --git a/autometrics-macros/tests/error_labels/fail/wrong_result_name.rs b/autometrics-macros/tests/error_labels/fail/wrong_result_name.rs index 8d52fe2f..745b2358 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_result_name.rs +++ b/autometrics-macros/tests/error_labels/fail/wrong_result_name.rs @@ -1,3 +1,6 @@ +// This test ensures that the macro fails with a readable +// error when the attribute given to one variant inside the +// enumeration does not use the correct key for the label. use autometrics_macros::ErrorLabels; struct Inner {} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr b/autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr index 45c7571d..dd6c9f0b 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr +++ b/autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr @@ -1,5 +1,5 @@ error: Only `result = "RES"` (RES can be "ok" or "error") is supported - --> tests/error_labels/fail/wrong_result_name.rs:8:13 - | -8 | #[label(unknown = "ok")] - | ^^^^^^^^^^^^^^ + --> tests/error_labels/fail/wrong_result_name.rs:11:13 + | +11 | #[label(unknown = "ok")] + | ^^^^^^^ diff --git a/autometrics-macros/tests/error_labels/fail/wrong_variant.rs b/autometrics-macros/tests/error_labels/fail/wrong_variant.rs index 2f97eede..7e60d753 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_variant.rs +++ b/autometrics-macros/tests/error_labels/fail/wrong_variant.rs @@ -1,3 +1,7 @@ +// This test ensures that the macro fails with a readable error when the +// attribute given to one variant inside the enumeration does not use one of the +// predetermined values (that would make the automatic queries fail, so the +// macros need to forbid wrong usage at compile time) use autometrics_macros::ErrorLabels; struct Inner {} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_variant.stderr b/autometrics-macros/tests/error_labels/fail/wrong_variant.stderr index fd45ea60..13066bef 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_variant.stderr +++ b/autometrics-macros/tests/error_labels/fail/wrong_variant.stderr @@ -1,5 +1,5 @@ error: Only "ok" or "error" are accepted as result values - --> tests/error_labels/fail/wrong_variant.rs:8:22 - | -8 | #[label(result = "not ok")] - | ^^^^^^^^ + --> tests/error_labels/fail/wrong_variant.rs:12:22 + | +12 | #[label(result = "not ok")] + | ^^^^^^^^ diff --git a/autometrics/src/labels.rs b/autometrics/src/labels.rs index 29185e92..41955ed2 100644 --- a/autometrics/src/labels.rs +++ b/autometrics/src/labels.rs @@ -134,7 +134,6 @@ impl GetLabelsFromResult for Result { fn __autometrics_get_labels(&self) -> Option { match self { Ok(ok) => Some((OK_KEY, ok.__autometrics_static_str())), - // TODO: get from err whether it should be an error or not. Err(err) => Some(( err.__autometrics_get_error_label(), err.__autometrics_static_str(), @@ -225,14 +224,20 @@ pub trait GetStaticStr { } impl_trait_for_types!(GetStaticStr); +/// Implementation detail to get enum variants to specify their own +/// "result" label pub trait GetErrorLabel { + /// Return the value to use for the [result](RESULT_KEY) value in the reported metrics fn __autometrics_get_error_label(&self) -> &'static str { ERROR_KEY } } impl_trait_for_types!(GetErrorLabel); +/// Implementation detail to get enum variants to specify their own +/// "result" label pub trait GetErrorLabelFromEnum { + /// Return the value to use for the [result](RESULT_KEY) value in the reported metrics fn __autometrics_get_error_label(&self) -> &'static str { ERROR_KEY } diff --git a/autometrics/src/lib.rs b/autometrics/src/lib.rs index 16bc8fd2..23b2cd79 100644 --- a/autometrics/src/lib.rs +++ b/autometrics/src/lib.rs @@ -134,7 +134,44 @@ mod tracker; /// pub use autometrics_macros::autometrics; -// TODO: write documentation +/// # Autometrics custom error labelling +/// +/// The ErrorLabels derive macro allows to specify +/// inside an enumeration whether variants should be considered as errors or +/// successes as far as the [automatic metrics](autometrics) are concerned. +/// +/// For example, this would allow you to put all the client-side errors in a +/// HTTP webserver (4**) as successes, since it means the handler function +/// _successfully_ rejected a bad request, and that should not affect the SLO or +/// the success rate of the function in the metrics. +/// +/// Putting such a policy in place would look like this in code: +/// +/// ```rust,ignore +/// use autometrics::ErrorLabels +/// +/// #[derive(ErrorLabels)] +/// pub enum ServiceError { +/// // By default, the variant will be labeled as an error, +/// // so you do not need to decorate every variant +/// Database, +/// // It is possible to mention it as well of course. +/// // Only "error" and "ok" are accepted values +/// #[label(result = "error")] +/// Network, +/// #[label(result = "ok")] +/// Authentication, +/// #[label(result = "ok")] +/// Authorization, +/// } +/// +/// pub type ServiceResult = Result; +/// ``` +/// +/// With these types, whenever a function returns a `ServiceResult`, having a +/// `ServiceError::Authentication` or `Authorization` would _not_ count as a +/// failure from your handler that should trigger alerts and consume the "error +/// budget" of the service. pub use autometrics_macros::ErrorLabels; // Optional exports From e3381ce40490dd08966aaa465f1a0ba8a5d9e399 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Fri, 28 Apr 2023 12:57:50 +0200 Subject: [PATCH 05/14] Review: rename error_labels to result_labels --- .gitignore | 3 +++ autometrics-macros/.gitignore | 1 - autometrics-macros/Cargo.toml | 4 ---- autometrics-macros/src/lib.rs | 8 ++++---- .../src/{error_labels.rs => result_labels.rs} | 12 ++++++------ autometrics-macros/tests/error_labels.rs | 8 -------- autometrics/Cargo.toml | 1 + autometrics/src/labels.rs | 12 ++++++------ autometrics/src/lib.rs | 8 ++++---- autometrics/tests/result_labels.rs | 8 ++++++++ .../tests/result_labels}/fail/wrong_attribute.rs | 4 ++-- .../tests/result_labels}/fail/wrong_attribute.stderr | 2 +- .../tests/result_labels}/fail/wrong_kv_attribute.rs | 4 ++-- .../result_labels}/fail/wrong_kv_attribute.stderr | 2 +- .../tests/result_labels}/fail/wrong_result_name.rs | 4 ++-- .../result_labels}/fail/wrong_result_name.stderr | 2 +- .../tests/result_labels}/fail/wrong_variant.rs | 4 ++-- .../tests/result_labels}/fail/wrong_variant.stderr | 2 +- .../tests/result_labels}/pass/macro.rs | 12 ++++++------ 19 files changed, 50 insertions(+), 51 deletions(-) delete mode 100644 autometrics-macros/.gitignore rename autometrics-macros/src/{error_labels.rs => result_labels.rs} (94%) delete mode 100644 autometrics-macros/tests/error_labels.rs create mode 100644 autometrics/tests/result_labels.rs rename {autometrics-macros/tests/error_labels => autometrics/tests/result_labels}/fail/wrong_attribute.rs (83%) rename {autometrics-macros/tests/error_labels => autometrics/tests/result_labels}/fail/wrong_attribute.stderr (68%) rename {autometrics-macros/tests/error_labels => autometrics/tests/result_labels}/fail/wrong_kv_attribute.rs (83%) rename {autometrics-macros/tests/error_labels => autometrics/tests/result_labels}/fail/wrong_kv_attribute.stderr (70%) rename {autometrics-macros/tests/error_labels => autometrics/tests/result_labels}/fail/wrong_result_name.rs (84%) rename {autometrics-macros/tests/error_labels => autometrics/tests/result_labels}/fail/wrong_result_name.stderr (69%) rename {autometrics-macros/tests/error_labels => autometrics/tests/result_labels}/fail/wrong_variant.rs (87%) rename {autometrics-macros/tests/error_labels => autometrics/tests/result_labels}/fail/wrong_variant.stderr (71%) rename {autometrics-macros/tests/error_labels => autometrics/tests/result_labels}/pass/macro.rs (68%) diff --git a/.gitignore b/.gitignore index 16411f16..bf68eb3d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ target Cargo.lock data + +# Trybuild compilation error outputs in development phase +wip diff --git a/autometrics-macros/.gitignore b/autometrics-macros/.gitignore deleted file mode 100644 index e7123041..00000000 --- a/autometrics-macros/.gitignore +++ /dev/null @@ -1 +0,0 @@ -wip diff --git a/autometrics-macros/Cargo.toml b/autometrics-macros/Cargo.toml index c186005c..881163be 100644 --- a/autometrics-macros/Cargo.toml +++ b/autometrics-macros/Cargo.toml @@ -18,7 +18,3 @@ percent-encoding = "2.2" proc-macro2 = "1" quote = "1" syn = { version = "1", features = ["full"] } - -[dev-dependencies] -trybuild = "1.0" -autometrics = { path = "../autometrics" } diff --git a/autometrics-macros/src/lib.rs b/autometrics-macros/src/lib.rs index 2938281f..20604d2c 100644 --- a/autometrics-macros/src/lib.rs +++ b/autometrics-macros/src/lib.rs @@ -5,7 +5,7 @@ use quote::quote; use std::env; use syn::{parse_macro_input, ImplItem, ItemFn, ItemImpl, Result}; -mod error_labels; +mod result_labels; mod parse; const COUNTER_NAME_PROMETHEUS: &str = "function_calls_count"; @@ -35,10 +35,10 @@ pub fn autometrics( output.into() } -#[proc_macro_derive(ErrorLabels, attributes(label))] -pub fn error_labels(input: proc_macro::TokenStream) -> proc_macro::TokenStream { +#[proc_macro_derive(ResultLabels, attributes(label))] +pub fn result_labels(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input = parse_macro_input!(input as syn::DeriveInput); - error_labels::expand(input) + result_labels::expand(input) .unwrap_or_else(syn::Error::into_compile_error) .into() } diff --git a/autometrics-macros/src/error_labels.rs b/autometrics-macros/src/result_labels.rs similarity index 94% rename from autometrics-macros/src/error_labels.rs rename to autometrics-macros/src/result_labels.rs index 6c29ff29..91238481 100644 --- a/autometrics-macros/src/error_labels.rs +++ b/autometrics-macros/src/result_labels.rs @@ -1,4 +1,4 @@ -//! The definition of the ErrorLabels derive macro, that allows to specify +//! The definition of the ResultLabels derive macro, that allows to specify //! inside an enumeration whether variants should be considered as errors or //! successes as far as the automatic metrics are concerned. //! @@ -8,7 +8,7 @@ //! the success rate of the function in the metrics. //! //! ```rust,ignore -//! #[derive(ErrorLabels)] +//! #[derive(ResultLabels)] //! enum ServiceError { //! // By default, the variant will be labeled as an error, //! // so you do not need to decorate every variant @@ -39,7 +39,7 @@ const RESULT_KEY: &str = "result"; const ATTR_LABEL: &str = "label"; const ACCEPTED_LABELS: [&str; 2] = [ERROR_KEY, OK_KEY]; -/// Entry point of the ErrorLabels macro +/// Entry point of the ResultLabels macro pub(crate) fn expand(input: DeriveInput) -> Result { let Data::Enum(DataEnum { variants, @@ -47,7 +47,7 @@ pub(crate) fn expand(input: DeriveInput) -> Result { { return Err(Error::new_spanned( input, - "ErrorLabels only works with 'Enum's.", + "ResultLabels only works with 'Enum's.", )) }; let enum_name = &input.ident; @@ -56,8 +56,8 @@ pub(crate) fn expand(input: DeriveInput) -> Result { Ok(quote! { #[automatically_derived] - impl #impl_generics ::autometrics::__private::GetErrorLabelFromEnum for #enum_name #ty_generics #where_clause { - fn __autometrics_get_error_label(&self) -> &'static str { + impl #impl_generics ::autometrics::__private::GetResultLabelFromEnum for #enum_name #ty_generics #where_clause { + fn __autometrics_get_result_label(&self) -> &'static str { #(#conditional_clauses_for_labels)* #ERROR_KEY } diff --git a/autometrics-macros/tests/error_labels.rs b/autometrics-macros/tests/error_labels.rs deleted file mode 100644 index 882bb8b6..00000000 --- a/autometrics-macros/tests/error_labels.rs +++ /dev/null @@ -1,8 +0,0 @@ -//! Tests for the ErrorLabels macro - -#[test] -fn harness() { - let t = trybuild::TestCases::new(); - t.pass("tests/error_labels/pass/*.rs"); - t.compile_fail("tests/error_labels/fail/*.rs") -} diff --git a/autometrics/Cargo.toml b/autometrics/Cargo.toml index 237b1641..61472169 100644 --- a/autometrics/Cargo.toml +++ b/autometrics/Cargo.toml @@ -48,6 +48,7 @@ const_format = { version = "0.2", features = ["rust_1_51"], optional = true } [dev-dependencies] regex = "1.7" http = "0.2" +trybuild = "1.0" [package.metadata.docs.rs] all-features = true diff --git a/autometrics/src/labels.rs b/autometrics/src/labels.rs index 41955ed2..32a43854 100644 --- a/autometrics/src/labels.rs +++ b/autometrics/src/labels.rs @@ -135,7 +135,7 @@ impl GetLabelsFromResult for Result { match self { Ok(ok) => Some((OK_KEY, ok.__autometrics_static_str())), Err(err) => Some(( - err.__autometrics_get_error_label(), + err.__autometrics_get_result_label(), err.__autometrics_static_str(), )), } @@ -226,19 +226,19 @@ impl_trait_for_types!(GetStaticStr); /// Implementation detail to get enum variants to specify their own /// "result" label -pub trait GetErrorLabel { +pub trait GetResultLabel { /// Return the value to use for the [result](RESULT_KEY) value in the reported metrics - fn __autometrics_get_error_label(&self) -> &'static str { + fn __autometrics_get_result_label(&self) -> &'static str { ERROR_KEY } } -impl_trait_for_types!(GetErrorLabel); +impl_trait_for_types!(GetResultLabel); /// Implementation detail to get enum variants to specify their own /// "result" label -pub trait GetErrorLabelFromEnum { +pub trait GetResultLabelFromEnum { /// Return the value to use for the [result](RESULT_KEY) value in the reported metrics - fn __autometrics_get_error_label(&self) -> &'static str { + fn __autometrics_get_result_label(&self) -> &'static str { ERROR_KEY } } diff --git a/autometrics/src/lib.rs b/autometrics/src/lib.rs index 23b2cd79..6c260952 100644 --- a/autometrics/src/lib.rs +++ b/autometrics/src/lib.rs @@ -136,7 +136,7 @@ pub use autometrics_macros::autometrics; /// # Autometrics custom error labelling /// -/// The ErrorLabels derive macro allows to specify +/// The ResultLabels derive macro allows to specify /// inside an enumeration whether variants should be considered as errors or /// successes as far as the [automatic metrics](autometrics) are concerned. /// @@ -148,9 +148,9 @@ pub use autometrics_macros::autometrics; /// Putting such a policy in place would look like this in code: /// /// ```rust,ignore -/// use autometrics::ErrorLabels +/// use autometrics::ResultLabels /// -/// #[derive(ErrorLabels)] +/// #[derive(ResultLabels)] /// pub enum ServiceError { /// // By default, the variant will be labeled as an error, /// // so you do not need to decorate every variant @@ -172,7 +172,7 @@ pub use autometrics_macros::autometrics; /// `ServiceError::Authentication` or `Authorization` would _not_ count as a /// failure from your handler that should trigger alerts and consume the "error /// budget" of the service. -pub use autometrics_macros::ErrorLabels; +pub use autometrics_macros::ResultLabels; // Optional exports #[cfg(feature = "prometheus-exporter")] diff --git a/autometrics/tests/result_labels.rs b/autometrics/tests/result_labels.rs new file mode 100644 index 00000000..6e0818e1 --- /dev/null +++ b/autometrics/tests/result_labels.rs @@ -0,0 +1,8 @@ +//! Tests for the ResultLabels macro + +#[test] +fn harness() { + let t = trybuild::TestCases::new(); + t.pass("tests/result_labels/pass/*.rs"); + t.compile_fail("tests/result_labels/fail/*.rs") +} diff --git a/autometrics-macros/tests/error_labels/fail/wrong_attribute.rs b/autometrics/tests/result_labels/fail/wrong_attribute.rs similarity index 83% rename from autometrics-macros/tests/error_labels/fail/wrong_attribute.rs rename to autometrics/tests/result_labels/fail/wrong_attribute.rs index 388daa92..3724202a 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_attribute.rs +++ b/autometrics/tests/result_labels/fail/wrong_attribute.rs @@ -1,11 +1,11 @@ // This test ensures that the macro fails with a readable // error when the attribute given to one variant inside the // enumeration is not in the correct form. -use autometrics_macros::ErrorLabels; +use autometrics_macros::ResultLabels; struct Inner {} -#[derive(ErrorLabels)] +#[derive(ResultLabels)] enum MyError { Empty, #[label] diff --git a/autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr b/autometrics/tests/result_labels/fail/wrong_attribute.stderr similarity index 68% rename from autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr rename to autometrics/tests/result_labels/fail/wrong_attribute.stderr index 17a96a43..482b5a3b 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_attribute.stderr +++ b/autometrics/tests/result_labels/fail/wrong_attribute.stderr @@ -1,5 +1,5 @@ error: Only `label(result = "RES")` (RES can be "ok" or "error") is supported - --> tests/error_labels/fail/wrong_attribute.rs:11:7 + --> tests/result_labels/fail/wrong_attribute.rs:11:7 | 11 | #[label] | ^^^^^ diff --git a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs b/autometrics/tests/result_labels/fail/wrong_kv_attribute.rs similarity index 83% rename from autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs rename to autometrics/tests/result_labels/fail/wrong_kv_attribute.rs index a465cc87..6a42c296 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.rs +++ b/autometrics/tests/result_labels/fail/wrong_kv_attribute.rs @@ -1,11 +1,11 @@ // This test ensures that the macro fails with a readable // error when the attribute given to one variant inside the // enumeration is not in the correct form. -use autometrics_macros::ErrorLabels; +use autometrics_macros::ResultLabels; struct Inner {} -#[derive(ErrorLabels)] +#[derive(ResultLabels)] enum MyError { Empty, #[label = "error"] diff --git a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr b/autometrics/tests/result_labels/fail/wrong_kv_attribute.stderr similarity index 70% rename from autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr rename to autometrics/tests/result_labels/fail/wrong_kv_attribute.stderr index 8caeda21..ea2201ab 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_kv_attribute.stderr +++ b/autometrics/tests/result_labels/fail/wrong_kv_attribute.stderr @@ -1,5 +1,5 @@ error: Only `label(result = "RES")` (RES can be "ok" or "error") is supported - --> tests/error_labels/fail/wrong_kv_attribute.rs:11:7 + --> tests/result_labels/fail/wrong_kv_attribute.rs:11:7 | 11 | #[label = "error"] | ^^^^^^^^^^^^^^^ diff --git a/autometrics-macros/tests/error_labels/fail/wrong_result_name.rs b/autometrics/tests/result_labels/fail/wrong_result_name.rs similarity index 84% rename from autometrics-macros/tests/error_labels/fail/wrong_result_name.rs rename to autometrics/tests/result_labels/fail/wrong_result_name.rs index 745b2358..0eb6b37c 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_result_name.rs +++ b/autometrics/tests/result_labels/fail/wrong_result_name.rs @@ -1,11 +1,11 @@ // This test ensures that the macro fails with a readable // error when the attribute given to one variant inside the // enumeration does not use the correct key for the label. -use autometrics_macros::ErrorLabels; +use autometrics_macros::ResultLabels; struct Inner {} -#[derive(ErrorLabels)] +#[derive(ResultLabels)] enum MyError { Empty, #[label(unknown = "ok")] diff --git a/autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr b/autometrics/tests/result_labels/fail/wrong_result_name.stderr similarity index 69% rename from autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr rename to autometrics/tests/result_labels/fail/wrong_result_name.stderr index dd6c9f0b..528e8751 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_result_name.stderr +++ b/autometrics/tests/result_labels/fail/wrong_result_name.stderr @@ -1,5 +1,5 @@ error: Only `result = "RES"` (RES can be "ok" or "error") is supported - --> tests/error_labels/fail/wrong_result_name.rs:11:13 + --> tests/result_labels/fail/wrong_result_name.rs:11:13 | 11 | #[label(unknown = "ok")] | ^^^^^^^ diff --git a/autometrics-macros/tests/error_labels/fail/wrong_variant.rs b/autometrics/tests/result_labels/fail/wrong_variant.rs similarity index 87% rename from autometrics-macros/tests/error_labels/fail/wrong_variant.rs rename to autometrics/tests/result_labels/fail/wrong_variant.rs index 7e60d753..09e5262b 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_variant.rs +++ b/autometrics/tests/result_labels/fail/wrong_variant.rs @@ -2,11 +2,11 @@ // attribute given to one variant inside the enumeration does not use one of the // predetermined values (that would make the automatic queries fail, so the // macros need to forbid wrong usage at compile time) -use autometrics_macros::ErrorLabels; +use autometrics_macros::ResultLabels; struct Inner {} -#[derive(ErrorLabels)] +#[derive(ResultLabels)] enum MyError { Empty, #[label(result = "not ok")] diff --git a/autometrics-macros/tests/error_labels/fail/wrong_variant.stderr b/autometrics/tests/result_labels/fail/wrong_variant.stderr similarity index 71% rename from autometrics-macros/tests/error_labels/fail/wrong_variant.stderr rename to autometrics/tests/result_labels/fail/wrong_variant.stderr index 13066bef..cd07d817 100644 --- a/autometrics-macros/tests/error_labels/fail/wrong_variant.stderr +++ b/autometrics/tests/result_labels/fail/wrong_variant.stderr @@ -1,5 +1,5 @@ error: Only "ok" or "error" are accepted as result values - --> tests/error_labels/fail/wrong_variant.rs:12:22 + --> tests/result_labels/fail/wrong_variant.rs:12:22 | 12 | #[label(result = "not ok")] | ^^^^^^^^ diff --git a/autometrics-macros/tests/error_labels/pass/macro.rs b/autometrics/tests/result_labels/pass/macro.rs similarity index 68% rename from autometrics-macros/tests/error_labels/pass/macro.rs rename to autometrics/tests/result_labels/pass/macro.rs index 5518dfba..5bfcaea6 100644 --- a/autometrics-macros/tests/error_labels/pass/macro.rs +++ b/autometrics/tests/result_labels/pass/macro.rs @@ -4,12 +4,12 @@ //! autometrics (the library) is then responsible for orchestrating the //! calls to `__autometrics_get_error_label` correctly when observing //! function call results for the metrics. -use autometrics::__private::GetErrorLabelFromEnum; -use autometrics_macros::ErrorLabels; +use autometrics::__private::GetResultLabelFromEnum; +use autometrics_macros::ResultLabels; struct Inner {} -#[derive(ErrorLabels)] +#[derive(ResultLabels)] enum MyError { #[label(result = "error")] Empty, @@ -22,11 +22,11 @@ enum MyError { fn main() { let err = MyError::ClientError { inner: Inner {} }; - assert_eq!(err.__autometrics_get_error_label(), "ok"); + assert_eq!(err.__autometrics_get_result_label(), "ok"); let err = MyError::Empty; - assert_eq!(err.__autometrics_get_error_label(), "error"); + assert_eq!(err.__autometrics_get_result_label(), "error"); let err = MyError::ServerError(502); - assert_eq!(err.__autometrics_get_error_label(), "error"); + assert_eq!(err.__autometrics_get_result_label(), "error"); } From 78f8588c5f5c282507ac8ce88b8538fbd4557bf9 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Fri, 28 Apr 2023 13:06:28 +0200 Subject: [PATCH 06/14] Simplify code --- autometrics-macros/src/result_labels.rs | 2 +- autometrics/src/labels.rs | 9 --------- autometrics/tests/result_labels/pass/macro.rs | 2 +- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/autometrics-macros/src/result_labels.rs b/autometrics-macros/src/result_labels.rs index 91238481..b3611c82 100644 --- a/autometrics-macros/src/result_labels.rs +++ b/autometrics-macros/src/result_labels.rs @@ -56,7 +56,7 @@ pub(crate) fn expand(input: DeriveInput) -> Result { Ok(quote! { #[automatically_derived] - impl #impl_generics ::autometrics::__private::GetResultLabelFromEnum for #enum_name #ty_generics #where_clause { + impl #impl_generics ::autometrics::__private::GetResultLabel for #enum_name #ty_generics #where_clause { fn __autometrics_get_result_label(&self) -> &'static str { #(#conditional_clauses_for_labels)* #ERROR_KEY diff --git a/autometrics/src/labels.rs b/autometrics/src/labels.rs index 32a43854..21af9de2 100644 --- a/autometrics/src/labels.rs +++ b/autometrics/src/labels.rs @@ -233,12 +233,3 @@ pub trait GetResultLabel { } } impl_trait_for_types!(GetResultLabel); - -/// Implementation detail to get enum variants to specify their own -/// "result" label -pub trait GetResultLabelFromEnum { - /// Return the value to use for the [result](RESULT_KEY) value in the reported metrics - fn __autometrics_get_result_label(&self) -> &'static str { - ERROR_KEY - } -} diff --git a/autometrics/tests/result_labels/pass/macro.rs b/autometrics/tests/result_labels/pass/macro.rs index 5bfcaea6..33a432ce 100644 --- a/autometrics/tests/result_labels/pass/macro.rs +++ b/autometrics/tests/result_labels/pass/macro.rs @@ -4,7 +4,7 @@ //! autometrics (the library) is then responsible for orchestrating the //! calls to `__autometrics_get_error_label` correctly when observing //! function call results for the metrics. -use autometrics::__private::GetResultLabelFromEnum; +use autometrics::__private::GetResultLabel; use autometrics_macros::ResultLabels; struct Inner {} From 1b3931d072a80be6b03e15ef96c28ea2dcd3f0f2 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Fri, 28 Apr 2023 13:18:25 +0200 Subject: [PATCH 07/14] Add reference test Make sure that having the type behind a reference does _not_ fall back to the generic default case --- autometrics/tests/result_labels/pass/macro.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/autometrics/tests/result_labels/pass/macro.rs b/autometrics/tests/result_labels/pass/macro.rs index 33a432ce..dfa78695 100644 --- a/autometrics/tests/result_labels/pass/macro.rs +++ b/autometrics/tests/result_labels/pass/macro.rs @@ -23,10 +23,13 @@ enum MyError { fn main() { let err = MyError::ClientError { inner: Inner {} }; assert_eq!(err.__autometrics_get_result_label(), "ok"); + assert_eq!((&err).__autometrics_get_result_label(), "ok"); let err = MyError::Empty; assert_eq!(err.__autometrics_get_result_label(), "error"); + assert_eq!((&err).__autometrics_get_result_label(), "error"); let err = MyError::ServerError(502); assert_eq!(err.__autometrics_get_result_label(), "error"); + assert_eq!((&err).__autometrics_get_result_label(), "error"); } From 2175cccd9ce758ce2661bcd531da65125d45cba4 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Fri, 28 Apr 2023 15:50:47 +0200 Subject: [PATCH 08/14] WIP: Get ResultLabels to work outside of results --- autometrics-macros/src/result_labels.rs | 87 ++++++++---- autometrics/src/labels.rs | 13 +- autometrics/tests/result_labels/pass/macro.rs | 126 +++++++++++++++--- 3 files changed, 177 insertions(+), 49 deletions(-) diff --git a/autometrics-macros/src/result_labels.rs b/autometrics-macros/src/result_labels.rs index b3611c82..c363a763 100644 --- a/autometrics-macros/src/result_labels.rs +++ b/autometrics-macros/src/result_labels.rs @@ -54,12 +54,28 @@ pub(crate) fn expand(input: DeriveInput) -> Result { let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let conditional_clauses_for_labels = conditional_label_clauses(variants, enum_name)?; + // NOTE: we cannot reuse the GetResultLabel implementation in the GetLabels implementation, + // because the GetResultLabel implementation is for a type T, while the GetLabels argument + // is a reference &T, and the blanket impl of GetResultLabel for &T will be used instead of + // the implementation we just wrote. Ok(quote! { #[automatically_derived] impl #impl_generics ::autometrics::__private::GetResultLabel for #enum_name #ty_generics #where_clause { - fn __autometrics_get_result_label(&self) -> &'static str { + fn __autometrics_get_result_label(&self) -> Option<&'static str> { #(#conditional_clauses_for_labels)* - #ERROR_KEY + } + } + + #[automatically_derived] + impl #impl_generics ::autometrics::__private::GetLabels for #enum_name #ty_generics #where_clause { + fn __autometrics_get_labels(&self) -> Option<::autometrics::__private::ResultAndReturnTypeLabels> { + use ::autometrics::__private::GetStaticStr; + + let result_label = { + #(#conditional_clauses_for_labels)* + }; + + result_label.map(|label| (label, label.__autometrics_static_str())) } } }) @@ -70,27 +86,35 @@ fn conditional_label_clauses( variants: &Punctuated, enum_name: &Ident, ) -> Result> { - variants - .iter() - .map(|variant| { - let variant_name = &variant.ident; - let variant_matcher: TokenStream = match variant.fields { - syn::Fields::Named(_) => quote! { #variant_name {..} }, - syn::Fields::Unnamed(_) => quote! { #variant_name (_) }, - syn::Fields::Unit => quote! { #variant_name }, - }; - if let Some(key) = extract_label_attribute(&variant.attrs)? { - Ok(quote! [ - if ::std::matches!(self, & #enum_name :: #variant_matcher) { - return #key - } - ]) - } else { - // Let the code flow through the last value - Ok(quote! {}) - } - }) - .collect() + // Dummy first clause to write all the useful payload with 'else if's + std::iter::once(Ok(quote![if false { + None + }])) + .chain(variants.iter().map(|variant| { + let variant_name = &variant.ident; + let variant_matcher: TokenStream = match variant.fields { + syn::Fields::Named(_) => quote! { #variant_name {..} }, + syn::Fields::Unnamed(_) => quote! { #variant_name (_) }, + syn::Fields::Unit => quote! { #variant_name }, + }; + if let Some(key) = extract_label_attribute(&variant.attrs)? { + Ok(quote! [ + else if ::std::matches!(self, & #enum_name :: #variant_matcher) { + Some(#key) + } + ]) + } else { + // Let the code flow through the last value + Ok(quote! {}) + } + })) + // Fallback case: we return None + .chain(std::iter::once(Ok(quote! [ + else { + None + } + ]))) + .collect() } /// Extract the wanted label from the annotation in the variant, if present. @@ -119,7 +143,7 @@ fn extract_label_attribute(attrs: &[Attribute]) -> Result> { // Only lists are allowed let Some(syn::NestedMeta::Meta(syn::Meta::NameValue(pair))) = list.nested.first() else { return Some(Err(Error::new_spanned( - meta, + meta, format!("Only `{ATTR_LABEL}({RESULT_KEY} = \"RES\")` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"), ))) }; @@ -150,10 +174,19 @@ fn extract_label_attribute(attrs: &[Attribute]) -> Result> { Some(Ok(lit_str.clone())) }, - _ => Some(Err(Error::new_spanned( - meta, + syn::Meta::NameValue(nv) if nv.path.segments.len() == 1 && nv.path.segments[0].ident == ATTR_LABEL => { + Some(Err(Error::new_spanned( + nv, format!("Only `{ATTR_LABEL}({RESULT_KEY} = \"RES\")` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"), - ))), + ))) + }, + syn::Meta::Path(p) if p.segments.len() == 1 && p.segments[0].ident == ATTR_LABEL => { + Some(Err(Error::new_spanned( + p, + format!("Only `{ATTR_LABEL}({RESULT_KEY} = \"RES\")` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"), + ))) + }, + _ => None, }, Err(e) => Some(Err(Error::new_spanned( att, diff --git a/autometrics/src/labels.rs b/autometrics/src/labels.rs index e51e2bae..282f1b42 100644 --- a/autometrics/src/labels.rs +++ b/autometrics/src/labels.rs @@ -2,7 +2,7 @@ use crate::{constants::*, objectives::*}; use std::ops::Deref; pub(crate) type Label = (&'static str, &'static str); -type ResultAndReturnTypeLabels = (&'static str, Option<&'static str>); +pub type ResultAndReturnTypeLabels = (&'static str, Option<&'static str>); /// These are the labels used for the `build_info` metric. pub struct BuildInfoLabels { @@ -158,9 +158,12 @@ pub trait GetLabelsFromResult { impl GetLabelsFromResult for Result { fn __autometrics_get_labels(&self) -> Option { match self { - Ok(ok) => Some((OK_KEY, ok.__autometrics_static_str())), + Ok(ok) => Some(( + ok.__autometrics_get_result_label().unwrap_or(OK_KEY), + ok.__autometrics_static_str(), + )), Err(err) => Some(( - err.__autometrics_get_result_label(), + err.__autometrics_get_result_label().unwrap_or(ERROR_KEY), err.__autometrics_static_str(), )), } @@ -253,8 +256,8 @@ impl_trait_for_types!(GetStaticStr); /// "result" label pub trait GetResultLabel { /// Return the value to use for the [result](RESULT_KEY) value in the reported metrics - fn __autometrics_get_result_label(&self) -> &'static str { - ERROR_KEY + fn __autometrics_get_result_label(&self) -> Option<&'static str> { + None } } impl_trait_for_types!(GetResultLabel); diff --git a/autometrics/tests/result_labels/pass/macro.rs b/autometrics/tests/result_labels/pass/macro.rs index dfa78695..3ec6660d 100644 --- a/autometrics/tests/result_labels/pass/macro.rs +++ b/autometrics/tests/result_labels/pass/macro.rs @@ -2,34 +2,126 @@ //! //! The goal here is to make sure that the macro has the effect we want. //! autometrics (the library) is then responsible for orchestrating the -//! calls to `__autometrics_get_error_label` correctly when observing +//! calls to `__autometrics_get_result_label` correctly when observing //! function call results for the metrics. -use autometrics::__private::GetResultLabel; +use autometrics::__private::{GetLabels, GetLabelsFromResult, GetResultLabel}; use autometrics_macros::ResultLabels; +#[derive(Clone)] struct Inner {} -#[derive(ResultLabels)] -enum MyError { +#[derive(ResultLabels, Clone)] +enum MyEnum { + /// When manually marked as 'error', returning this variant will + /// _ALWAYS_ be considered as an error for Autometrics. + /// Notably, even if you return Ok(MyEnum::Empty) from a function. #[label(result = "error")] Empty, + /// When manually marked as 'ok', returning this variant will + /// _ALWAYS_ be considered as a succesful result for Autometrics. + /// Notably, even if you return Err(MyEnum::Empty) from a function. #[label(result = "ok")] - ClientError { - inner: Inner, - }, - ServerError(u64), + ClientError { inner: Inner }, + /// Without any manual override, Autometrics will guess from the + /// context when possible to know whether something is an issue or + /// not. This means: + /// - Ok(MyEnum::AmbiguousValue(_)) is a success for Autometrics + /// - Err(MyEnum::AmbiguousValue(_)) is an error for Autometrics + /// - Just returning MyEnum::AmbiguousValue(_) won't do anything (just like returning + /// a bare primitive type like usize) + AmbiguousValue(u64), } fn main() { - let err = MyError::ClientError { inner: Inner {} }; - assert_eq!(err.__autometrics_get_result_label(), "ok"); - assert_eq!((&err).__autometrics_get_result_label(), "ok"); + let is_ok = MyEnum::ClientError { inner: Inner {} }; + assert_eq!(is_ok.__autometrics_get_result_label().unwrap(), "ok"); + assert_eq!((&is_ok).__autometrics_get_result_label().unwrap(), "ok"); + assert_eq!(is_ok.__autometrics_get_labels().unwrap().0, "ok"); + assert_eq!((&is_ok).__autometrics_get_labels().unwrap().0, "ok"); - let err = MyError::Empty; - assert_eq!(err.__autometrics_get_result_label(), "error"); - assert_eq!((&err).__autometrics_get_result_label(), "error"); + let err = MyEnum::Empty; + assert_eq!(err.__autometrics_get_result_label().unwrap(), "error"); + assert_eq!((&err).__autometrics_get_result_label().unwrap(), "error"); + assert_eq!(err.__autometrics_get_labels().unwrap().0, "error"); + assert_eq!((&err).__autometrics_get_labels().unwrap().0, "error"); - let err = MyError::ServerError(502); - assert_eq!(err.__autometrics_get_result_label(), "error"); - assert_eq!((&err).__autometrics_get_result_label(), "error"); + let no_idea = MyEnum::AmbiguousValue(42); + assert_eq!(no_idea.__autometrics_get_result_label(), None); + assert_eq!((&no_idea).__autometrics_get_result_label(), None); + assert_eq!(no_idea.__autometrics_get_labels(), None); + assert_eq!((&no_idea).__autometrics_get_labels(), None); + + // Testing behaviour within an Ok() error variant + let ok: Result = Ok(is_ok.clone()); + assert_eq!( + ok.__autometrics_get_labels().unwrap().0, + "ok", + "When wrapped as the Ok variant of a result, a manually marked 'ok' variant translates to 'ok'." + ); + assert_eq!( + (&ok).__autometrics_get_labels().unwrap().0, + "ok", + "When wrapped as the Ok variant of a result, a manually marked 'ok' variant translates to 'ok'." + ); + + let ok: Result = Ok(no_idea.clone()); + assert_eq!( + ok.__autometrics_get_labels().unwrap().0, + "ok", + "When wrapped as the Ok variant of a result, an ambiguous variant translates to 'ok'." + ); + assert_eq!( + (&ok).__autometrics_get_labels().unwrap().0, + "ok", + "When wrapped as the Ok variant of a result, an ambiguous variant translates to 'ok'." + ); + + let err_in_ok: Result = Ok(err.clone()); + assert_eq!( + err_in_ok.__autometrics_get_labels().unwrap().0, + "error", + "When wrapped as the Ok variant of a result, a manually marked 'error' variant translates to 'error'." + ); + assert_eq!( + (&err_in_ok).__autometrics_get_labels().unwrap().0, + "error", + "When wrapped as the Ok variant of a result, a manually marked 'error' variant translates to 'error'." + ); + + // Testing behaviour within an Err() error variant + let ok_in_err: Result<(), MyEnum> = Err(is_ok); + assert_eq!( + ok_in_err.__autometrics_get_labels().unwrap().0, + "ok", + "When wrapped as the Err variant of a result, a manually marked 'ok' variant translates to 'ok'." + ); + assert_eq!( + (&ok_in_err).__autometrics_get_labels().unwrap().0, + "ok", + "When wrapped as the Err variant of a result, a manually marked 'ok' variant translates to 'ok'." + ); + + let not_ok: Result<(), MyEnum> = Err(err); + assert_eq!( + not_ok.__autometrics_get_labels().unwrap().0, + "error", + "When wrapped as the Err variant of a result, a manually marked 'error' variant translates to 'error'." + ); + assert_eq!( + (¬_ok).__autometrics_get_labels().unwrap().0, + "error", + "When wrapped as the Err variant of a result, a manually marked 'error' variant translates to 'error'." + ); + + let ambiguous: Result<(), MyEnum> = Err(no_idea); + assert_eq!( + ambiguous.__autometrics_get_labels().unwrap().0, + "error", + "When wrapped as the Err variant of a result, an ambiguous variant translates to 'error'." + ); + assert_eq!( + (&ambiguous).__autometrics_get_labels().unwrap().0, + "error", + "When wrapped as the Err variant of a result, an ambiguous variant translates to 'error'." + ); } From dba82141a82505dd5dc305759c0897ca9162f2c5 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Tue, 2 May 2023 17:03:43 +0200 Subject: [PATCH 09/14] Use spez to determine result labels --- autometrics-macros/Cargo.toml | 1 + autometrics-macros/src/lib.rs | 12 +- autometrics-macros/src/result_labels.rs | 17 +-- autometrics/Cargo.toml | 1 + autometrics/src/labels.rs | 105 ++++++++++++------ autometrics/src/lib.rs | 3 + autometrics/tests/result_labels/pass/macro.rs | 70 ++++-------- 7 files changed, 106 insertions(+), 103 deletions(-) diff --git a/autometrics-macros/Cargo.toml b/autometrics-macros/Cargo.toml index 8cdfab85..40c4fc1f 100644 --- a/autometrics-macros/Cargo.toml +++ b/autometrics-macros/Cargo.toml @@ -19,3 +19,4 @@ percent-encoding = "2.2" proc-macro2 = "1" quote = "1" syn = { version = "1", features = ["full"] } +spez = { version = "0.1.2" } diff --git a/autometrics-macros/src/lib.rs b/autometrics-macros/src/lib.rs index 82b18edf..f1e96f4c 100644 --- a/autometrics-macros/src/lib.rs +++ b/autometrics-macros/src/lib.rs @@ -5,8 +5,8 @@ use quote::quote; use std::env; use syn::{parse_macro_input, ImplItem, ItemFn, ItemImpl, Result}; -mod result_labels; mod parse; +mod result_labels; const COUNTER_NAME_PROMETHEUS: &str = "function_calls_count"; const HISTOGRAM_BUCKET_NAME_PROMETHEUS: &str = "function_calls_duration_bucket"; @@ -106,12 +106,14 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result Result { // is a reference &T, and the blanket impl of GetResultLabel for &T will be used instead of // the implementation we just wrote. Ok(quote! { - #[automatically_derived] - impl #impl_generics ::autometrics::__private::GetResultLabel for #enum_name #ty_generics #where_clause { - fn __autometrics_get_result_label(&self) -> Option<&'static str> { - #(#conditional_clauses_for_labels)* - } - } - #[automatically_derived] impl #impl_generics ::autometrics::__private::GetLabels for #enum_name #ty_generics #where_clause { - fn __autometrics_get_labels(&self) -> Option<::autometrics::__private::ResultAndReturnTypeLabels> { - use ::autometrics::__private::GetStaticStr; - - let result_label = { - #(#conditional_clauses_for_labels)* - }; - - result_label.map(|label| (label, label.__autometrics_static_str())) + fn __autometrics_get_labels(&self) -> Option<&'static str> { + #(#conditional_clauses_for_labels)* } } }) diff --git a/autometrics/Cargo.toml b/autometrics/Cargo.toml index a9ac325b..da9fb8dc 100644 --- a/autometrics/Cargo.toml +++ b/autometrics/Cargo.toml @@ -29,6 +29,7 @@ custom-objective-latency = [] [dependencies] autometrics-macros = { workspace = true } +spez = { version = "0.1.2" } # Used for opentelemetry feature opentelemetry_api = { version = "0.18", default-features = false, features = ["metrics"], optional = true } diff --git a/autometrics/src/labels.rs b/autometrics/src/labels.rs index 282f1b42..05505a15 100644 --- a/autometrics/src/labels.rs +++ b/autometrics/src/labels.rs @@ -149,26 +149,6 @@ impl GaugeLabels { // and this answer explains why it works: // https://users.rust-lang.org/t/how-to-check-types-within-macro/33803/8 -pub trait GetLabelsFromResult { - fn __autometrics_get_labels(&self) -> Option { - None - } -} - -impl GetLabelsFromResult for Result { - fn __autometrics_get_labels(&self) -> Option { - match self { - Ok(ok) => Some(( - ok.__autometrics_get_result_label().unwrap_or(OK_KEY), - ok.__autometrics_static_str(), - )), - Err(err) => Some(( - err.__autometrics_get_result_label().unwrap_or(ERROR_KEY), - err.__autometrics_static_str(), - )), - } - } -} pub enum LabelArray { Three([Label; 3]), Four([Label; 4]), @@ -188,9 +168,7 @@ impl Deref for LabelArray { } pub trait GetLabels { - fn __autometrics_get_labels(&self) -> Option { - None - } + fn __autometrics_get_labels(&self) -> Option<&'static str>; } /// Implement the given trait for &T and all primitive types. @@ -230,8 +208,6 @@ macro_rules! impl_trait_for_types { }; } -impl_trait_for_types!(GetLabels); - pub trait GetStaticStrFromIntoStaticStr<'a> { fn __autometrics_static_str(&'a self) -> Option<&'static str>; } @@ -252,12 +228,75 @@ pub trait GetStaticStr { } impl_trait_for_types!(GetStaticStr); -/// Implementation detail to get enum variants to specify their own -/// "result" label -pub trait GetResultLabel { - /// Return the value to use for the [result](RESULT_KEY) value in the reported metrics - fn __autometrics_get_result_label(&self) -> Option<&'static str> { - None - } +#[macro_export] +macro_rules! result_labels { + ($e:expr) => {{ + use $crate::__private::{ + GetLabels, GetStaticStr, ResultAndReturnTypeLabels, ERROR_KEY, OK_KEY, + }; + $crate::__private::spez! { + for val = $e; + + match &Result -> Option { + match val { + Ok(ok) => Some(( + ok.__autometrics_get_labels().unwrap_or(OK_KEY), + ok.__autometrics_static_str(), + )), + Err(err) => Some(( + err.__autometrics_get_labels().unwrap_or(ERROR_KEY), + err.__autometrics_static_str(), + )), + } + } + + match &Result -> Option { + match val { + Ok(ok) => Some(( + ok.__autometrics_get_labels().unwrap_or(OK_KEY), + ok.__autometrics_static_str(), + )), + Err(err) => Some(( + ERROR_KEY, + err.__autometrics_static_str(), + )), + } + } + + match &Result -> Option { + match val { + Ok(ok) => Some(( + OK_KEY, + ok.__autometrics_static_str(), + )), + Err(err) => Some(( + err.__autometrics_get_labels().unwrap_or(ERROR_KEY), + err.__autometrics_static_str(), + )), + } + } + + match &Result -> Option { + match val { + Ok(ok) => Some(( + OK_KEY, + ok.__autometrics_static_str(), + )), + Err(err) => Some(( + ERROR_KEY, + err.__autometrics_static_str(), + )), + } + } + + match &T -> Option { + val.__autometrics_get_labels().map(|label| + (label, val.__autometrics_static_str())) + } + + match &T -> Option { + None + } + } + }}; } -impl_trait_for_types!(GetResultLabel); diff --git a/autometrics/src/lib.rs b/autometrics/src/lib.rs index 6c260952..9a45ffe6 100644 --- a/autometrics/src/lib.rs +++ b/autometrics/src/lib.rs @@ -198,6 +198,7 @@ pub mod __private { use crate::task_local::LocalKey; use std::{cell::RefCell, thread_local}; + pub use crate::constants::*; pub use crate::labels::*; pub use crate::tracker::{AutometricsTracker, TrackMetrics}; @@ -213,4 +214,6 @@ pub mod __private { LocalKey { inner: CALLER_KEY } }; + + pub use spez::spez; } diff --git a/autometrics/tests/result_labels/pass/macro.rs b/autometrics/tests/result_labels/pass/macro.rs index 3ec6660d..14c7c85c 100644 --- a/autometrics/tests/result_labels/pass/macro.rs +++ b/autometrics/tests/result_labels/pass/macro.rs @@ -2,9 +2,9 @@ //! //! The goal here is to make sure that the macro has the effect we want. //! autometrics (the library) is then responsible for orchestrating the -//! calls to `__autometrics_get_result_label` correctly when observing +//! calls to `result_labels!` correctly when observing //! function call results for the metrics. -use autometrics::__private::{GetLabels, GetLabelsFromResult, GetResultLabel}; +use autometrics::result_labels; use autometrics_macros::ResultLabels; #[derive(Clone)] @@ -34,93 +34,63 @@ enum MyEnum { fn main() { let is_ok = MyEnum::ClientError { inner: Inner {} }; - assert_eq!(is_ok.__autometrics_get_result_label().unwrap(), "ok"); - assert_eq!((&is_ok).__autometrics_get_result_label().unwrap(), "ok"); - assert_eq!(is_ok.__autometrics_get_labels().unwrap().0, "ok"); - assert_eq!((&is_ok).__autometrics_get_labels().unwrap().0, "ok"); + let labels = result_labels!(&is_ok); + assert_eq!(labels.unwrap().0, "ok"); let err = MyEnum::Empty; - assert_eq!(err.__autometrics_get_result_label().unwrap(), "error"); - assert_eq!((&err).__autometrics_get_result_label().unwrap(), "error"); - assert_eq!(err.__autometrics_get_labels().unwrap().0, "error"); - assert_eq!((&err).__autometrics_get_labels().unwrap().0, "error"); + let labels = result_labels!(&err); + assert_eq!(labels.unwrap().0, "error"); let no_idea = MyEnum::AmbiguousValue(42); - assert_eq!(no_idea.__autometrics_get_result_label(), None); - assert_eq!((&no_idea).__autometrics_get_result_label(), None); - assert_eq!(no_idea.__autometrics_get_labels(), None); - assert_eq!((&no_idea).__autometrics_get_labels(), None); + let labels = result_labels!(&no_idea); + assert_eq!(labels, None); // Testing behaviour within an Ok() error variant let ok: Result = Ok(is_ok.clone()); + let labels = result_labels!(&ok); assert_eq!( - ok.__autometrics_get_labels().unwrap().0, - "ok", - "When wrapped as the Ok variant of a result, a manually marked 'ok' variant translates to 'ok'." - ); - assert_eq!( - (&ok).__autometrics_get_labels().unwrap().0, + labels.unwrap().0, "ok", "When wrapped as the Ok variant of a result, a manually marked 'ok' variant translates to 'ok'." ); let ok: Result = Ok(no_idea.clone()); + let labels = result_labels!(&ok); assert_eq!( - ok.__autometrics_get_labels().unwrap().0, - "ok", - "When wrapped as the Ok variant of a result, an ambiguous variant translates to 'ok'." - ); - assert_eq!( - (&ok).__autometrics_get_labels().unwrap().0, + labels.unwrap().0, "ok", "When wrapped as the Ok variant of a result, an ambiguous variant translates to 'ok'." ); let err_in_ok: Result = Ok(err.clone()); + let labels = result_labels!(&err_in_ok); assert_eq!( - err_in_ok.__autometrics_get_labels().unwrap().0, - "error", - "When wrapped as the Ok variant of a result, a manually marked 'error' variant translates to 'error'." - ); - assert_eq!( - (&err_in_ok).__autometrics_get_labels().unwrap().0, + labels.unwrap().0, "error", "When wrapped as the Ok variant of a result, a manually marked 'error' variant translates to 'error'." ); // Testing behaviour within an Err() error variant let ok_in_err: Result<(), MyEnum> = Err(is_ok); + let labels = result_labels!(&ok_in_err); assert_eq!( - ok_in_err.__autometrics_get_labels().unwrap().0, - "ok", - "When wrapped as the Err variant of a result, a manually marked 'ok' variant translates to 'ok'." - ); - assert_eq!( - (&ok_in_err).__autometrics_get_labels().unwrap().0, + labels.unwrap().0, "ok", "When wrapped as the Err variant of a result, a manually marked 'ok' variant translates to 'ok'." ); let not_ok: Result<(), MyEnum> = Err(err); + let labels = result_labels!(¬_ok); assert_eq!( - not_ok.__autometrics_get_labels().unwrap().0, - "error", - "When wrapped as the Err variant of a result, a manually marked 'error' variant translates to 'error'." - ); - assert_eq!( - (¬_ok).__autometrics_get_labels().unwrap().0, + labels.unwrap().0, "error", "When wrapped as the Err variant of a result, a manually marked 'error' variant translates to 'error'." ); let ambiguous: Result<(), MyEnum> = Err(no_idea); + let labels = result_labels!(&ambiguous); assert_eq!( - ambiguous.__autometrics_get_labels().unwrap().0, - "error", - "When wrapped as the Err variant of a result, an ambiguous variant translates to 'error'." - ); - assert_eq!( - (&ambiguous).__autometrics_get_labels().unwrap().0, + labels.unwrap().0, "error", "When wrapped as the Err variant of a result, an ambiguous variant translates to 'error'." ); From 4621282a515c1ad8bcaa1045cbd650f166862492 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Wed, 3 May 2023 12:52:33 +0200 Subject: [PATCH 10/14] Try switching match arms --- autometrics-macros/src/lib.rs | 6 +----- autometrics/src/labels.rs | 17 ++++++++--------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/autometrics-macros/src/lib.rs b/autometrics-macros/src/lib.rs index f1e96f4c..19b7ec98 100644 --- a/autometrics-macros/src/lib.rs +++ b/autometrics-macros/src/lib.rs @@ -106,14 +106,10 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result &Result -> Option { + match &Result -> Option { match val { Ok(ok) => Some(( - ok.__autometrics_get_labels().unwrap_or(OK_KEY), + OK_KEY, ok.__autometrics_static_str(), )), Err(err) => Some(( - ERROR_KEY, + err.__autometrics_get_labels().unwrap_or(ERROR_KEY), err.__autometrics_static_str(), )), } } - match &Result -> Option { + match &Result -> Option { match val { Ok(ok) => Some(( - OK_KEY, + ok.__autometrics_get_labels().unwrap_or(OK_KEY), ok.__autometrics_static_str(), )), Err(err) => Some(( - err.__autometrics_get_labels().unwrap_or(ERROR_KEY), + ERROR_KEY, err.__autometrics_static_str(), )), } @@ -290,11 +290,10 @@ macro_rules! result_labels { } match &T -> Option { - val.__autometrics_get_labels().map(|label| - (label, val.__autometrics_static_str())) + val.__autometrics_get_labels().map(|label| (label, val.__autometrics_static_str())) } - match &T -> Option { + match T -> Option { None } } From 6b1869a15d3e37c1e49f22770cf2f6a1a7d3cefa Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Wed, 3 May 2023 14:11:34 +0200 Subject: [PATCH 11/14] Annotate temporary result type when possible This change is necessary to make spez work, otherwise the compiler gets confused about the specialization to choose in the spez::Match generated traits. The issue is not spez-specific, as the macro is simple sugar over the auto-deref specialization trick in general. This commit is also the "maximalist" solution to #74, as it simply ignores the return type if it has syntax incompatible with the context we want to use. Closes: #74 Co-Authored-By: Anthony Deschamps --- autometrics-macros/src/lib.rs | 11 +++++++++-- autometrics/src/labels.rs | 12 ++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/autometrics-macros/src/lib.rs b/autometrics-macros/src/lib.rs index 19b7ec98..21a6c7c8 100644 --- a/autometrics-macros/src/lib.rs +++ b/autometrics-macros/src/lib.rs @@ -3,7 +3,7 @@ use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; use proc_macro2::TokenStream; use quote::quote; use std::env; -use syn::{parse_macro_input, ImplItem, ItemFn, ItemImpl, Result}; +use syn::{parse_macro_input, ImplItem, ItemFn, ItemImpl, Result, ReturnType, Type}; mod parse; mod result_labels; @@ -60,6 +60,13 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result quote! { : () }, + ReturnType::Type(_, ref t) if matches!(t.as_ref(), &Type::ImplTrait(_)) => quote! {}, + ReturnType::Type(_, ref t) => quote! { : #t }, + }; + // Wrap the body of the original function, using a slightly different approach based on whether the function is async let call_function = if sig.asyncness.is_some() { quote! { @@ -144,7 +151,7 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result &Result -> Option { + match &::std::result::Result where T: GetLabels, E: GetLabels -> ::std::option::Option { match val { Ok(ok) => Some(( ok.__autometrics_get_labels().unwrap_or(OK_KEY), @@ -250,7 +250,7 @@ macro_rules! result_labels { } } - match &Result -> Option { + match &::std::result::Result where E: GetLabels -> ::std::option::Option { match val { Ok(ok) => Some(( OK_KEY, @@ -263,7 +263,7 @@ macro_rules! result_labels { } } - match &Result -> Option { + match &::std::result::Result where T: GetLabels -> ::std::option::Option { match val { Ok(ok) => Some(( ok.__autometrics_get_labels().unwrap_or(OK_KEY), @@ -276,7 +276,7 @@ macro_rules! result_labels { } } - match &Result -> Option { + match &::std::result::Result -> ::std::option::Option { match val { Ok(ok) => Some(( OK_KEY, @@ -289,11 +289,11 @@ macro_rules! result_labels { } } - match &T -> Option { + match &T where T: GetLabels -> ::std::option::Option { val.__autometrics_get_labels().map(|label| (label, val.__autometrics_static_str())) } - match T -> Option { + match T -> ::std::option::Option { None } } From 4e67714ce9d08fefb2682db4f43dc5dbd77f48c9 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Wed, 3 May 2023 16:24:30 +0200 Subject: [PATCH 12/14] Add comments --- autometrics-macros/src/lib.rs | 25 ++++++++++++++++++++++++- autometrics-macros/src/result_labels.rs | 4 ---- autometrics/src/labels.rs | 1 + 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/autometrics-macros/src/lib.rs b/autometrics-macros/src/lib.rs index 21a6c7c8..45c923b2 100644 --- a/autometrics-macros/src/lib.rs +++ b/autometrics-macros/src/lib.rs @@ -60,7 +60,30 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result examples/full-api/src/routes.rs:48:1 + // | + //48 | #[autometrics(objective = API_SLO)] + // | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `GetLabels` is not implemented for `ApiError` + // | + // = help: the trait `create_user::{closure#0}::Match2` is implemented for `&&&&create_user::{closure#0}::Match<&Result>` + //note: required for `&&&&create_user::{closure#0}::Match<&Result, ApiError>>` to implement `create_user::{closure#0}::Match2` + // --> examples/full-api/src/routes.rs:48:1 + // | + //48 | #[autometrics(objective = API_SLO)] + // | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + // = note: this error originates in the macro `$crate::__private::spez` which comes from the expansion of the attribute macro `autometrics` (in Nightly builds, run with -Z macro-backtrace for more info) + // ``` + // + // specifying the return type makes the compiler select the (correct) fallback case of `ApiError` not being a + // `GetLabels` implementor. let return_type = match sig.output { ReturnType::Default => quote! { : () }, ReturnType::Type(_, ref t) if matches!(t.as_ref(), &Type::ImplTrait(_)) => quote! {}, diff --git a/autometrics-macros/src/result_labels.rs b/autometrics-macros/src/result_labels.rs index 545ae676..b1cb131b 100644 --- a/autometrics-macros/src/result_labels.rs +++ b/autometrics-macros/src/result_labels.rs @@ -54,10 +54,6 @@ pub(crate) fn expand(input: DeriveInput) -> Result { let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let conditional_clauses_for_labels = conditional_label_clauses(variants, enum_name)?; - // NOTE: we cannot reuse the GetResultLabel implementation in the GetLabels implementation, - // because the GetResultLabel implementation is for a type T, while the GetLabels argument - // is a reference &T, and the blanket impl of GetResultLabel for &T will be used instead of - // the implementation we just wrote. Ok(quote! { #[automatically_derived] impl #impl_generics ::autometrics::__private::GetLabels for #enum_name #ty_generics #where_clause { diff --git a/autometrics/src/labels.rs b/autometrics/src/labels.rs index 8f7ed6d0..411f311d 100644 --- a/autometrics/src/labels.rs +++ b/autometrics/src/labels.rs @@ -167,6 +167,7 @@ impl Deref for LabelArray { } } +/// A trait to override the inferred label for the "result" of a function call. pub trait GetLabels { fn __autometrics_get_labels(&self) -> Option<&'static str>; } From b88cff349e2f4ffe9794070259620c15dc69ca71 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Thu, 4 May 2023 11:10:00 +0200 Subject: [PATCH 13/14] Review --- autometrics-macros/Cargo.toml | 1 - autometrics-macros/src/lib.rs | 2 +- autometrics-macros/src/result_labels.rs | 113 ++++++++---------- autometrics/src/labels.rs | 13 +- autometrics/src/lib.rs | 36 ++++-- autometrics/tests/result_labels/pass/macro.rs | 22 ++-- 6 files changed, 99 insertions(+), 88 deletions(-) diff --git a/autometrics-macros/Cargo.toml b/autometrics-macros/Cargo.toml index 40c4fc1f..8cdfab85 100644 --- a/autometrics-macros/Cargo.toml +++ b/autometrics-macros/Cargo.toml @@ -19,4 +19,3 @@ percent-encoding = "2.2" proc-macro2 = "1" quote = "1" syn = { version = "1", features = ["full"] } -spez = { version = "0.1.2" } diff --git a/autometrics-macros/src/lib.rs b/autometrics-macros/src/lib.rs index 45c923b2..e55efbd8 100644 --- a/autometrics-macros/src/lib.rs +++ b/autometrics-macros/src/lib.rs @@ -139,7 +139,7 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result Result { - let Data::Enum(DataEnum { - variants, - ..}) = &input.data else - { - return Err(Error::new_spanned( - input, - "ResultLabels only works with 'Enum's.", - )) - }; + let variants = match &input.data { + Data::Enum(DataEnum { variants, .. }) => variants, + _ => { + return Err(Error::new_spanned( + input, + "ResultLabels only works with 'Enum's.", + )) + } + }; let enum_name = &input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let conditional_clauses_for_labels = conditional_label_clauses(variants, enum_name)?; @@ -58,7 +35,7 @@ pub(crate) fn expand(input: DeriveInput) -> Result { #[automatically_derived] impl #impl_generics ::autometrics::__private::GetLabels for #enum_name #ty_generics #where_clause { fn __autometrics_get_labels(&self) -> Option<&'static str> { - #(#conditional_clauses_for_labels)* + #conditional_clauses_for_labels } } }) @@ -68,36 +45,38 @@ pub(crate) fn expand(input: DeriveInput) -> Result { fn conditional_label_clauses( variants: &Punctuated, enum_name: &Ident, -) -> Result> { - // Dummy first clause to write all the useful payload with 'else if's - std::iter::once(Ok(quote![if false { - None - }])) - .chain(variants.iter().map(|variant| { - let variant_name = &variant.ident; - let variant_matcher: TokenStream = match variant.fields { - syn::Fields::Named(_) => quote! { #variant_name {..} }, - syn::Fields::Unnamed(_) => quote! { #variant_name (_) }, - syn::Fields::Unit => quote! { #variant_name }, - }; - if let Some(key) = extract_label_attribute(&variant.attrs)? { - Ok(quote! [ - else if ::std::matches!(self, & #enum_name :: #variant_matcher) { - Some(#key) - } - ]) - } else { - // Let the code flow through the last value - Ok(quote! {}) +) -> Result { + let clauses: Vec = variants + .iter() + .map(|variant| { + let variant_name = &variant.ident; + let variant_matcher: TokenStream = match variant.fields { + syn::Fields::Named(_) => quote! { #variant_name {..} }, + syn::Fields::Unnamed(_) => quote! { #variant_name (_) }, + syn::Fields::Unit => quote! { #variant_name }, + }; + if let Some(key) = extract_label_attribute(&variant.attrs)? { + Ok(quote! [ + else if ::std::matches!(self, & #enum_name :: #variant_matcher) { + Some(#key) + } + ]) + } else { + // Let the code flow through the last value + Ok(quote! {}) + } + }) + .collect::>>()?; + + Ok(quote! [ + if false { + None } - })) - // Fallback case: we return None - .chain(std::iter::once(Ok(quote! [ + #(#clauses)* else { None } - ]))) - .collect() + ]) } /// Extract the wanted label from the annotation in the variant, if present. @@ -124,11 +103,12 @@ fn extract_label_attribute(attrs: &[Attribute]) -> Result> { } // Only lists are allowed - let Some(syn::NestedMeta::Meta(syn::Meta::NameValue(pair))) = list.nested.first() else { - return Some(Err(Error::new_spanned( + let pair = match list.nested.first() { + Some(syn::NestedMeta::Meta(syn::Meta::NameValue(pair))) => pair, + _ => return Some(Err(Error::new_spanned( meta, format!("Only `{ATTR_LABEL}({RESULT_KEY} = \"RES\")` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"), - ))) + ))), }; // Inside list, only 'result = ...' are allowed @@ -140,11 +120,14 @@ fn extract_label_attribute(attrs: &[Attribute]) -> Result> { } // Inside 'result = val', 'val' must be a string literal - let Lit::Str(ref lit_str) = pair.lit else { + let lit_str = match pair.lit { + Lit::Str(ref lit_str) => lit_str, + _ => { return Some(Err(Error::new_spanned( &pair.lit, format!("Only {OK_KEY:?} or {ERROR_KEY:?}, as string literals, are accepted as result values"), ))); + } }; // Inside 'result = val', 'val' must be one of the allowed string literals diff --git a/autometrics/src/labels.rs b/autometrics/src/labels.rs index 411f311d..b27a2a3d 100644 --- a/autometrics/src/labels.rs +++ b/autometrics/src/labels.rs @@ -229,8 +229,19 @@ pub trait GetStaticStr { } impl_trait_for_types!(GetStaticStr); +/// Return the value of labels to use for the "result" counter according to +/// the value's exact type and attributes. +/// +/// The macro uses the autoref specialization trick through spez to get the labels for the type in a variety of circumstances. +/// Specifically, if the value is a Result, it will add the ok or error label accordingly unless one or both of the types that +/// the Result is generic over implements the GetLabels trait. The label allows to override the inferred label, and the +/// [`ResultLabels`](crate::result_labels) macro implements the GetLabels trait for the user using annotations. +/// +/// The macro is meant to be called with a reference as argument: `get_result_labels_for_value(&return_value)` +/// +/// Ref: https://github.com/dtolnay/case-studies/blob/master/autoref-specialization/README.md #[macro_export] -macro_rules! result_labels { +macro_rules! get_result_labels_for_value { ($e:expr) => {{ use $crate::__private::{ GetLabels, GetStaticStr, ResultAndReturnTypeLabels, ERROR_KEY, OK_KEY, diff --git a/autometrics/src/lib.rs b/autometrics/src/lib.rs index 9a45ffe6..c97f1f61 100644 --- a/autometrics/src/lib.rs +++ b/autometrics/src/lib.rs @@ -136,14 +136,13 @@ pub use autometrics_macros::autometrics; /// # Autometrics custom error labelling /// -/// The ResultLabels derive macro allows to specify -/// inside an enumeration whether variants should be considered as errors or -/// successes as far as the [automatic metrics](autometrics) are concerned. +/// The `ResultLabels` derive macro allows to specify +/// inside an enumeration whether its variants should be considered as errors or +/// successes from the perspective of [automatic metrics](autometrics) generated by +/// autometrics. /// -/// For example, this would allow you to put all the client-side errors in a -/// HTTP webserver (4**) as successes, since it means the handler function -/// _successfully_ rejected a bad request, and that should not affect the SLO or -/// the success rate of the function in the metrics. +/// For example, this would allow you to ignore the client-side HTTP errors (400-499) +/// from the function's success rate and any Service-Level Objectives (SLOs) it is part of. /// /// Putting such a policy in place would look like this in code: /// @@ -152,13 +151,21 @@ pub use autometrics_macros::autometrics; /// /// #[derive(ResultLabels)] /// pub enum ServiceError { -/// // By default, the variant will be labeled as an error, -/// // so you do not need to decorate every variant +/// // By default, the variant will be inferred by the context, +/// // so you do not need to decorate every variant. +/// // - if ServiceError::Database is in an `Err(_)` variant, it will be an "error", +/// // - if ServiceError::Database is in an `Ok(_)` variant, it will be an "ok", +/// // - otherwise, no label will be added /// Database, /// // It is possible to mention it as well of course. /// // Only "error" and "ok" are accepted values +/// // +/// // Forcing "error" here means that even returning `Ok(ServiceError::Network)` +/// // from a function will count as an error for autometrics. /// #[label(result = "error")] /// Network, +/// // Forcing "ok" here means that even returning `Err(ServiceError::Authentication)` +/// // from a function will count as a success for autometrics. /// #[label(result = "ok")] /// Authentication, /// #[label(result = "ok")] @@ -172,6 +179,17 @@ pub use autometrics_macros::autometrics; /// `ServiceError::Authentication` or `Authorization` would _not_ count as a /// failure from your handler that should trigger alerts and consume the "error /// budget" of the service. +/// +/// ## Per-function labelling +/// +/// The `ResultLabels` macro does _not_ have the granularity to behave +/// differently on different functions: if returning +/// `ServiceError::Authentication` from `function_a` is "ok", then returning +/// `ServiceError::Authentication` from `function_b` will be "ok" too. +/// +/// To work around this, you must use the `ok_if` or `error_if` arguments to the +/// [autometrics](crate::autometrics) invocation on `function_b`: those +/// directives have priority over the ResultLabels annotations. pub use autometrics_macros::ResultLabels; // Optional exports diff --git a/autometrics/tests/result_labels/pass/macro.rs b/autometrics/tests/result_labels/pass/macro.rs index 14c7c85c..df399eb5 100644 --- a/autometrics/tests/result_labels/pass/macro.rs +++ b/autometrics/tests/result_labels/pass/macro.rs @@ -2,9 +2,9 @@ //! //! The goal here is to make sure that the macro has the effect we want. //! autometrics (the library) is then responsible for orchestrating the -//! calls to `result_labels!` correctly when observing +//! calls to `get_result_labels_for_value!` correctly when observing //! function call results for the metrics. -use autometrics::result_labels; +use autometrics::get_result_labels_for_value; use autometrics_macros::ResultLabels; #[derive(Clone)] @@ -34,20 +34,20 @@ enum MyEnum { fn main() { let is_ok = MyEnum::ClientError { inner: Inner {} }; - let labels = result_labels!(&is_ok); + let labels = get_result_labels_for_value!(&is_ok); assert_eq!(labels.unwrap().0, "ok"); let err = MyEnum::Empty; - let labels = result_labels!(&err); + let labels = get_result_labels_for_value!(&err); assert_eq!(labels.unwrap().0, "error"); let no_idea = MyEnum::AmbiguousValue(42); - let labels = result_labels!(&no_idea); + let labels = get_result_labels_for_value!(&no_idea); assert_eq!(labels, None); // Testing behaviour within an Ok() error variant let ok: Result = Ok(is_ok.clone()); - let labels = result_labels!(&ok); + let labels = get_result_labels_for_value!(&ok); assert_eq!( labels.unwrap().0, "ok", @@ -55,7 +55,7 @@ fn main() { ); let ok: Result = Ok(no_idea.clone()); - let labels = result_labels!(&ok); + let labels = get_result_labels_for_value!(&ok); assert_eq!( labels.unwrap().0, "ok", @@ -63,7 +63,7 @@ fn main() { ); let err_in_ok: Result = Ok(err.clone()); - let labels = result_labels!(&err_in_ok); + let labels = get_result_labels_for_value!(&err_in_ok); assert_eq!( labels.unwrap().0, "error", @@ -72,7 +72,7 @@ fn main() { // Testing behaviour within an Err() error variant let ok_in_err: Result<(), MyEnum> = Err(is_ok); - let labels = result_labels!(&ok_in_err); + let labels = get_result_labels_for_value!(&ok_in_err); assert_eq!( labels.unwrap().0, "ok", @@ -80,7 +80,7 @@ fn main() { ); let not_ok: Result<(), MyEnum> = Err(err); - let labels = result_labels!(¬_ok); + let labels = get_result_labels_for_value!(¬_ok); assert_eq!( labels.unwrap().0, "error", @@ -88,7 +88,7 @@ fn main() { ); let ambiguous: Result<(), MyEnum> = Err(no_idea); - let labels = result_labels!(&ambiguous); + let labels = get_result_labels_for_value!(&ambiguous); assert_eq!( labels.unwrap().0, "error", From a0acdc80f5ca93e624578e65f02a03755d465e99 Mon Sep 17 00:00:00 2001 From: Gerry Agbobada Date: Tue, 9 May 2023 17:02:04 +0200 Subject: [PATCH 14/14] Add changelog --- CHANGELOG.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65b05480..31a84692 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,19 +10,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added -- + +- `ResultLabels` derive macro allows to specify on an enum whether variants should + always be "ok", or "error" for the success rate metrics of functions using them. (#61) ### Changed -- + +- `GetLabels` trait (publicly exported but meant for internal use) changed the signature + of its function to accomodate the new `ResultLabels` macro. This change is not significant + if you never imported `autometrics::__private` manually (#61) ### Deprecated - ### Removed -- + +- `GetLabelsForResult` trait (publicly exported but meant for internal use) was removed + to accomodate the new `ResultLabels` macro. This change is not significant + if you never imported `autometrics::__private` manually (#61) ### Fixed -- + +- `#[autometrics]` now works on functions that use type inference in their return statement + (#74, #61) ### Security -