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

Change description to be SharedString #312

Merged
merged 2 commits into from
Jul 7, 2022
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
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