Skip to content

Commit

Permalink
Change description to be SharedString (#312)
Browse files Browse the repository at this point in the history
  • Loading branch information
fredr authored Jul 7, 2022
1 parent 1af6758 commit 4d92d07
Show file tree
Hide file tree
Showing 17 changed files with 135 additions and 127 deletions.
8 changes: 4 additions & 4 deletions metrics-benchmark/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use hdrhistogram::Histogram as HdrHistogram;
use log::{error, info};
use metrics::{
gauge, histogram, increment_counter, register_counter, register_gauge, register_histogram,
Counter, Gauge, Histogram, Key, KeyName, Recorder, Unit,
Counter, Gauge, Histogram, Key, KeyName, Recorder, SharedString, Unit,
};
use metrics_util::registry::{AtomicStorage, Registry};
use quanta::{Clock, Instant as QuantaInstant};
Expand Down Expand Up @@ -62,11 +62,11 @@ impl BenchmarkingRecorder {
}

impl Recorder for BenchmarkingRecorder {
fn describe_counter(&self, _: KeyName, _: Option<Unit>, _: &'static str) {}
fn describe_counter(&self, _: KeyName, _: Option<Unit>, _: SharedString) {}

fn describe_gauge(&self, _: KeyName, _: Option<Unit>, _: &'static str) {}
fn describe_gauge(&self, _: KeyName, _: Option<Unit>, _: SharedString) {}

fn describe_histogram(&self, _: KeyName, _: Option<Unit>, _: &'static str) {}
fn describe_histogram(&self, _: KeyName, _: Option<Unit>, _: SharedString) {}

fn register_counter(&self, key: &Key) -> Counter {
self.registry.get_or_create_counter(key, |c| Counter::from_arc(c.clone()))
Expand Down
2 changes: 1 addition & 1 deletion metrics-exporter-prometheus/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,7 @@ mod tests {
let key_name = KeyName::from("yee_haw:lets go");
let key = Key::from_name(key_name.clone())
.with_extra_labels(vec![Label::new("øhno", "\"yeet\nies\\\"")]);
recorder.describe_counter(key_name, None, "\"Simplë stuff.\nRëally.\"");
recorder.describe_counter(key_name, None, "\"Simplë stuff.\nRëally.\"".into());
let counter1 = recorder.register_counter(&key);
counter1.increment(1);

Expand Down
12 changes: 6 additions & 6 deletions metrics-exporter-prometheus/src/recorder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::sync::atomic::Ordering;
use std::sync::Arc;

use indexmap::IndexMap;
use metrics::{Counter, Gauge, Histogram, Key, KeyName, Recorder, Unit};
use metrics::{Counter, Gauge, Histogram, Key, KeyName, Recorder, SharedString, Unit};
use metrics_util::registry::{Recency, Registry};
use parking_lot::RwLock;
use quanta::Instant;
Expand All @@ -20,7 +20,7 @@ pub(crate) struct Inner {
pub recency: Recency<Key>,
pub distributions: RwLock<HashMap<String, IndexMap<Vec<String>, Distribution>>>,
pub distribution_builder: DistributionBuilder,
pub descriptions: RwLock<HashMap<String, &'static str>>,
pub descriptions: RwLock<HashMap<String, SharedString>>,
pub global_labels: IndexMap<String, String>,
}

Expand Down Expand Up @@ -213,7 +213,7 @@ impl PrometheusRecorder {
PrometheusHandle { inner: self.inner.clone() }
}

fn add_description_if_missing(&self, key_name: &KeyName, description: &'static str) {
fn add_description_if_missing(&self, key_name: &KeyName, description: SharedString) {
let sanitized = sanitize_metric_name(key_name.as_str());
let mut descriptions = self.inner.descriptions.write();
descriptions.entry(sanitized).or_insert(description);
Expand All @@ -227,19 +227,19 @@ impl From<Inner> for PrometheusRecorder {
}

impl Recorder for PrometheusRecorder {
fn describe_counter(&self, key_name: KeyName, _unit: Option<Unit>, description: &'static str) {
fn describe_counter(&self, key_name: KeyName, _unit: Option<Unit>, description: SharedString) {
self.add_description_if_missing(&key_name, description);
}

fn describe_gauge(&self, key_name: KeyName, _unit: Option<Unit>, description: &'static str) {
fn describe_gauge(&self, key_name: KeyName, _unit: Option<Unit>, description: SharedString) {
self.add_description_if_missing(&key_name, description);
}

fn describe_histogram(
&self,
key_name: KeyName,
_unit: Option<Unit>,
description: &'static str,
description: SharedString,
) {
self.add_description_if_missing(&key_name, description);
}
Expand Down
23 changes: 12 additions & 11 deletions metrics-exporter-tcp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ use bytes::Bytes;
use crossbeam_channel::{bounded, unbounded, Receiver, Sender};
use metrics::{
Counter, CounterFn, Gauge, GaugeFn, Histogram, HistogramFn, Key, KeyName, Recorder,
SetRecorderError, Unit,
SetRecorderError, SharedString, Unit,
};
use mio::{
net::{TcpListener, TcpStream},
Expand Down Expand Up @@ -93,7 +93,7 @@ enum MetricOperation {
}

enum Event {
Metadata(KeyName, MetricType, Option<Unit>, &'static str),
Metadata(KeyName, MetricType, Option<Unit>, SharedString),
Metric(Key, MetricOperation),
}

Expand Down Expand Up @@ -152,7 +152,7 @@ impl State {
key_name: KeyName,
metric_type: MetricType,
unit: Option<Unit>,
description: &'static str,
description: SharedString,
) {
let _ = self.tx.try_send(Event::Metadata(key_name, metric_type, unit, description));
self.wake();
Expand Down Expand Up @@ -303,15 +303,15 @@ impl Default for TcpBuilder {
}

impl Recorder for TcpRecorder {
fn describe_counter(&self, key_name: KeyName, unit: Option<Unit>, description: &'static str) {
fn describe_counter(&self, key_name: KeyName, unit: Option<Unit>, description: SharedString) {
self.state.register_metric(key_name, MetricType::Counter, unit, description);
}

fn describe_gauge(&self, key_name: KeyName, unit: Option<Unit>, description: &'static str) {
fn describe_gauge(&self, key_name: KeyName, unit: Option<Unit>, description: SharedString) {
self.state.register_metric(key_name, MetricType::Gauge, unit, description);
}

fn describe_histogram(&self, key_name: KeyName, unit: Option<Unit>, description: &'static str) {
fn describe_histogram(&self, key_name: KeyName, unit: Option<Unit>, description: SharedString) {
self.state.register_metric(key_name, MetricType::Histogram, unit, description);
}

Expand Down Expand Up @@ -501,12 +501,13 @@ fn run_transport(

#[allow(clippy::mutable_key_type)]
fn generate_metadata_messages(
metadata: &HashMap<KeyName, (MetricType, Option<Unit>, Option<&'static str>)>,
metadata: &HashMap<KeyName, (MetricType, Option<Unit>, Option<SharedString>)>,
) -> VecDeque<Bytes> {
let mut bufs = VecDeque::new();
for (key_name, (metric_type, unit, desc)) in metadata.iter() {
let msg = convert_metadata_to_protobuf_encoded(key_name, *metric_type, *unit, *desc)
.expect("failed to encode metadata buffer");
let msg =
convert_metadata_to_protobuf_encoded(key_name, *metric_type, *unit, desc.as_ref())
.expect("failed to encode metadata buffer");
bufs.push_back(msg);
}
bufs
Expand Down Expand Up @@ -562,14 +563,14 @@ fn convert_metadata_to_protobuf_encoded(
key_name: &KeyName,
metric_type: MetricType,
unit: Option<Unit>,
desc: Option<&'static str>,
desc: Option<&SharedString>,
) -> Result<Bytes, EncodeError> {
let name = key_name.as_str().to_string();
let metadata = proto::Metadata {
name,
metric_type: metric_type.into(),
unit: unit.map(|u| proto::metadata::Unit::UnitValue(u.as_str().to_owned())),
description: desc.map(|d| proto::metadata::Description::DescriptionValue(d.to_owned())),
description: desc.map(|d| proto::metadata::Description::DescriptionValue(d.to_string())),
};
let event = proto::Event { event: Some(proto::event::Event::Metadata(metadata)) };

Expand Down
2 changes: 1 addition & 1 deletion metrics-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ fn get_describe_code(
{
// Only do this work if there's a recorder installed.
if let Some(recorder) = metrics::try_recorder() {
recorder.#describe_ident(#name.into(), #unit, #description);
recorder.#describe_ident(#name.into(), #unit, #description.into());
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions metrics-macros/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn test_get_describe_code() {
let expected = concat!(
"{ ",
"if let Some (recorder) = metrics :: try_recorder () { ",
"recorder . describe_mytype (\"mykeyname\" . into () , None , \"a counter\") ; ",
"recorder . describe_mytype (\"mykeyname\" . into () , None , \"a counter\" . into ()) ; ",
"} ",
"}",
);
Expand All @@ -38,7 +38,7 @@ fn test_get_describe_code_with_qualified_unit() {
let expected = concat!(
"{ ",
"if let Some (recorder) = metrics :: try_recorder () { ",
"recorder . describe_mytype (\"mykeyname\" . into () , Some (metrics :: Unit :: Nanoseconds) , \"a counter\") ; ",
"recorder . describe_mytype (\"mykeyname\" . into () , Some (metrics :: Unit :: Nanoseconds) , \"a counter\" . into ()) ; ",
"} ",
"}",
);
Expand All @@ -60,7 +60,7 @@ fn test_get_describe_code_with_relative_unit() {
let expected = concat!(
"{ ",
"if let Some (recorder) = metrics :: try_recorder () { ",
"recorder . describe_mytype (\"mykeyname\" . into () , Some (Unit :: Nanoseconds) , \"a counter\") ; ",
"recorder . describe_mytype (\"mykeyname\" . into () , Some (Unit :: Nanoseconds) , \"a counter\" . into ()) ; ",
"} ",
"}",
);
Expand All @@ -77,7 +77,7 @@ fn test_get_describe_code_with_constants() {
let expected = concat!(
"{ ",
"if let Some (recorder) = metrics :: try_recorder () { ",
"recorder . describe_mytype (KEY_NAME . into () , None , COUNTER_DESC) ; ",
"recorder . describe_mytype (KEY_NAME . into () , None , COUNTER_DESC . into ()) ; ",
"} ",
"}",
);
Expand All @@ -99,7 +99,7 @@ fn test_get_describe_code_with_constants_and_with_qualified_unit() {
let expected = concat!(
"{ ",
"if let Some (recorder) = metrics :: try_recorder () { ",
"recorder . describe_mytype (KEY_NAME . into () , Some (metrics :: Unit :: Nanoseconds) , COUNTER_DESC) ; ",
"recorder . describe_mytype (KEY_NAME . into () , Some (metrics :: Unit :: Nanoseconds) , COUNTER_DESC . into ()) ; ",
"} ",
"}",
);
Expand All @@ -121,7 +121,7 @@ fn test_get_describe_code_with_constants_and_with_relative_unit() {
let expected = concat!(
"{ ",
"if let Some (recorder) = metrics :: try_recorder () { ",
"recorder . describe_mytype (KEY_NAME . into () , Some (Unit :: Nanoseconds) , COUNTER_DESC) ; ",
"recorder . describe_mytype (KEY_NAME . into () , Some (Unit :: Nanoseconds) , COUNTER_DESC . into ()) ; ",
"} ",
"}",
);
Expand Down
8 changes: 4 additions & 4 deletions metrics-tracing-context/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
#![deny(missing_docs)]
#![cfg_attr(docsrs, feature(doc_cfg), deny(rustdoc::broken_intra_doc_links))]

use metrics::{Counter, Gauge, Histogram, Key, KeyName, Label, Recorder, Unit};
use metrics::{Counter, Gauge, Histogram, Key, KeyName, Label, Recorder, SharedString, Unit};
use metrics_util::layers::Layer;

pub mod label_filter;
Expand Down Expand Up @@ -203,15 +203,15 @@ where
R: Recorder,
F: LabelFilter,
{
fn describe_counter(&self, key_name: KeyName, unit: Option<Unit>, description: &'static str) {
fn describe_counter(&self, key_name: KeyName, unit: Option<Unit>, description: SharedString) {
self.inner.describe_counter(key_name, unit, description)
}

fn describe_gauge(&self, key_name: KeyName, unit: Option<Unit>, description: &'static str) {
fn describe_gauge(&self, key_name: KeyName, unit: Option<Unit>, description: SharedString) {
self.inner.describe_gauge(key_name, unit, description)
}

fn describe_histogram(&self, key_name: KeyName, unit: Option<Unit>, description: &'static str) {
fn describe_histogram(&self, key_name: KeyName, unit: Option<Unit>, description: SharedString) {
self.inner.describe_histogram(key_name, unit, description)
}

Expand Down
30 changes: 14 additions & 16 deletions metrics-util/src/debugging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::registry::AtomicStorage;
use crate::{kind::MetricKind, registry::Registry, CompositeKey};

use indexmap::IndexMap;
use metrics::{Counter, Gauge, Histogram, Key, KeyName, Recorder, Unit};
use metrics::{Counter, Gauge, Histogram, Key, KeyName, Recorder, SharedString, Unit};
use ordered_float::OrderedFloat;

thread_local! {
Expand All @@ -34,22 +34,22 @@ impl CompositeKeyName {
}

/// A point-in-time snapshot of all metrics in [`DebuggingRecorder`].
pub struct Snapshot(Vec<(CompositeKey, Option<Unit>, Option<&'static str>, DebugValue)>);
pub struct Snapshot(Vec<(CompositeKey, Option<Unit>, Option<SharedString>, DebugValue)>);

impl Snapshot {
/// Converts this snapshot to a mapping of metric data, keyed by the metric key itself.
#[allow(clippy::mutable_key_type)]
pub fn into_hashmap(
self,
) -> HashMap<CompositeKey, (Option<Unit>, Option<&'static str>, DebugValue)> {
) -> HashMap<CompositeKey, (Option<Unit>, Option<SharedString>, DebugValue)> {
self.0
.into_iter()
.map(|(k, unit, desc, value)| (k, (unit, desc, value)))
.collect::<HashMap<_, _>>()
}

/// Converts this snapshot to a vector of metric data tuples.
pub fn into_vec(self) -> Vec<(CompositeKey, Option<Unit>, Option<&'static str>, DebugValue)> {
pub fn into_vec(self) -> Vec<(CompositeKey, Option<Unit>, Option<SharedString>, DebugValue)> {
self.0
}
}
Expand All @@ -68,7 +68,7 @@ pub enum DebugValue {
struct Inner {
registry: Registry<Key, AtomicStorage>,
seen: Mutex<IndexMap<CompositeKey, ()>>,
metadata: Mutex<IndexMap<CompositeKeyName, (Option<Unit>, &'static str)>>,
metadata: Mutex<IndexMap<CompositeKeyName, (Option<Unit>, SharedString)>>,
}

impl Inner {
Expand Down Expand Up @@ -117,8 +117,7 @@ impl Snapshotter {
let ckn = CompositeKeyName::new(ck.kind(), ck.key().name().to_string().into());
let (unit, desc) = metadata
.get(&ckn)
.copied()
.map(|(u, d)| (u, Some(d)))
.map(|(u, d)| (u.to_owned(), Some(d.to_owned())))
.unwrap_or_else(|| (None, None));

// If there's no value for the key, that means the metric was only ever described, and
Expand Down Expand Up @@ -168,8 +167,7 @@ impl Snapshotter {
let ckn = CompositeKeyName::new(ck.kind(), ck.key().name().to_string().into());
let (unit, desc) = metadata
.get(&ckn)
.copied()
.map(|(u, d)| (u, Some(d)))
.map(|(u, d)| (u.to_owned(), Some(d.to_owned())))
.unwrap_or_else(|| (None, None));

// If there's no value for the key, that means the metric was only ever described, and
Expand Down Expand Up @@ -219,7 +217,7 @@ impl DebuggingRecorder {
Snapshotter { inner: Arc::clone(&self.inner) }
}

fn describe_metric(&self, rkey: CompositeKeyName, unit: Option<Unit>, desc: &'static str) {
fn describe_metric(&self, rkey: CompositeKeyName, unit: Option<Unit>, desc: SharedString) {
if self.is_per_thread {
PER_THREAD_INNER.with(|cell| {
// Create the inner state if it doesn't yet exist.
Expand All @@ -231,15 +229,15 @@ impl DebuggingRecorder {
let inner = maybe_inner.get_or_insert_with(Inner::new);

let mut metadata = inner.metadata.lock().expect("metadata lock poisoned");
let (uentry, dentry) = metadata.entry(rkey).or_insert((None, desc));
let (uentry, dentry) = metadata.entry(rkey).or_insert((None, desc.to_owned()));
if unit.is_some() {
*uentry = unit;
}
*dentry = desc;
*dentry = desc.to_owned();
});
} else {
let mut metadata = self.inner.metadata.lock().expect("metadata lock poisoned");
let (uentry, dentry) = metadata.entry(rkey).or_insert((None, desc));
let (uentry, dentry) = metadata.entry(rkey).or_insert((None, desc.to_owned()));
if unit.is_some() {
*uentry = unit;
}
Expand Down Expand Up @@ -274,17 +272,17 @@ impl DebuggingRecorder {
}

impl Recorder for DebuggingRecorder {
fn describe_counter(&self, key: KeyName, unit: Option<Unit>, description: &'static str) {
fn describe_counter(&self, key: KeyName, unit: Option<Unit>, description: SharedString) {
let ckey = CompositeKeyName::new(MetricKind::Counter, key);
self.describe_metric(ckey, unit, description);
}

fn describe_gauge(&self, key: KeyName, unit: Option<Unit>, description: &'static str) {
fn describe_gauge(&self, key: KeyName, unit: Option<Unit>, description: SharedString) {
let ckey = CompositeKeyName::new(MetricKind::Gauge, key);
self.describe_metric(ckey, unit, description);
}

fn describe_histogram(&self, key: KeyName, unit: Option<Unit>, description: &'static str) {
fn describe_histogram(&self, key: KeyName, unit: Option<Unit>, description: SharedString) {
let ckey = CompositeKeyName::new(MetricKind::Histogram, key);
self.describe_metric(ckey, unit, description);
}
Expand Down
Loading

0 comments on commit 4d92d07

Please sign in to comment.