From e271f6054f5fc35c2e427a6264f002798449c7d0 Mon Sep 17 00:00:00 2001 From: C J Silverio Date: Mon, 27 Feb 2023 14:40:17 -0800 Subject: [PATCH 1/3] Bring the metrics-observer protobufs up to date Aligned the proto file in the observer sub-crate with the one in the TCP exporter. Updated the observer message handling code to use the new structures. This format de-sync will occur again if the writer code is changed without also updating the observer. The right solution here is to have the two crates share the protobuf file, but that requires more structural tinkering than a newcomer to the project should do :). Satisfied the clippy lints complaining about type complexity by creeating some subtypes. --- metrics-observer/build.rs | 2 +- metrics-observer/proto/event.proto | 27 +--- metrics-observer/src/metrics.rs | 199 ++++++++++++++++------------- 3 files changed, 117 insertions(+), 111 deletions(-) diff --git a/metrics-observer/build.rs b/metrics-observer/build.rs index 15559df8..7fe5a9ef 100644 --- a/metrics-observer/build.rs +++ b/metrics-observer/build.rs @@ -1,6 +1,6 @@ fn main() { println!("cargo:rerun-if-changed=proto/event.proto"); let mut prost_build = prost_build::Config::new(); - prost_build.btree_map(&["."]); + prost_build.btree_map(["."]); prost_build.compile_protos(&["proto/event.proto"], &["proto/"]).unwrap(); } diff --git a/metrics-observer/proto/event.proto b/metrics-observer/proto/event.proto index f24116a7..ccbe76ea 100644 --- a/metrics-observer/proto/event.proto +++ b/metrics-observer/proto/event.proto @@ -24,29 +24,16 @@ message Metric { string name = 1; google.protobuf.Timestamp timestamp = 2; map labels = 3; - oneof value { - Counter counter = 4; - Gauge gauge = 5; - Histogram histogram = 6; + oneof operation { + uint64 increment_counter = 4; + uint64 set_counter = 5; + double increment_gauge = 6; + double decrement_gauge = 7; + double set_gauge = 8; + double record_histogram = 9; } } -message Counter { - uint64 value = 1; -} - -message Gauge { - oneof value { - double absolute = 1; - double increment = 2; - double decrement = 3; - } -} - -message Histogram { - double value = 1; -} - message Event { oneof event { Metadata metadata = 1; diff --git a/metrics-observer/src/metrics.rs b/metrics-observer/src/metrics.rs index a5ff87e8..fcaf241f 100644 --- a/metrics-observer/src/metrics.rs +++ b/metrics-observer/src/metrics.rs @@ -16,11 +16,8 @@ mod proto { include!(concat!(env!("OUT_DIR"), "/event.proto.rs")); } -use self::proto::{ - event::Event, - metadata::{Description as DescriptionMetadata, MetricType, Unit as UnitMetadata}, - Event as EventWrapper, -}; +use self::proto::metadata::MetricType; +use proto::event::Event as ProtoEvent; #[derive(Clone)] pub enum ClientState { @@ -35,10 +32,14 @@ pub enum MetricData { Histogram(Summary), } +// Intermediate types used to satisfy clippy's dislike of complexity. +type MetadataKey = (MetricKind, String); +type MetdataValue = (Option, Option); +type MetadataStorage = Arc>>; pub struct Client { state: Arc>, metrics: Arc>>, - metadata: Arc, Option)>>>, + metadata: MetadataStorage, } impl Client { @@ -93,7 +94,7 @@ struct Runner { addr: String, client_state: Arc>, metrics: Arc>>, - metadata: Arc, Option)>>>, + metadata: MetadataStorage, } impl Runner { @@ -101,7 +102,7 @@ impl Runner { addr: String, state: Arc>, metrics: Arc>>, - metadata: Arc, Option)>>>, + metadata: MetadataStorage, ) -> Runner { Runner { state: RunnerState::Disconnected, addr, client_state: state, metrics, metadata } } @@ -172,98 +173,116 @@ impl Runner { Err(e) => eprintln!("read error: {:?}", e), }; - let event = match EventWrapper::decode_length_delimited(&mut buf) { + let message = match self::proto::Event::decode_length_delimited(&mut buf) { Err(e) => { eprintln!("decode error: {:?}", e); continue; } - Ok(event) => event, + Ok(v) => v, }; - if let Some(event) = event.event { - match event { - Event::Metadata(metadata) => { - let metric_type = MetricType::from_i32(metadata.metric_type) - .expect("unknown metric type over wire"); - let metric_kind = match metric_type { - MetricType::Counter => MetricKind::Counter, - MetricType::Gauge => MetricKind::Gauge, - MetricType::Histogram => MetricKind::Histogram, - }; - let key = (metric_kind, metadata.name); - let mut mmap = self - .metadata - .write() - .expect("failed to get metadata write lock"); - let entry = mmap.entry(key).or_insert((None, None)); - let (uentry, dentry) = entry; - *uentry = metadata - .unit - .map(|u| match u { - UnitMetadata::UnitValue(us) => us, - }) - .and_then(|s| Unit::from_string(s.as_str())); - *dentry = metadata.description.map(|d| match d { - DescriptionMetadata::DescriptionValue(ds) => ds, - }); - } - Event::Metric(metric) => { - let mut labels_raw = - metric.labels.into_iter().collect::>(); - labels_raw.sort_by(|a, b| a.0.cmp(&b.0)); - let labels = labels_raw - .into_iter() - .map(|(k, v)| Label::new(k, v)) - .collect::>(); - let key_data: Key = (metric.name, labels).into(); + let event = match message.event { + Some(e) => e, + None => continue, + }; + + match event { + ProtoEvent::Metadata(metadata) => { + let metric_type = MetricType::from_i32(metadata.metric_type) + .expect("unknown metric type over wire"); + let metric_kind = match metric_type { + MetricType::Counter => MetricKind::Counter, + MetricType::Gauge => MetricKind::Gauge, + MetricType::Histogram => MetricKind::Histogram, + }; + let key = (metric_kind, metadata.name); + let mut mmap = self + .metadata + .write() + .expect("failed to get metadata write lock"); + let entry = mmap.entry(key).or_insert((None, None)); + let (uentry, dentry) = entry; + *uentry = metadata + .unit + .map(|u| match u { + proto::metadata::Unit::UnitValue(u) => u, + }) + .and_then(|s| Unit::from_string(s.as_str())); + *dentry = metadata.description.map(|d| match d { + proto::metadata::Description::DescriptionValue(ds) => ds, + }); + } + ProtoEvent::Metric(metric) => { + let mut labels_raw = metric.labels.into_iter().collect::>(); + labels_raw.sort_by(|a, b| a.0.cmp(&b.0)); + let labels = labels_raw + .into_iter() + .map(|(k, v)| Label::new(k, v)) + .collect::>(); + let key_data: Key = (metric.name, labels).into(); - match metric.value.expect("no metric value") { - proto::metric::Value::Counter(value) => { - let key = - CompositeKey::new(MetricKind::Counter, key_data); - let mut metrics = self.metrics.write().unwrap(); - let counter = metrics - .entry(key) - .or_insert_with(|| MetricData::Counter(0)); - if let MetricData::Counter(inner) = counter { - *inner += value.value; - } + match metric.operation.expect("no metric operation") { + proto::metric::Operation::IncrementCounter(value) => { + let key = CompositeKey::new(MetricKind::Counter, key_data); + let mut metrics = self.metrics.write().unwrap(); + let counter = metrics + .entry(key) + .or_insert_with(|| MetricData::Counter(0)); + if let MetricData::Counter(inner) = counter { + *inner += value; + } + } + proto::metric::Operation::SetCounter(value) => { + let key = CompositeKey::new(MetricKind::Counter, key_data); + let mut metrics = self.metrics.write().unwrap(); + let counter = metrics + .entry(key) + .or_insert_with(|| MetricData::Counter(0)); + if let MetricData::Counter(inner) = counter { + *inner = value; } - proto::metric::Value::Gauge(value) => { - let key = - CompositeKey::new(MetricKind::Gauge, key_data); - let mut metrics = self.metrics.write().unwrap(); - let gauge = metrics - .entry(key) - .or_insert_with(|| MetricData::Gauge(0.0)); - if let MetricData::Gauge(inner) = gauge { - match value.value { - Some(proto::gauge::Value::Absolute(val)) => { - *inner = val - } - Some(proto::gauge::Value::Increment(val)) => { - *inner += val - } - Some(proto::gauge::Value::Decrement(val)) => { - *inner -= val - } - None => {} - } - } + } + proto::metric::Operation::IncrementGauge(value) => { + let key = CompositeKey::new(MetricKind::Gauge, key_data); + let mut metrics = self.metrics.write().unwrap(); + let gauge = metrics + .entry(key) + .or_insert_with(|| MetricData::Gauge(0.0)); + if let MetricData::Gauge(inner) = gauge { + *inner += value; + } + } + proto::metric::Operation::DecrementGauge(value) => { + let key = CompositeKey::new(MetricKind::Gauge, key_data); + let mut metrics = self.metrics.write().unwrap(); + let gauge = metrics + .entry(key) + .or_insert_with(|| MetricData::Gauge(0.0)); + if let MetricData::Gauge(inner) = gauge { + *inner -= value; } - proto::metric::Value::Histogram(value) => { - let key = - CompositeKey::new(MetricKind::Histogram, key_data); - let mut metrics = self.metrics.write().unwrap(); - let histogram = - metrics.entry(key).or_insert_with(|| { - let summary = Summary::with_defaults(); - MetricData::Histogram(summary) - }); + } + proto::metric::Operation::SetGauge(value) => { + let key = CompositeKey::new(MetricKind::Gauge, key_data); + let mut metrics = self.metrics.write().unwrap(); + let gauge = metrics + .entry(key) + .or_insert_with(|| MetricData::Gauge(0.0)); + if let MetricData::Gauge(inner) = gauge { + *inner = value; + } + } + proto::metric::Operation::RecordHistogram(value) => { + let key = + CompositeKey::new(MetricKind::Histogram, key_data); + let mut metrics = self.metrics.write().unwrap(); + let histogram = metrics.entry(key).or_insert_with(|| { + let summary = Summary::with_defaults(); + MetricData::Histogram(summary) + }); - if let MetricData::Histogram(inner) = histogram { - inner.add(value.value); - } + if let MetricData::Histogram(inner) = histogram { + inner.add(value); } } } From c2e66f0f40ac9b7dbbfabc20caf537e3f9e3d2d7 Mon Sep 17 00:00:00 2001 From: C J Silverio Date: Mon, 13 Mar 2023 13:20:16 -0700 Subject: [PATCH 2/3] Clean up qualified type names. The prost generated message wrapper type is named `Event`, and this overlaps with our metrics Event type that is in its payload. Named the wrapper `ProstMessage` to make its meaning more clear. This one is only used once and might reasonably be used fully-qualified inline. Also, fixed the typo in MetadataValue. --- metrics-observer/src/metrics.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/metrics-observer/src/metrics.rs b/metrics-observer/src/metrics.rs index fcaf241f..ff6e9400 100644 --- a/metrics-observer/src/metrics.rs +++ b/metrics-observer/src/metrics.rs @@ -16,8 +16,12 @@ mod proto { include!(concat!(env!("OUT_DIR"), "/event.proto.rs")); } -use self::proto::metadata::MetricType; -use proto::event::Event as ProtoEvent; +// These two Event types are distinct types. The first is the type generated by prost. +// It is the protobufs message, with decoding functions. The second is our metrics event. +use proto::Event as ProstMessage; +use proto::event::Event; +use proto::metric::Operation; +use proto::metadata::MetricType; #[derive(Clone)] pub enum ClientState { @@ -34,8 +38,8 @@ pub enum MetricData { // Intermediate types used to satisfy clippy's dislike of complexity. type MetadataKey = (MetricKind, String); -type MetdataValue = (Option, Option); -type MetadataStorage = Arc>>; +type MetadataValue = (Option, Option); +type MetadataStorage = Arc>>; pub struct Client { state: Arc>, metrics: Arc>>, @@ -173,7 +177,7 @@ impl Runner { Err(e) => eprintln!("read error: {:?}", e), }; - let message = match self::proto::Event::decode_length_delimited(&mut buf) { + let message = match ProstMessage::decode_length_delimited(&mut buf) { Err(e) => { eprintln!("decode error: {:?}", e); continue; @@ -187,7 +191,7 @@ impl Runner { }; match event { - ProtoEvent::Metadata(metadata) => { + Event::Metadata(metadata) => { let metric_type = MetricType::from_i32(metadata.metric_type) .expect("unknown metric type over wire"); let metric_kind = match metric_type { @@ -212,7 +216,7 @@ impl Runner { proto::metadata::Description::DescriptionValue(ds) => ds, }); } - ProtoEvent::Metric(metric) => { + Event::Metric(metric) => { let mut labels_raw = metric.labels.into_iter().collect::>(); labels_raw.sort_by(|a, b| a.0.cmp(&b.0)); let labels = labels_raw @@ -222,7 +226,7 @@ impl Runner { let key_data: Key = (metric.name, labels).into(); match metric.operation.expect("no metric operation") { - proto::metric::Operation::IncrementCounter(value) => { + Operation::IncrementCounter(value) => { let key = CompositeKey::new(MetricKind::Counter, key_data); let mut metrics = self.metrics.write().unwrap(); let counter = metrics @@ -232,7 +236,7 @@ impl Runner { *inner += value; } } - proto::metric::Operation::SetCounter(value) => { + Operation::SetCounter(value) => { let key = CompositeKey::new(MetricKind::Counter, key_data); let mut metrics = self.metrics.write().unwrap(); let counter = metrics @@ -242,7 +246,7 @@ impl Runner { *inner = value; } } - proto::metric::Operation::IncrementGauge(value) => { + Operation::IncrementGauge(value) => { let key = CompositeKey::new(MetricKind::Gauge, key_data); let mut metrics = self.metrics.write().unwrap(); let gauge = metrics @@ -252,7 +256,7 @@ impl Runner { *inner += value; } } - proto::metric::Operation::DecrementGauge(value) => { + Operation::DecrementGauge(value) => { let key = CompositeKey::new(MetricKind::Gauge, key_data); let mut metrics = self.metrics.write().unwrap(); let gauge = metrics @@ -262,7 +266,7 @@ impl Runner { *inner -= value; } } - proto::metric::Operation::SetGauge(value) => { + Operation::SetGauge(value) => { let key = CompositeKey::new(MetricKind::Gauge, key_data); let mut metrics = self.metrics.write().unwrap(); let gauge = metrics @@ -272,7 +276,7 @@ impl Runner { *inner = value; } } - proto::metric::Operation::RecordHistogram(value) => { + Operation::RecordHistogram(value) => { let key = CompositeKey::new(MetricKind::Histogram, key_data); let mut metrics = self.metrics.write().unwrap(); From 50aaa4e88625f4e01c2e89ad2a5976d9e7853289 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Wed, 5 Apr 2023 10:43:53 -0400 Subject: [PATCH 3/3] stylistic changes --- metrics-exporter-tcp/build.rs | 2 +- metrics-observer/src/metrics.rs | 20 +++++++------------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/metrics-exporter-tcp/build.rs b/metrics-exporter-tcp/build.rs index 15559df8..7fe5a9ef 100644 --- a/metrics-exporter-tcp/build.rs +++ b/metrics-exporter-tcp/build.rs @@ -1,6 +1,6 @@ fn main() { println!("cargo:rerun-if-changed=proto/event.proto"); let mut prost_build = prost_build::Config::new(); - prost_build.btree_map(&["."]); + prost_build.btree_map(["."]); prost_build.compile_protos(&["proto/event.proto"], &["proto/"]).unwrap(); } diff --git a/metrics-observer/src/metrics.rs b/metrics-observer/src/metrics.rs index ff6e9400..83c2a45f 100644 --- a/metrics-observer/src/metrics.rs +++ b/metrics-observer/src/metrics.rs @@ -16,12 +16,10 @@ mod proto { include!(concat!(env!("OUT_DIR"), "/event.proto.rs")); } -// These two Event types are distinct types. The first is the type generated by prost. -// It is the protobufs message, with decoding functions. The second is our metrics event. -use proto::Event as ProstMessage; -use proto::event::Event; -use proto::metric::Operation; -use proto::metadata::MetricType; +use proto::{event::Event, metadata::MetricType, metric::Operation, Event as ProstMessage}; + +type MetadataKey = (MetricKind, String); +type MetadataValue = (Option, Option); #[derive(Clone)] pub enum ClientState { @@ -36,14 +34,10 @@ pub enum MetricData { Histogram(Summary), } -// Intermediate types used to satisfy clippy's dislike of complexity. -type MetadataKey = (MetricKind, String); -type MetadataValue = (Option, Option); -type MetadataStorage = Arc>>; pub struct Client { state: Arc>, metrics: Arc>>, - metadata: MetadataStorage, + metadata: Arc>>, } impl Client { @@ -98,7 +92,7 @@ struct Runner { addr: String, client_state: Arc>, metrics: Arc>>, - metadata: MetadataStorage, + metadata: Arc>>, } impl Runner { @@ -106,7 +100,7 @@ impl Runner { addr: String, state: Arc>, metrics: Arc>>, - metadata: MetadataStorage, + metadata: Arc>>, ) -> Runner { Runner { state: RunnerState::Disconnected, addr, client_state: state, metrics, metadata } }