Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refactor]: update iroha_telemetry_derive to use syn2 #4168

Merged
merged 4 commits into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions telemetry/derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ is-it-maintained-open-issues = { repository = "https://github.com/hyperledger/ir
maintenance = { status = "actively-developed" }

[dependencies]
syn = { workspace = true, features = ["default", "full"] }
syn2 = { workspace = true }
quote = { workspace = true }
proc-macro2 = { workspace = true }
proc-macro-error = { workspace = true }
manyhow = { workspace = true }
iroha_macro_utils = { workspace = true }

[dev-dependencies]
iroha_core = { workspace = true }
Expand Down
125 changes: 78 additions & 47 deletions telemetry/derive/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
//! Attribute-like macro for instrumenting `isi` for `prometheus`
//! metrics. See [`macro@metrics`] for more details.

use proc_macro::TokenStream;
#[cfg(feature = "metric-instrumentation")]
use proc_macro2::TokenStream as TokenStream2;
use proc_macro_error::{abort, proc_macro_error};
use iroha_macro_utils::Emitter;
use manyhow::{emit, manyhow, Result};
use proc_macro2::TokenStream;
use quote::{quote, ToTokens};
use syn::{
parse::Parse, parse_macro_input, punctuated::Punctuated, token::Comma, FnArg, ItemFn, LitStr,
Path, Type,
};
use syn2::{parse::Parse, punctuated::Punctuated, token::Comma, FnArg, LitStr, Path, Type};

// TODO: export these as soon as proc-macro crates are able to export
// anything other than proc-macros.
Expand Down Expand Up @@ -43,19 +39,19 @@ fn type_has_metrics_field(ty: &Type) -> bool {
///
/// # Errors
/// If no argument is of type `WorldStateView`.
fn arg_metrics(input: &Punctuated<FnArg, Comma>) -> Result<syn::Ident, &Punctuated<FnArg, Comma>> {
fn arg_metrics(input: &Punctuated<FnArg, Comma>) -> Result<syn2::Ident, &Punctuated<FnArg, Comma>> {
input
.iter()
.find(|arg| match arg {
FnArg::Typed(typ) => match &*typ.ty {
syn::Type::Reference(typ) => type_has_metrics_field(&typ.elem),
syn2::Type::Reference(typ) => type_has_metrics_field(&typ.elem),
_ => false,
},
_ => false,
})
.and_then(|arg| match arg {
FnArg::Typed(typ) => match *typ.pat.clone() {
syn::Pat::Ident(ident) => Some(ident.ident),
syn2::Pat::Ident(ident) => Some(ident.ident),
_ => None,
},
_ => None,
Expand All @@ -67,7 +63,7 @@ fn arg_metrics(input: &Punctuated<FnArg, Comma>) -> Result<syn::Ident, &Punctuat
struct MetricSpecs(Vec<MetricSpec>); // `HashSet` — idiomatic; slow

impl Parse for MetricSpecs {
fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
fn parse(input: syn2::parse::ParseStream) -> syn2::Result<Self> {
let vars = Punctuated::<MetricSpec, Comma>::parse_terminated(input)?;
Ok(Self(vars.into_iter().collect()))
}
Expand All @@ -80,22 +76,22 @@ struct MetricSpec {
}

impl Parse for MetricSpec {
fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
let _timing = <syn::Token![+]>::parse(input).is_ok();
let metric_name_lit = syn::Lit::parse(input)?;
fn parse(input: syn2::parse::ParseStream) -> syn2::Result<Self> {
let _timing = <syn2::Token![+]>::parse(input).is_ok();
let metric_name_lit = syn2::Lit::parse(input)?;

let metric_name = match metric_name_lit {
syn::Lit::Str(lit_str) => {
syn2::Lit::Str(lit_str) => {
if lit_str.value().contains(' ') {
return Err(syn::Error::new(
return Err(syn2::Error::new(
proc_macro2::Span::call_site(),
"Spaces are not allowed. Use underscores '_'",
));
};
lit_str
}
_ => {
return Err(syn::Error::new(
return Err(syn2::Error::new(
proc_macro2::Span::call_site(),
"Must be a string literal. Format `[+]\"name_of_metric\"`.",
))
Expand Down Expand Up @@ -145,54 +141,91 @@ impl ToTokens for MetricSpec {
/// Ok(())
/// }
/// ```
#[proc_macro_error]
#[manyhow]
#[proc_macro_attribute]
pub fn metrics(attr: TokenStream, item: TokenStream) -> TokenStream {
let ItemFn {
let mut emitter = Emitter::new();

let Some(func): Option<syn2::ItemFn> = emitter.handle(syn2::parse2(item)) else {
return emitter.finish_token_stream();
};

// This is a good sanity check. Possibly redundant.
if func.sig.ident != "execute" {
emit!(
emitter,
func.sig.ident,
"Function should be an `impl execute`"
);
}

if func.sig.inputs.is_empty() {
emit!(
emitter,
func.sig,
"Function must have at least one argument of type `WorldStateView`."
);
return emitter.finish_token_stream();
}

let Some(metric_specs): Option<MetricSpecs> = emitter.handle(syn2::parse2(attr)) else {
return emitter.finish_token_stream();
};

let result = impl_metrics(&mut emitter, &metric_specs, &func);

emitter.finish_token_stream_with(result)
}

fn impl_metrics(emitter: &mut Emitter, _specs: &MetricSpecs, func: &syn2::ItemFn) -> TokenStream {
let syn2::ItemFn {
attrs,
vis,
sig,
block,
}: ItemFn = parse_macro_input!(item as ItemFn);
} = func;

// This is a good sanity check. Possibly redundant.
if sig.ident != "execute" {
abort!(sig.ident, "Function should be an `impl execute`");
}
match sig.output.clone() {
syn::ReturnType::Default => abort!(
syn2::ReturnType::Default => emit!(
emitter,
sig.output,
"`Fn` must return `Result`. Returns nothing instead. "
),
#[allow(clippy::string_to_string)]
syn::ReturnType::Type(_, typ) => match *typ {
syn2::ReturnType::Type(_, typ) => match *typ {
Type::Path(pth) => {
let Path { segments, .. } = pth.path;
let type_name = &segments.last().expect("non-empty path").ident;
if *type_name != "Result" {
abort!(
emit!(
emitter,
type_name,
format!("Should return `Result`. Found {type_name}")
"Should return `Result`. Found {}",
type_name
);
}
}
_ => abort!(
_ => emit!(
emitter,
typ,
"Should return `Result`. Non-path result type specification found"
),
},
}
if sig.inputs.is_empty() {
abort!(
sig,
"Function must have at least one argument of type `WorldStateView`."
);
}
let _specs = parse_macro_input!(attr as MetricSpecs);

// Again this may seem fragile, but if we move the metrics from
// the `WorldStateView`, we'd need to refactor many things anyway
let _metric_arg_ident = arg_metrics(&sig.inputs)
.unwrap_or_else(|args| abort!(args, "At least one argument must be a `WorldStateView`."));
let _metric_arg_ident = match arg_metrics(&sig.inputs) {
Ok(ident) => ident,
Err(args) => {
emit!(
emitter,
args,
"At least one argument must be a `WorldStateView`."
);
return quote!();
}
};

#[cfg(feature = "metric-instrumentation")]
let res = {
Expand All @@ -210,25 +243,23 @@ pub fn metrics(attr: TokenStream, item: TokenStream) -> TokenStream {
#successes
};
res
})
.into();
});
};

#[cfg(not(feature = "metric-instrumentation"))]
let res = quote!(
#(#attrs)* #vis #sig {
#block
}
)
.into();
);
res
}

#[cfg(feature = "metric-instrumentation")]
fn write_metrics(
metric_arg_ident: proc_macro2::Ident,
specs: MetricSpecs,
) -> (TokenStream2, TokenStream2, TokenStream2) {
) -> (TokenStream, TokenStream, TokenStream) {
let inc_metric = |spec: &MetricSpec, kind: &str| {
quote!(
#metric_arg_ident
Expand All @@ -246,17 +277,17 @@ fn write_metrics(
.observe((end_time.as_millis() - start_time.as_millis()) as f64);
)
};
let totals: TokenStream2 = specs
let totals: TokenStream = specs
.0
.iter()
.map(|spec| inc_metric(spec, "total"))
.collect();
let successes: TokenStream2 = specs
let successes: TokenStream = specs
.0
.iter()
.map(|spec| inc_metric(spec, "success"))
.collect();
let times: TokenStream2 = specs
let times: TokenStream = specs
.0
.iter()
.filter(|spec| spec.timing)
Expand Down
1 change: 1 addition & 0 deletions telemetry/derive/tests/ui_fail/not_execute.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use iroha_telemetry_derive::metrics;
use iroha_core::prelude::WorldStateView;

#[metrics(+"test_query", "another_test_query_without_timing")]
fn exequte(wsv: &WorldStateView) -> Result<(), ()> {
Expand Down
4 changes: 2 additions & 2 deletions telemetry/derive/tests/ui_fail/not_execute.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: Function should be an `impl execute`
--> tests/ui_fail/not_execute.rs:4:4
--> tests/ui_fail/not_execute.rs:5:4
|
4 | fn exequte(wsv: &WorldStateView) -> Result<(), ()> {
5 | fn exequte(wsv: &WorldStateView) -> Result<(), ()> {
| ^^^^^^^
7 changes: 5 additions & 2 deletions telemetry/derive/tests/ui_fail/not_return_result.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use iroha_telemetry_derive::metrics;
use iroha_core::prelude::WorldStateView;

type MyNotResult = Option<i32>;

#[metrics(+"test_query", "another_test_query_without_timing")]
fn execute(_wsv: &WorldStateView) -> iroha_core::RESULT {
Ok(())
fn execute(_wsv: &WorldStateView) -> MyNotResult {
None
}

fn main() {
Expand Down
8 changes: 4 additions & 4 deletions telemetry/derive/tests/ui_fail/not_return_result.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: Should return `Result`. Found RESULT
--> tests/ui_fail/not_return_result.rs:4:50
error: Should return `Result`. Found MyNotResult
--> tests/ui_fail/not_return_result.rs:7:38
|
4 | fn execute(_wsv: &WorldStateView) -> iroha_core::RESULT {
| ^^^^^^
7 | fn execute(_wsv: &WorldStateView) -> MyNotResult {
| ^^^^^^^^^^^
3 changes: 2 additions & 1 deletion telemetry/derive/tests/ui_fail/return_nothing.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use iroha_telemetry_derive::metrics;
use iroha_core::prelude::WorldStateView;

#[metrics(+"test_query", "another_test_query_without_timing")]
fn execute(wsv: &WorldStateView) {
Ok(())
Ok::<(), ()>(());
}

fn main() {
Expand Down
4 changes: 2 additions & 2 deletions telemetry/derive/tests/ui_fail/return_nothing.stderr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
error: `Fn` must return `Result`. Returns nothing instead.
--> tests/ui_fail/return_nothing.rs:3:1
--> tests/ui_fail/return_nothing.rs:4:1
|
3 | #[metrics(+"test_query", "another_test_query_without_timing")]
4 | #[metrics(+"test_query", "another_test_query_without_timing")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this error originates in the attribute macro `metrics` (in Nightly builds, run with -Z macro-backtrace for more info)
Loading