Skip to content

Commit 56c3916

Browse files
gagboadeschamps
andauthored
Allow specifying result labels for enum variants (#61)
* 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). * Add tests for ResultLabels using TryBuild This allowed to discover a bug in the code generated, which has also been fixed in the commit. * Use spez to determine result labels This makes code generation simpler to understand, and allows to get rid of the autoref specialization trick used in the GetLabels and GetLabelsForResult traits. Instead that logic is fully contained in the expansion of the spez macro * 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<i> 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 <anthony.j.deschamps@gmail.com>
1 parent 5cc2aa5 commit 56c3916

File tree

17 files changed

+570
-31
lines changed

17 files changed

+570
-31
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
target
22
Cargo.lock
33
data
4+
5+
# Trybuild compilation error outputs in development phase
6+
wip

CHANGELOG.md

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
## [Unreleased]
1111

1212
### Added
13-
-
13+
14+
- `ResultLabels` derive macro allows to specify on an enum whether variants should
15+
always be "ok", or "error" for the success rate metrics of functions using them. (#61)
1416

1517
### Changed
16-
-
18+
19+
- `GetLabels` trait (publicly exported but meant for internal use) changed the signature
20+
of its function to accomodate the new `ResultLabels` macro. This change is not significant
21+
if you never imported `autometrics::__private` manually (#61)
1722

1823
### Deprecated
1924
-
2025

2126
### Removed
22-
-
27+
28+
- `GetLabelsForResult` trait (publicly exported but meant for internal use) was removed
29+
to accomodate the new `ResultLabels` macro. This change is not significant
30+
if you never imported `autometrics::__private` manually (#61)
2331

2432
### Fixed
25-
-
33+
34+
- `#[autometrics]` now works on functions that use type inference in their return statement
35+
(#74, #61)
2636

2737
### Security
2838
-

autometrics-macros/src/lib.rs

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC};
33
use proc_macro2::TokenStream;
44
use quote::quote;
55
use std::env;
6-
use syn::{parse_macro_input, ImplItem, ItemFn, ItemImpl, Result};
6+
use syn::{parse_macro_input, ImplItem, ItemFn, ItemImpl, Result, ReturnType, Type};
77

88
mod parse;
9+
mod result_labels;
910

1011
const COUNTER_NAME_PROMETHEUS: &str = "function_calls_count";
1112
const HISTOGRAM_BUCKET_NAME_PROMETHEUS: &str = "function_calls_duration_bucket";
@@ -36,6 +37,14 @@ pub fn autometrics(
3637
output.into()
3738
}
3839

40+
#[proc_macro_derive(ResultLabels, attributes(label))]
41+
pub fn result_labels(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
42+
let input = parse_macro_input!(input as syn::DeriveInput);
43+
result_labels::expand(input)
44+
.unwrap_or_else(syn::Error::into_compile_error)
45+
.into()
46+
}
47+
3948
/// Add autometrics instrumentation to a single function
4049
fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result<TokenStream> {
4150
let sig = item.sig;
@@ -51,6 +60,36 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result<TokenStre
5160
// Build the documentation we'll add to the function's RustDocs
5261
let metrics_docs = create_metrics_docs(&prometheus_url, &function_name, args.track_concurrency);
5362

63+
// Type annotation to allow type inference to work on return expressions (such as `.collect()`), as
64+
// well as prevent compiler type-inference from selecting the wrong branch in the `spez` macro later.
65+
//
66+
// Type inference can make the compiler select one of the early cases of `autometrics::result_labels!`
67+
// even if the types `T` or `E` do not implement the `GetLabels` trait. That leads to a compilation error
68+
// looking like this:
69+
// ```
70+
// error[E0277]: the trait bound `ApiError: GetLabels` is not satisfied
71+
// --> examples/full-api/src/routes.rs:48:1
72+
// |
73+
//48 | #[autometrics(objective = API_SLO)]
74+
// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `GetLabels` is not implemented for `ApiError`
75+
// |
76+
// = help: the trait `create_user::{closure#0}::Match2` is implemented for `&&&&create_user::{closure#0}::Match<&Result<T, E>>`
77+
//note: required for `&&&&create_user::{closure#0}::Match<&Result<Json<User>, ApiError>>` to implement `create_user::{closure#0}::Match2`
78+
// --> examples/full-api/src/routes.rs:48:1
79+
// |
80+
//48 | #[autometrics(objective = API_SLO)]
81+
// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
82+
// = 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)
83+
// ```
84+
//
85+
// specifying the return type makes the compiler select the (correct) fallback case of `ApiError` not being a
86+
// `GetLabels` implementor.
87+
let return_type = match sig.output {
88+
ReturnType::Default => quote! { : () },
89+
ReturnType::Type(_, ref t) if matches!(t.as_ref(), &Type::ImplTrait(_)) => quote! {},
90+
ReturnType::Type(_, ref t) => quote! { : #t },
91+
};
92+
5493
// Wrap the body of the original function, using a slightly different approach based on whether the function is async
5594
let call_function = if sig.asyncness.is_some() {
5695
quote! {
@@ -97,12 +136,10 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result<TokenStre
97136
}
98137
}
99138
} else {
100-
// This will use the traits defined in the `labels` module to determine if
101-
// the return value was a `Result` and, if so, assign the appropriate labels
102139
quote! {
103140
{
104-
use autometrics::__private::{CALLER, CounterLabels, GetLabels, GetLabelsFromResult};
105-
let result_labels = (&result).__autometrics_get_labels();
141+
use autometrics::__private::{CALLER, CounterLabels, GetLabels};
142+
let result_labels = autometrics::get_result_labels_for_value!(&result);
106143
CounterLabels::new(
107144
#function_name,
108145
module_path!(),
@@ -137,7 +174,7 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result<TokenStre
137174
AutometricsTracker::start(#gauge_labels)
138175
};
139176

140-
let result = #call_function;
177+
let result #return_type = #call_function;
141178

142179
{
143180
use autometrics::__private::{HistogramLabels, TrackMetrics};
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
//! The definition of the ResultLabels derive macro, see
2+
//! autometrics::ResultLabels for more information.
3+
4+
use proc_macro2::TokenStream;
5+
use quote::quote;
6+
use syn::{
7+
punctuated::Punctuated, token::Comma, Attribute, Data, DataEnum, DeriveInput, Error, Ident,
8+
Lit, LitStr, Result, Variant,
9+
};
10+
11+
// These labels must match autometrics::ERROR_KEY and autometrics::OK_KEY,
12+
// to avoid a dependency loop just for 2 constants we recreate these here.
13+
const OK_KEY: &str = "ok";
14+
const ERROR_KEY: &str = "error";
15+
const RESULT_KEY: &str = "result";
16+
const ATTR_LABEL: &str = "label";
17+
const ACCEPTED_LABELS: [&str; 2] = [ERROR_KEY, OK_KEY];
18+
19+
/// Entry point of the ResultLabels macro
20+
pub(crate) fn expand(input: DeriveInput) -> Result<TokenStream> {
21+
let variants = match &input.data {
22+
Data::Enum(DataEnum { variants, .. }) => variants,
23+
_ => {
24+
return Err(Error::new_spanned(
25+
input,
26+
"ResultLabels only works with 'Enum's.",
27+
))
28+
}
29+
};
30+
let enum_name = &input.ident;
31+
let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
32+
let conditional_clauses_for_labels = conditional_label_clauses(variants, enum_name)?;
33+
34+
Ok(quote! {
35+
#[automatically_derived]
36+
impl #impl_generics ::autometrics::__private::GetLabels for #enum_name #ty_generics #where_clause {
37+
fn __autometrics_get_labels(&self) -> Option<&'static str> {
38+
#conditional_clauses_for_labels
39+
}
40+
}
41+
})
42+
}
43+
44+
/// Build the list of match clauses for the generated code.
45+
fn conditional_label_clauses(
46+
variants: &Punctuated<Variant, Comma>,
47+
enum_name: &Ident,
48+
) -> Result<TokenStream> {
49+
let clauses: Vec<TokenStream> = variants
50+
.iter()
51+
.map(|variant| {
52+
let variant_name = &variant.ident;
53+
let variant_matcher: TokenStream = match variant.fields {
54+
syn::Fields::Named(_) => quote! { #variant_name {..} },
55+
syn::Fields::Unnamed(_) => quote! { #variant_name (_) },
56+
syn::Fields::Unit => quote! { #variant_name },
57+
};
58+
if let Some(key) = extract_label_attribute(&variant.attrs)? {
59+
Ok(quote! [
60+
else if ::std::matches!(self, & #enum_name :: #variant_matcher) {
61+
Some(#key)
62+
}
63+
])
64+
} else {
65+
// Let the code flow through the last value
66+
Ok(quote! {})
67+
}
68+
})
69+
.collect::<Result<Vec<_>>>()?;
70+
71+
Ok(quote! [
72+
if false {
73+
None
74+
}
75+
#(#clauses)*
76+
else {
77+
None
78+
}
79+
])
80+
}
81+
82+
/// Extract the wanted label from the annotation in the variant, if present.
83+
/// The function looks for `#[label(result = "ok")]` kind of labels.
84+
///
85+
/// ## Error cases
86+
///
87+
/// The function will error out with the smallest possible span when:
88+
///
89+
/// - The attribute on a variant is not a "list" type (so `#[label]` is not allowed),
90+
/// - The key in the key value pair is not "result", as it's the only supported keyword
91+
/// for now (so `#[label(non_existing_label = "ok")]` is not allowed),
92+
/// - The value for the "result" label is not in the autometrics supported set (so
93+
/// `#[label(result = "random label that will break queries")]` is not allowed)
94+
fn extract_label_attribute(attrs: &[Attribute]) -> Result<Option<LitStr>> {
95+
attrs
96+
.iter()
97+
.find_map(|att| match att.parse_meta() {
98+
Ok(meta) => match &meta {
99+
syn::Meta::List(list) => {
100+
// Ignore attribute if it's not `label(...)`
101+
if list.path.segments.len() != 1 || list.path.segments[0].ident != ATTR_LABEL {
102+
return None;
103+
}
104+
105+
// Only lists are allowed
106+
let pair = match list.nested.first() {
107+
Some(syn::NestedMeta::Meta(syn::Meta::NameValue(pair))) => pair,
108+
_ => return Some(Err(Error::new_spanned(
109+
meta,
110+
format!("Only `{ATTR_LABEL}({RESULT_KEY} = \"RES\")` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"),
111+
))),
112+
};
113+
114+
// Inside list, only 'result = ...' are allowed
115+
if pair.path.segments.len() != 1 || pair.path.segments[0].ident != RESULT_KEY {
116+
return Some(Err(Error::new_spanned(
117+
pair.path.clone(),
118+
format!("Only `{RESULT_KEY} = \"RES\"` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"),
119+
)));
120+
}
121+
122+
// Inside 'result = val', 'val' must be a string literal
123+
let lit_str = match pair.lit {
124+
Lit::Str(ref lit_str) => lit_str,
125+
_ => {
126+
return Some(Err(Error::new_spanned(
127+
&pair.lit,
128+
format!("Only {OK_KEY:?} or {ERROR_KEY:?}, as string literals, are accepted as result values"),
129+
)));
130+
}
131+
};
132+
133+
// Inside 'result = val', 'val' must be one of the allowed string literals
134+
if !ACCEPTED_LABELS.contains(&lit_str.value().as_str()) {
135+
return Some(Err(Error::new_spanned(
136+
lit_str,
137+
format!("Only {OK_KEY:?} or {ERROR_KEY:?} are accepted as result values"),
138+
)));
139+
}
140+
141+
Some(Ok(lit_str.clone()))
142+
},
143+
syn::Meta::NameValue(nv) if nv.path.segments.len() == 1 && nv.path.segments[0].ident == ATTR_LABEL => {
144+
Some(Err(Error::new_spanned(
145+
nv,
146+
format!("Only `{ATTR_LABEL}({RESULT_KEY} = \"RES\")` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"),
147+
)))
148+
},
149+
syn::Meta::Path(p) if p.segments.len() == 1 && p.segments[0].ident == ATTR_LABEL => {
150+
Some(Err(Error::new_spanned(
151+
p,
152+
format!("Only `{ATTR_LABEL}({RESULT_KEY} = \"RES\")` (RES can be {OK_KEY:?} or {ERROR_KEY:?}) is supported"),
153+
)))
154+
},
155+
_ => None,
156+
},
157+
Err(e) => Some(Err(Error::new_spanned(
158+
att,
159+
format!("could not parse the meta attribute: {e}"),
160+
))),
161+
})
162+
.transpose()
163+
}

autometrics/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ custom-objective-latency = []
2929

3030
[dependencies]
3131
autometrics-macros = { workspace = true }
32+
spez = { version = "0.1.2" }
3233

3334
# Used for opentelemetry feature
3435
opentelemetry_api = { version = "0.18", default-features = false, features = ["metrics"], optional = true }
@@ -51,6 +52,7 @@ axum = { version = "0.6", features = ["tokio"] }
5152
regex = "1.7"
5253
http = "0.2"
5354
tokio = { version = "1", features = ["full"] }
55+
trybuild = "1.0"
5456
vergen = { version = "8.1", features = ["git", "gitcl"] }
5557

5658
[package.metadata.docs.rs]

0 commit comments

Comments
 (0)