feat: produce measurement with attributes #43
Conversation
| || name.starts_with(METRIC_PREFIX_MONOTONIC_COUNTER) | ||
| || name.starts_with(METRIC_PREFIX_HISTOGRAM) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| impl<S> Layer<S> for MetricsLayer | ||
| where | ||
| S: Subscriber + for<'span> LookupSpan<'span>, | ||
| { | ||
| fn on_event(&self, event: &tracing::Event<'_>, _ctx: Context<'_, S>) { |
There was a problem hiding this comment.
To avoid allocating each time on_event() is called, we determine if the event has a metrics field.
However, this caused RwLock::read() to be called for each on_event().
I am concerned that an implementation that gets read lock for every on_event may not be acceptable.
There was a problem hiding this comment.
Yeah we may want to see if there is another way, or have this be an opt-in for non-performance sensitive cases
There was a problem hiding this comment.
I will try per layer filtering
| pub(crate) instruments: &'a Instruments, | ||
| pub(crate) meter: &'a Meter, | ||
| attributes: &'a mut SmallVec<[KeyValue; 8]>, | ||
| visited_metrics: &'a mut SmallVec<[(&'static str, InstrumentType); 2]>, |
There was a problem hiding this comment.
SmallVec size has no specific basis.
|
@jtescher |
ymgyt
force-pushed
the
metrics-attributes
branch
from
125bc1f to
c365058
Compare
ymgyt
force-pushed
the
metrics-attributes
branch
from
2992efc to
3cfc96f
Compare
ymgyt
force-pushed
the
metrics-attributes
branch
from
262e15a to
2c0dc3a
Compare
|
Here are the benchmark results on my local machine
|
Cargo.toml
Outdated
| @@ -35,6 +35,7 @@ once_cell = "1.13.0" | |||
| # Fix minimal-versions | |||
| async-trait = { version = "0.1.56", optional = true } | |||
| thiserror = { version = "1.0.31", optional = true } | |||
| smallvec = "1.0" | |||
There was a problem hiding this comment.
nit: can this be option and enabled with the metrics feature?
|
@jtescher Thanks for the review, I made smallvec optional I will update the document with follow up PR |
|
Looks great, thanks @ymgyt |
|
Nice work! |
## Motivation * fix broken link in doc * update histogram metrics field prefix (`value` -> `histogram`) * add metrics layer attributes description #43
Motivation
Support #32
This change will allow user to associate Attributes with metrics from tracing.
Solution
counter, so I have utilized the tracing_subscriber Filter trait.Counter:add()).SmallVecis used to hold Attribute KeyValues, Metrics names, and Instruments for invocation, aiming to avoid allocations in typical use cases.