fix(metrics): metrics derive separator (#625)

* fix(metrics): metrics derive separator

* fix existing metrics

* rm log

* static regex
This commit is contained in:
Roman Krasiuk
2022-12-27 13:16:41 +02:00
committed by GitHub
parent a2c1cdb399
commit 5bb14ecb7c
16 changed files with 201 additions and 108 deletions

1
Cargo.lock generated
View File

@ -3609,6 +3609,7 @@ dependencies = [
"once_cell",
"proc-macro2",
"quote",
"regex",
"serial_test",
"syn",
"trybuild",

View File

@ -14,8 +14,9 @@ proc-macro2 = "1.0"
syn = { version = "1.0", features = ["extra-traits"] }
quote = "1.0"
metrics = "0.20.1"
regex = "1.6.0"
once_cell = "1.15.0"
[dev-dependencies]
trybuild = "1.0"
once_cell = "1.15.0"
serial_test = "0.10"

View File

@ -1,4 +1,6 @@
use once_cell::sync::Lazy;
use quote::{quote, ToTokens};
use regex::Regex;
use syn::{
punctuated::Punctuated, Attribute, Data, DeriveInput, Error, Lit, LitStr, MetaNameValue,
Result, Token,
@ -6,6 +8,11 @@ use syn::{
use crate::{metric::Metric, with_attrs::WithAttrs};
/// Metric name regex according to Prometheus data model
/// https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
static METRIC_NAME_RE: Lazy<Regex> =
Lazy::new(|| Regex::new(r"^[a-zA-Z_:][a-zA-Z0-9_:]*$").unwrap());
pub(crate) fn derive(node: &DeriveInput) -> Result<proc_macro2::TokenStream> {
let ty = &node.ident;
let ident_name = ty.to_string();
@ -17,7 +24,7 @@ pub(crate) fn derive(node: &DeriveInput) -> Result<proc_macro2::TokenStream> {
.iter()
.map(|metric| {
let field_name = &metric.field.ident;
let register_stmt = metric.register_stmt(&metrics_attr.scope)?;
let register_stmt = metric.register_stmt(&metrics_attr)?;
Ok(quote! {
#field_name: #register_stmt,
})
@ -26,7 +33,7 @@ pub(crate) fn derive(node: &DeriveInput) -> Result<proc_macro2::TokenStream> {
let describe_stmts = metric_fields
.iter()
.map(|metric| metric.describe_stmt(&metrics_attr.scope))
.map(|metric| metric.describe_stmt(&metrics_attr))
.collect::<Result<Vec<_>>>()?;
Ok(quote! {
@ -56,23 +63,42 @@ pub(crate) fn derive(node: &DeriveInput) -> Result<proc_macro2::TokenStream> {
}
pub(crate) struct MetricsAttr {
scope: LitStr,
pub(crate) scope: LitStr,
pub(crate) separator: Option<LitStr>,
}
fn parse_metrics_attr(node: &DeriveInput) -> Result<MetricsAttr> {
let metrics_attr = parse_single_required_attr(node, "metrics")?;
let parsed =
metrics_attr.parse_args_with(Punctuated::<MetaNameValue, Token![,]>::parse_terminated)?;
let mut parsed_iter = parsed.into_iter();
if let Some(kv) = parsed_iter.next() {
if !kv.path.is_ident("scope") || parsed_iter.next().is_some() {
Err(Error::new_spanned(kv, "Only single `scope = ..` value is supported."))
let (mut scope, mut separator) = (None, None);
for kv in parsed {
if kv.path.is_ident("scope") {
if scope.is_some() {
return Err(Error::new_spanned(kv, "Duplicate `scope` value provided."))
}
let scope_lit = parse_str_lit(&kv.lit)?;
validate_metric_name(&scope_lit)?;
scope = Some(scope_lit);
} else if kv.path.is_ident("separator") {
if separator.is_some() {
return Err(Error::new_spanned(kv, "Duplicate `separator` value provided."))
}
let separator_lit = parse_str_lit(&kv.lit)?;
if separator_lit.value() != ":" && separator_lit.value() != "_" {
return Err(Error::new_spanned(
kv,
"Unsupported `separator` value. Supported: `:` and `_`.",
))
}
separator = Some(separator_lit);
} else {
Ok(MetricsAttr { scope: parse_str_lit(kv.lit)? })
return Err(Error::new_spanned(kv, "Unsupported attribute entry."))
}
} else {
Err(Error::new_spanned(metrics_attr, "`scope = ..` must be set."))
}
let scope = scope.ok_or(Error::new_spanned(node, "`scope = ..` must be set."))?;
Ok(MetricsAttr { scope, separator })
}
fn parse_metric_fields(node: &DeriveInput) -> Result<Vec<Metric<'_>>> {
@ -91,12 +117,14 @@ fn parse_metric_fields(node: &DeriveInput) -> Result<Vec<Metric<'_>>> {
if describe.is_some() {
return Err(Error::new_spanned(kv, "Duplicate `describe` value provided."))
}
describe = Some(parse_str_lit(kv.lit)?);
describe = Some(parse_str_lit(&kv.lit)?);
} else if kv.path.is_ident("rename") {
if rename.is_some() {
return Err(Error::new_spanned(kv, "Duplicate `rename` value provided."))
}
rename = Some(parse_str_lit(kv.lit)?)
let rename_lit = parse_str_lit(&kv.lit)?;
validate_metric_name(&rename_lit)?;
rename = Some(rename_lit)
} else {
return Err(Error::new_spanned(kv, "Unsupported attribute entry."))
}
@ -111,7 +139,7 @@ fn parse_metric_fields(node: &DeriveInput) -> Result<Vec<Metric<'_>>> {
None => {
return Err(Error::new_spanned(
field,
"Either doc comment or `describe` must be provided.",
"Either doc comment or `describe = ..` must be set.",
))
}
},
@ -123,6 +151,14 @@ fn parse_metric_fields(node: &DeriveInput) -> Result<Vec<Metric<'_>>> {
Ok(metrics)
}
fn validate_metric_name(name: &LitStr) -> Result<()> {
if METRIC_NAME_RE.is_match(&name.value()) {
Ok(())
} else {
Err(Error::new_spanned(name, format!("Value must match regex {}", METRIC_NAME_RE.as_str())))
}
}
fn parse_single_attr<'a, T: WithAttrs + ToTokens>(
token: &'a T,
ident: &str,
@ -171,9 +207,9 @@ fn parse_docs_to_string<T: WithAttrs>(token: &T) -> Result<Option<String>> {
Ok(doc_str)
}
fn parse_str_lit(lit: Lit) -> Result<LitStr> {
fn parse_str_lit(lit: &Lit) -> Result<LitStr> {
match lit {
Lit::Str(lit_str) => Ok(lit_str),
Lit::Str(lit_str) => Ok(lit_str.to_owned()),
_ => Err(Error::new_spanned(lit, "Value **must** be a string literal.")),
}
}

View File

@ -30,7 +30,7 @@ mod with_attrs;
/// use metrics::{Counter, Gauge, Histogram};
///
/// #[derive(Metrics)]
/// #[metrics(scope = "metrics.custom")]
/// #[metrics(scope = "metrics_custom")]
/// struct CustomMetrics {
/// /// A gauge with doc comment description.
/// gauge: Gauge,
@ -60,10 +60,10 @@ mod with_attrs;
/// impl Default for CustomMetrics {
/// fn default() -> Self {
/// Self {
/// gauge: metrics::register_gauge!("metrics.custom.gauge"),
/// gauge2: metrics::register_gauge!("metrics.custom.second_gauge"),
/// counter: metrics::register_counter!("metrics.custom.counter"),
/// histo: metrics::register_histogram!("metrics.custom.histogram"),
/// gauge: metrics::register_gauge!("metrics_custom_gauge"),
/// gauge2: metrics::register_gauge!("metrics_custom_second_gauge"),
/// counter: metrics::register_counter!("metrics_custom_counter"),
/// histo: metrics::register_histogram!("metrics_custom_histogram"),
/// }
/// }
/// }
@ -77,10 +77,10 @@ mod with_attrs;
/// impl CustomMetrics {
/// /// Describe all exposed metrics
/// pub fn describe() {
/// metrics::describe_gauge!("metrics.custom.gauge", "A gauge with doc comment description.");
/// metrics::describe_gauge!("metrics.custom.second_gauge", "A gauge with metric attribute description.");
/// metrics::describe_counter!("metrics.custom.counter", "Metric attribute description will be preffered over doc comment.");
/// metrics::describe_histogram!("metrics.custom.histogram", "A renamed histogram.");
/// metrics::describe_gauge!("metrics_custom_gauge", "A gauge with doc comment description.");
/// metrics::describe_gauge!("metrics_custom_second_gauge", "A gauge with metric attribute description.");
/// metrics::describe_counter!("metrics_custom_counter", "Metric attribute description will be preffered over doc comment.");
/// metrics::describe_histogram!("metrics_custom_histogram", "A renamed histogram.");
/// }
/// }
/// ```

View File

@ -1,6 +1,8 @@
use quote::quote;
use syn::{Error, Field, LitStr, Result, Type};
use crate::expand::MetricsAttr;
const COUNTER_TY: &str = "Counter";
const HISTOGRAM_TY: &str = "Histogram";
const GAUGE_TY: &str = "Gauge";
@ -12,21 +14,26 @@ pub(crate) struct Metric<'a> {
}
impl<'a> Metric<'a> {
const DEFAULT_SEPARATOR: &str = "_";
pub(crate) fn new(field: &'a Field, description: String, rename: Option<LitStr>) -> Self {
Self { field, description, rename }
}
pub(crate) fn metric_name(&self, scope: &LitStr) -> String {
let scope = scope.value();
pub(crate) fn metric_name(&self, config: &MetricsAttr) -> String {
let scope = config.scope.value();
let metric = match self.rename.as_ref() {
Some(name) => name.value(),
None => self.field.ident.as_ref().map(ToString::to_string).unwrap_or_default(),
};
format!("{scope}.{metric}")
match config.separator.as_ref() {
Some(separator) => format!("{scope}{}{metric}", separator.value()),
None => format!("{scope}{}{metric}", Metric::DEFAULT_SEPARATOR),
}
}
pub(crate) fn register_stmt(&self, scope: &LitStr) -> Result<proc_macro2::TokenStream> {
let metric_name = self.metric_name(scope);
pub(crate) fn register_stmt(&self, config: &MetricsAttr) -> Result<proc_macro2::TokenStream> {
let metric_name = self.metric_name(config);
if let Type::Path(ref path_ty) = self.field.ty {
if let Some(last) = path_ty.path.segments.last() {
@ -44,8 +51,8 @@ impl<'a> Metric<'a> {
Err(Error::new_spanned(&self.field.ty, "Unsupported metric type"))
}
pub(crate) fn describe_stmt(&self, scope: &LitStr) -> Result<proc_macro2::TokenStream> {
let metric_name = self.metric_name(scope);
pub(crate) fn describe_stmt(&self, config: &MetricsAttr) -> Result<proc_macro2::TokenStream> {
let metric_name = self.metric_name(config);
let description = &self.description;
if let Type::Path(ref path_ty) = self.field.ty {

View File

@ -7,55 +7,55 @@ use reth_metrics_derive::Metrics;
fn main() {}
#[derive(Metrics)]
#[metrics(scope = "some.scope")]
#[metrics(scope = "some_scope")]
struct CustomMetrics {
gauge: Gauge,
}
#[derive(Metrics)]
#[metrics(scope = "some.scope")]
#[metrics(scope = "some_scope")]
struct CustomMetrics2 {
#[metric()]
gauge: Gauge,
}
#[derive(Metrics)]
#[metrics(scope = "some.scope")]
#[metrics(scope = "some_scope")]
struct CustomMetrics3 {
#[metric(random = "value")]
gauge: Gauge,
}
#[derive(Metrics)]
#[metrics(scope = "some.scope")]
#[metrics(scope = "some_scope")]
struct CustomMetrics4 {
#[metric(describe = 123)]
gauge: Gauge,
}
#[derive(Metrics)]
#[metrics(scope = "some.scope")]
#[metrics(scope = "some_scope")]
struct CustomMetrics5 {
#[metric(rename = 123)]
gauge: Gauge,
}
#[derive(Metrics)]
#[metrics(scope = "some.scope")]
#[metrics(scope = "some_scope")]
struct CustomMetrics6 {
#[metric(describe = "", describe = "")]
gauge: Gauge,
}
#[derive(Metrics)]
#[metrics(scope = "some.scope")]
#[metrics(scope = "some_scope")]
struct CustomMetrics7 {
#[metric(rename = "_gauge", rename = "_gauge")]
gauge: Gauge,
}
#[derive(Metrics)]
#[metrics(scope = "some.scope")]
#[metrics(scope = "some_scope")]
struct CustomMetrics8 {
#[metric(describe = "")]
gauge: String,

View File

@ -1,10 +1,10 @@
error: Either doc comment or `describe` must be provided.
error: Either doc comment or `describe = ..` must be set.
--> tests/compile-fail/metric_attr.rs:12:5
|
12 | gauge: Gauge,
| ^^^^^^^^^^^^
error: Either doc comment or `describe` must be provided.
error: Either doc comment or `describe = ..` must be set.
--> tests/compile-fail/metric_attr.rs:18:5
|
18 | / #[metric()]

View File

@ -10,3 +10,43 @@ struct CustomMetrics;
#[metrics()]
#[metrics()]
struct CustomMetrics2;
#[derive(Metrics)]
#[metrics()]
struct CustomMetrics3;
#[derive(Metrics)]
#[metrics(scope = value)]
struct CustomMetrics4;
#[derive(Metrics)]
#[metrics(scope = 123)]
struct CustomMetrics5;
#[derive(Metrics)]
#[metrics(scope = "some.scope")]
struct CustomMetrics6;
#[derive(Metrics)]
#[metrics(scope = "some_scope", scope = "another_scope")]
struct CustomMetrics7;
#[derive(Metrics)]
#[metrics(separator = value)]
struct CustomMetrics8;
#[derive(Metrics)]
#[metrics(separator = 123)]
struct CustomMetrics9;
#[derive(Metrics)]
#[metrics(separator = ".")]
struct CustomMetrics10;
#[derive(Metrics)]
#[metrics(separator = "_", separator = ":")]
struct CustomMetrics11;
#[derive(Metrics)]
#[metrics(random = "value")]
struct CustomMetrics12;

View File

@ -9,3 +9,64 @@ error: Duplicate `#[metrics(..)]` attribute provided.
|
11 | #[metrics()]
| ^^^^^^^^^^^^
error: `scope = ..` must be set.
--> tests/compile-fail/metrics_attr.rs:15:1
|
15 | / #[metrics()]
16 | | struct CustomMetrics3;
| |______________________^
error: expected literal
--> tests/compile-fail/metrics_attr.rs:19:19
|
19 | #[metrics(scope = value)]
| ^^^^^
error: Value **must** be a string literal.
--> tests/compile-fail/metrics_attr.rs:23:19
|
23 | #[metrics(scope = 123)]
| ^^^
error: Value must match regex ^[a-zA-Z_:][a-zA-Z0-9_:]*$
--> tests/compile-fail/metrics_attr.rs:27:19
|
27 | #[metrics(scope = "some.scope")]
| ^^^^^^^^^^^^
error: Duplicate `scope` value provided.
--> tests/compile-fail/metrics_attr.rs:31:33
|
31 | #[metrics(scope = "some_scope", scope = "another_scope")]
| ^^^^^^^^^^^^^^^^^^^^^^^
error: expected literal
--> tests/compile-fail/metrics_attr.rs:35:23
|
35 | #[metrics(separator = value)]
| ^^^^^
error: Value **must** be a string literal.
--> tests/compile-fail/metrics_attr.rs:39:23
|
39 | #[metrics(separator = 123)]
| ^^^
error: Unsupported `separator` value. Supported: `:` and `_`.
--> tests/compile-fail/metrics_attr.rs:43:11
|
43 | #[metrics(separator = ".")]
| ^^^^^^^^^^^^^^^
error: Duplicate `separator` value provided.
--> tests/compile-fail/metrics_attr.rs:47:28
|
47 | #[metrics(separator = "_", separator = ":")]
| ^^^^^^^^^^^^^^^
error: Unsupported attribute entry.
--> tests/compile-fail/metrics_attr.rs:51:11
|
51 | #[metrics(random = "value")]
| ^^^^^^^^^^^^^^^^

View File

@ -1,24 +0,0 @@
extern crate reth_metrics_derive;
use reth_metrics_derive::Metrics;
fn main() {}
#[derive(Metrics)]
#[metrics()]
struct CustomMetrics;
#[derive(Metrics)]
#[metrics(scope = value)]
struct CustomMetrics2;
#[derive(Metrics)]
#[metrics(scope = 123)]
struct CustomMetrics3;
#[derive(Metrics)]
#[metrics(scope = "some.scope", scope = "another.scope")]
struct CustomMetrics4;
#[derive(Metrics)]
#[metrics(random = "value")]
struct CustomMetrics5;

View File

@ -1,29 +0,0 @@
error: `scope = ..` must be set.
--> tests/compile-fail/metrics_scope.rs:7:1
|
7 | #[metrics()]
| ^^^^^^^^^^^^
error: expected literal
--> tests/compile-fail/metrics_scope.rs:11:19
|
11 | #[metrics(scope = value)]
| ^^^^^
error: Value **must** be a string literal.
--> tests/compile-fail/metrics_scope.rs:15:19
|
15 | #[metrics(scope = 123)]
| ^^^
error: Only single `scope = ..` value is supported.
--> tests/compile-fail/metrics_scope.rs:19:11
|
19 | #[metrics(scope = "some.scope", scope = "another.scope")]
| ^^^^^^^^^^^^^^^^^^^^
error: Only single `scope = ..` value is supported.
--> tests/compile-fail/metrics_scope.rs:23:11
|
23 | #[metrics(random = "value")]
| ^^^^^^^^^^^^^^^^

View File

@ -9,7 +9,7 @@ use serial_test::serial;
#[allow(dead_code)]
#[derive(Metrics)]
#[metrics(scope = "metrics.custom")]
#[metrics(scope = "metrics_custom")]
struct CustomMetrics {
/// A gauge with doc comment description.
gauge: Gauge,
@ -34,7 +34,7 @@ fn describe_metrics() {
assert_eq!(RECORDER.metrics_len(), 4);
let gauge = RECORDER.get_metric("metrics.custom.gauge");
let gauge = RECORDER.get_metric("metrics_custom_gauge");
assert!(gauge.is_some());
assert_eq!(
gauge.unwrap(),
@ -44,7 +44,7 @@ fn describe_metrics() {
}
);
let second_gauge = RECORDER.get_metric("metrics.custom.second_gauge");
let second_gauge = RECORDER.get_metric("metrics_custom_second_gauge");
assert!(second_gauge.is_some());
assert_eq!(
second_gauge.unwrap(),
@ -54,7 +54,7 @@ fn describe_metrics() {
}
);
let counter = RECORDER.get_metric("metrics.custom.counter");
let counter = RECORDER.get_metric("metrics_custom_counter");
assert!(counter.is_some());
assert_eq!(
counter.unwrap(),
@ -66,7 +66,7 @@ fn describe_metrics() {
}
);
let histogram = RECORDER.get_metric("metrics.custom.histogram");
let histogram = RECORDER.get_metric("metrics_custom_histogram");
assert!(histogram.is_some());
assert_eq!(
histogram.unwrap(),
@ -88,19 +88,19 @@ fn register_metrics() {
assert_eq!(RECORDER.metrics_len(), 4);
let gauge = RECORDER.get_metric("metrics.custom.gauge");
let gauge = RECORDER.get_metric("metrics_custom_gauge");
assert!(gauge.is_some());
assert_eq!(gauge.unwrap(), TestMetric { ty: TestMetricTy::Gauge, description: None });
let second_gauge = RECORDER.get_metric("metrics.custom.second_gauge");
let second_gauge = RECORDER.get_metric("metrics_custom_second_gauge");
assert!(second_gauge.is_some());
assert_eq!(second_gauge.unwrap(), TestMetric { ty: TestMetricTy::Gauge, description: None });
let counter = RECORDER.get_metric("metrics.custom.counter");
let counter = RECORDER.get_metric("metrics_custom_counter");
assert!(counter.is_some());
assert_eq!(counter.unwrap(), TestMetric { ty: TestMetricTy::Counter, description: None });
let histogram = RECORDER.get_metric("metrics.custom.histogram");
let histogram = RECORDER.get_metric("metrics_custom_histogram");
assert!(histogram.is_some());
assert_eq!(histogram.unwrap(), TestMetric { ty: TestMetricTy::Histogram, description: None });

View File

@ -31,7 +31,7 @@ impl StageId {
tx: &impl DbTxMut<'db>,
block: BlockNumber,
) -> Result<(), DbError> {
absolute_counter!("stage.progress", block, "stage" => self.0);
absolute_counter!("stage_progress", block, "stage" => self.0);
tx.put::<SyncStage>(self.0.as_bytes().to_vec(), block)
}
}

View File

@ -12,7 +12,7 @@
//!
//! This library exposes metrics via the [`metrics`][metrics_core] crate:
//!
//! - `stage.progress{stage}`: The block number each stage has currently reached.
//! - `stage_progress{stage}`: The block number each stage has currently reached.
mod db;
mod error;

View File

@ -4,7 +4,7 @@ use reth_metrics_derive::Metrics;
/// Stagedsync header metrics
#[derive(Metrics)]
#[metrics(scope = "stages.headers")]
#[metrics(scope = "stages_headers")]
pub struct HeaderMetrics {
/// Number of headers successfully retrieved
pub headers_counter: Counter,

View File

@ -32,7 +32,7 @@ Each metric is identified by a [`Key`][metrics.Key], which itself is composed of
The `KeyName` represents the actual metric name, and the labels are used to further drill down into the metric.
For example, a metric that represents stage progress would have a key name of `stage.progress` and a `stage` label that can be used to get the progress of individual stages.
For example, a metric that represents stage progress would have a key name of `stage_progress` and a `stage` label that can be used to get the progress of individual stages.
There will only ever exist one description per metric `KeyName`; it is not possible to add a description for a label, or a `KeyName`/`Label` group.