Fix up all the review nits.

This commit is contained in:
Toby Lawrence 2019-06-01 15:55:28 -04:00
parent f14d645cfb
commit 0a3e8bbff6
No known key found for this signature in database
GPG Key ID: 3BB201B0EEE9212E
9 changed files with 68 additions and 38 deletions

View File

@ -71,9 +71,9 @@ impl Generator {
0
};
//let _ = self.stats.record_count("ok", 1);
let _ = self.stats.record_count("ok", 1);
let _ = self.stats.record_timing("ok", t0, t1);
//let _ = self.stats.record_gauge("total", self.gauge);
let _ = self.stats.record_gauge("total", self.gauge);
if start != 0 {
let delta = self.stats.now() - start;
@ -81,7 +81,7 @@ impl Generator {
// We also increment our global counter for the sample rate here.
self.rate_counter
.fetch_add(LOOP_SAMPLE * 1, Ordering::AcqRel);
.fetch_add(LOOP_SAMPLE * 3, Ordering::AcqRel);
}
}
@ -114,9 +114,9 @@ impl Generator {
0
};
//let _ = counter_handle.record(1);
let _ = counter_handle.record(1);
let _ = timing_handle.record_timing(t0, t1);
//let _ = gauge_handle.record(self.gauge);
let _ = gauge_handle.record(self.gauge);
if start != 0 {
let delta = self.stats.now() - start;
@ -124,7 +124,7 @@ impl Generator {
// We also increment our global counter for the sample rate here.
self.rate_counter
.fetch_add(LOOP_SAMPLE * 1, Ordering::AcqRel);
.fetch_add(LOOP_SAMPLE * 3, Ordering::AcqRel);
}
}

View File

@ -1,4 +1,4 @@
use crate::Receiver;
use crate::{config::Configuration, Receiver};
use std::error::Error;
use std::fmt;
use std::time::Duration;
@ -20,6 +20,9 @@ pub enum BuilderError {
/// that the read and write operations exclusively rely on. While this source is not as
/// up-to-date as the real clock, it is much faster to access.
UpkeepFailure,
#[doc(hidden)]
_NonExhaustive,
}
impl Error for BuilderError {}
@ -28,6 +31,7 @@ impl fmt::Display for BuilderError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
BuilderError::UpkeepFailure => write!(f, "failed to spawn quanta upkeep thread"),
BuilderError::_NonExhaustive => write!(f, "non-exhaustive matching"),
}
}
}
@ -37,6 +41,7 @@ impl fmt::Display for BuilderError {
pub struct Builder {
pub(crate) histogram_window: Duration,
pub(crate) histogram_granularity: Duration,
pub(crate) upkeep_interval: Duration,
}
impl Default for Builder {
@ -44,6 +49,7 @@ impl Default for Builder {
Self {
histogram_window: Duration::from_secs(10),
histogram_granularity: Duration::from_secs(1),
upkeep_interval: Duration::from_millis(50),
}
}
}
@ -70,8 +76,23 @@ impl Builder {
self
}
/// Sets the upkeep interval.
///
/// Defaults to 50 milliseconds.
///
/// This controls how often the time source, used internally by histograms, is updated with the
/// real time. For performance reasons, histograms use a sampled time source when they perform
/// checks to see if internal maintenance needs to occur. If the histogram granularity is set
/// very low, then this interval might need to be similarly reduced to make sure we're able to
/// update the time more often than histograms need to perform upkeep.
pub fn upkeep_interval(mut self, interval: Duration) -> Self {
self.upkeep_interval = interval;
self
}
/// Create a [`Receiver`] based on this configuration.
pub fn build(self) -> Result<Receiver, BuilderError> {
Receiver::from_builder(self)
let config = Configuration::from_builder(&self);
Receiver::from_config(config)
}
}

View File

@ -20,7 +20,10 @@ pub type MetricName = Cow<'static, str>;
/// See also: [Sink::scoped](crate::Sink::scoped).
#[derive(PartialEq, Eq, Hash, Clone)]
pub enum MetricScope {
/// Root scope.
Root,
/// A nested scope, with arbitrarily deep nesting.
Nested(Vec<String>),
}
@ -79,15 +82,15 @@ impl MetricValue {
}
}
pub(crate) fn new_counter() -> Self {
pub fn counter() -> Self {
Self::new(ValueState::Counter(AtomicU64::new(0)))
}
pub(crate) fn new_gauge() -> Self {
pub fn gauge() -> Self {
Self::new(ValueState::Gauge(AtomicI64::new(0)))
}
pub(crate) fn new_histogram(window: Duration, granularity: Duration, clock: Clock) -> Self {
pub fn histogram(window: Duration, granularity: Duration, clock: Clock) -> Self {
Self::new(ValueState::Histogram(AtomicWindowedHistogram::new(
window,
granularity,
@ -95,7 +98,7 @@ impl MetricValue {
)))
}
pub(crate) fn update_counter(&self, value: u64) {
pub fn update_counter(&self, value: u64) {
match self.state.deref() {
ValueState::Counter(inner) => {
inner.fetch_add(value, Ordering::Release);
@ -104,21 +107,21 @@ impl MetricValue {
}
}
pub(crate) fn update_gauge(&self, value: i64) {
pub fn update_gauge(&self, value: i64) {
match self.state.deref() {
ValueState::Gauge(inner) => inner.store(value, Ordering::Release),
_ => unreachable!("tried to access as gauge, not a gauge"),
}
}
pub(crate) fn update_histogram(&self, value: u64) {
pub fn update_histogram(&self, value: u64) {
match self.state.deref() {
ValueState::Histogram(inner) => inner.record(value),
_ => unreachable!("tried to access as histogram, not a histogram"),
}
}
pub(crate) fn snapshot(&self) -> ValueSnapshot {
pub fn snapshot(&self) -> ValueSnapshot {
match self.state.deref() {
ValueState::Counter(inner) => {
let value = inner.load(Ordering::Acquire);
@ -138,6 +141,10 @@ impl MetricValue {
/// Trait for types that represent time and can be subtracted from each other to generate a delta.
pub trait Delta {
/// Get the delta between this value and another value.
///
/// For `Instant`, we explicitly return the nanosecond difference. For `u64`, we return the
/// integer difference, but the timescale itself can be whatever the user desires.
fn delta(&self, other: Self) -> u64;
}
@ -203,14 +210,14 @@ mod tests {
#[test]
fn test_metric_values() {
let counter = MetricValue::new_counter();
let counter = MetricValue::counter();
counter.update_counter(42);
match counter.snapshot() {
ValueSnapshot::Counter(value) => assert_eq!(value, 42),
_ => panic!("incorrect value snapshot type for counter"),
}
let gauge = MetricValue::new_gauge();
let gauge = MetricValue::gauge();
gauge.update_gauge(23);
match gauge.snapshot() {
ValueSnapshot::Gauge(value) => assert_eq!(value, 23),
@ -219,7 +226,7 @@ mod tests {
let (mock, _) = Clock::mock();
let histogram =
MetricValue::new_histogram(Duration::from_secs(10), Duration::from_secs(1), mock);
MetricValue::histogram(Duration::from_secs(10), Duration::from_secs(1), mock);
histogram.update_histogram(8675309);
histogram.update_histogram(5551212);
match histogram.snapshot() {

View File

@ -2,16 +2,19 @@ use crate::Builder;
use std::time::Duration;
/// Holds the configuration for complex metric types.
pub(crate) struct MetricConfiguration {
#[derive(Clone)]
pub(crate) struct Configuration {
pub histogram_window: Duration,
pub histogram_granularity: Duration,
pub upkeep_interval: Duration,
}
impl MetricConfiguration {
impl Configuration {
pub fn from_builder(builder: &Builder) -> Self {
Self {
histogram_window: builder.histogram_window,
histogram_granularity: builder.histogram_granularity,
upkeep_interval: builder.upkeep_interval,
}
}
}

View File

@ -11,6 +11,9 @@ use std::sync::Arc;
pub enum SnapshotError {
/// The future was polled again after returning the snapshot.
AlreadyUsed,
#[doc(hidden)]
_NonExhaustive,
}
impl Error for SnapshotError {}
@ -19,6 +22,7 @@ impl fmt::Display for SnapshotError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
SnapshotError::AlreadyUsed => write!(f, "snapshot already returned from future"),
SnapshotError::_NonExhaustive => write!(f, "non-exhaustive matching"),
}
}
}

View File

@ -165,6 +165,8 @@
//!
//! [metrics_core]: https://docs.rs/metrics-core
//! [`Recorder`]: https://docs.rs/metrics-core/0.3.1/metrics_core/trait.Recorder.html
#![deny(missing_docs)]
#![warn(unused_extern_crates)]
mod builder;
mod common;
mod config;

View File

@ -1,14 +1,13 @@
use crate::{
builder::{Builder, BuilderError},
common::MetricScope,
config::MetricConfiguration,
config::Configuration,
control::Controller,
registry::{MetricRegistry, ScopeRegistry},
sink::Sink,
};
use quanta::{Builder as UpkeepBuilder, Clock, Handle as UpkeepHandle};
use std::sync::Arc;
use std::time::Duration;
/// Central store for metrics.
///
@ -23,23 +22,20 @@ pub struct Receiver {
}
impl Receiver {
pub(crate) fn from_builder(builder: Builder) -> Result<Receiver, BuilderError> {
pub(crate) fn from_config(config: Configuration) -> Result<Receiver, BuilderError> {
// Configure our clock and configure the quanta upkeep thread. The upkeep thread does that
// for us, and keeps us within `upkeep_interval` of the true time. The reads of this cache
// time are faster than calling the underlying time source directly, and for histogram
// windowing, we can afford to have a very granular value compared to the raw nanosecond
// precsion provided by quanta by default.
let clock = Clock::new();
let upkeep_interval = Duration::from_millis(50);
let upkeep = UpkeepBuilder::new_with_clock(upkeep_interval, clock.clone());
let upkeep = UpkeepBuilder::new_with_clock(config.upkeep_interval, clock.clone());
let _upkeep_handle = upkeep.start().map_err(|_| BuilderError::UpkeepFailure)?;
let metric_config = MetricConfiguration::from_builder(&builder);
let scope_registry = Arc::new(ScopeRegistry::new());
let metric_registry = Arc::new(MetricRegistry::new(
scope_registry.clone(),
metric_config,
config,
clock.clone(),
));

View File

@ -1,5 +1,5 @@
use crate::common::{MetricIdentifier, MetricKind, MetricValue};
use crate::config::MetricConfiguration;
use crate::config::Configuration;
use crate::data::Snapshot;
use crate::registry::ScopeRegistry;
use arc_swap::{ptr_eq, ArcSwap};
@ -11,16 +11,12 @@ use std::sync::Arc;
pub(crate) struct MetricRegistry {
scope_registry: Arc<ScopeRegistry>,
metrics: ArcSwap<HashMap<MetricIdentifier, MetricValue>>,
config: MetricConfiguration,
config: Configuration,
clock: Clock,
}
impl MetricRegistry {
pub fn new(
scope_registry: Arc<ScopeRegistry>,
config: MetricConfiguration,
clock: Clock,
) -> Self {
pub fn new(scope_registry: Arc<ScopeRegistry>, config: Configuration, clock: Clock) -> Self {
MetricRegistry {
scope_registry,
metrics: ArcSwap::new(Arc::new(HashMap::new())),
@ -39,9 +35,9 @@ impl MetricRegistry {
};
let value_handle = match kind {
MetricKind::Counter => MetricValue::new_counter(),
MetricKind::Gauge => MetricValue::new_gauge(),
MetricKind::Histogram => MetricValue::new_histogram(
MetricKind::Counter => MetricValue::counter(),
MetricKind::Gauge => MetricValue::gauge(),
MetricKind::Histogram => MetricValue::histogram(
self.config.histogram_window,
self.config.histogram_granularity,
self.clock.clone(),

View File

@ -37,6 +37,7 @@ impl fmt::Display for SinkError {
/// scope, to avoid needing to allocate in the case where we want to be able to specify multiple
/// scope levels in a single go.
pub trait AsScoped<'a> {
/// Creates a new [`MetricScope`] by adding `self` to the `base` scope.
fn as_scoped(&'a self, base: MetricScope) -> MetricScope;
}