feat: adding gauge to track total peers connected #80

gausnes · 2021-10-12T21:00:12Z

Adding a new gauge to monitor the number of peers connected to Wireguard. This metric provides low cardinality context to the number of peers connected to the process. Label cardinality becomes notable for DataDog pricing.

gausnes · 2021-10-12T21:47:32Z

Cargo.toml

@@ -29,7 +29,7 @@ thiserror		 = "1.0"
 hyper                    = { version       = "0.14", features = ["stream"] }
 http                     = "0.2"
 tokio                    = { version 	   = "1.0", features = ["macros", "rt"] }
-prometheus_exporter_base = { version       = "1.2", features = ["hyper_server"] }
+prometheus_exporter_base = { git = "https://github.com/gausnes/prometheus_exporter_base", branch = "ContentType", features = ["hyper_server"] }


TODO: update when MindFlavor/prometheus_exporter_base#25 is merged

gausnes · 2021-10-12T21:48:04Z

src/main.rs

@@ -194,7 +201,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
    info!("using options: {:?}", options);

    let bind = matches.value_of("port").unwrap();
-    let bind = (&bind).parse::<u16>().expect("port must be a valid number");
+    let bind = bind.parse::<u16>().expect("port must be a valid number");


fixed a number of issues the compiler complained about

gausnes · 2021-10-12T21:50:42Z

src/metrics.rs

+            .render();
+    }
+
+    pub fn latest_handshake(&mut self, instance: &PrometheusInstance<u128, MissingValue>, latest: u128) {


I'm been thinking decoupling these contracts from PrometheusInstance and instead define structs that contain labels. This would:

Decrease the amount of code in wireguard.rs

Make it easier to adopt a different exporter

After working with prom a bit it'd be nice for this to support statsd or dogstats for my usage, the wireguard integration code could be shared

gausnes · 2021-10-12T21:52:11Z

src/wireguard.rs

+# HELP wireguard_peers_total Total number of peers
+# TYPE wireguard_peers_total gauge
+wireguard_peers_total{interface=\"Pippo\"} 1
+";


Feels like a function could be created to generate the expected output, tried to hold off on what refactoring to make this concise

gausnes · 2021-10-12T21:53:53Z

src/metrics.rs

+    pub handshake_timeout_seconds: Option<u64>
+}
+
+pub struct EndpointMetrics<'a> {


Added these classes in an attempt to not add clutter to existing method signatures

gausnes · 2021-10-12T21:54:47Z

src/wireguard.rs

+        )
+    }
+
+    pub(self) fn populate_interface_metrics(


This method containers the core of the previous render_with_names implementation. Tried to create a distinction between the two classifications of metrics.

gausnes · 2021-10-12T21:58:28Z

src/wireguard.rs

+# HELP wireguard_peers_total Total number of peers
+# TYPE wireguard_peers_total gauge
+wireguard_peers_total{{interface=\"Pippo\",seen_recently=\"true\"}} 1
+wireguard_peers_total{{interface=\"Pippo\",seen_recently=\"false\"}} 1


Thoughts on this label name? Feels like there a number of better options than what is initially defined:

connected

recent_handshake

timed_out

and/or have enumerated values e.g.

state: TIMED_OUT, RECENT_SEEN

gausnes · 2021-10-12T22:21:23Z

src/main.rs

@@ -155,6 +156,12 @@ async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
                .help("exports peer's remote ip and port as labels (if available)")
                .takes_value(false),
        )
+        .arg(
+            Arg::with_name("handshake_timeout_seconds")


900 (15 minutes) was the value was the initial value I was going to configure, I can add that as the default if desired

src/wireguard.rs

BroderickCarlin · 2021-10-13T16:21:41Z

src/wireguard.rs

+                        let tokens: Vec<&str> = ip_and_subnet.split('/').collect();
+                        debug!("WireGuard::render_with_names tokens == {:?}", tokens);
+                        let addr = tokens[0];
+                        let subnet = tokens[1];
+                        (addr, subnet)


There is the potential here for a panic if ip_and_subnet is malformed (as in, splitting on / yields only a single value). You may consider a .map_filter() just to safegaurd against bad data

Did at adding a bit of hardening to the data handling here

BroderickCarlin · 2021-10-13T16:30:13Z

src/wireguard.rs

+            if let Some(pehm) = pehm {
+                if let Some(ep_friendly_description) = pehm.get(&ep.public_key as &str) {
+                    if let Some(friendly_description) =


Nothing inherently wrong with this, but another way this could be approached is through a .map() and .flatten() chain such as:

Suggested change

if let Some(pehm) = pehm {

if let Some(ep_friendly_description) = pehm.get(&ep.public_key as &str) {

if let Some(friendly_description) =

pehm

.map(|p| p.get(&ep.public_key as &str))

.flatten()

.map(|ep_friendly_desc| &ep_friendly_desc.friendly_description)

.flatten()

.map(|friendly_desc|

match friendly_description { ...

I personally find the existing implementation is a bit more clear to what is happening, however I can appreciate the horizontal space savings on using .map 🤔

src/wireguard.rs

gausnes commented Oct 12, 2021

View reviewed changes

gausnes marked this pull request as draft October 12, 2021 21:58

gausnes commented Oct 12, 2021

View reviewed changes

gausnes force-pushed the TotalPeerGauge branch from 08fc037 to f023843 Compare October 13, 2021 01:21

BroderickCarlin reviewed Oct 13, 2021

View reviewed changes

src/wireguard.rs Outdated Show resolved Hide resolved

BroderickCarlin reviewed Oct 13, 2021

View reviewed changes

src/wireguard.rs Outdated Show resolved Hide resolved

BroderickCarlin reviewed Oct 13, 2021

View reviewed changes

src/wireguard.rs Outdated Show resolved Hide resolved

BroderickCarlin reviewed Oct 13, 2021

View reviewed changes

src/wireguard.rs Outdated Show resolved Hide resolved

gausnes force-pushed the TotalPeerGauge branch from f023843 to bb91c5b Compare October 14, 2021 17:07

feat: adding gauge to track total peers connected

0dfc007

gausnes force-pushed the TotalPeerGauge branch from bb91c5b to 0dfc007 Compare October 14, 2021 17:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adding gauge to track total peers connected #80

feat: adding gauge to track total peers connected #80

gausnes commented Oct 12, 2021 •

edited

Loading

gausnes Oct 12, 2021

gausnes Oct 12, 2021

gausnes Oct 12, 2021 •

edited

Loading

gausnes Oct 12, 2021

gausnes Oct 12, 2021 •

edited

Loading

gausnes Oct 12, 2021

gausnes Oct 12, 2021

gausnes Oct 12, 2021

BroderickCarlin Oct 13, 2021 •

edited

Loading

gausnes Oct 14, 2021

BroderickCarlin Oct 13, 2021

gausnes Oct 14, 2021

-            if let Some(pehm) = pehm {
-                if let Some(ep_friendly_description) = pehm.get(&ep.public_key as &str) {
-                    if let Some(friendly_description) =
+           pehm
+               .map(|p| p.get(&ep.public_key as &str))
+               .flatten()
+               .map(|ep_friendly_desc| &ep_friendly_desc.friendly_description)
+               .flatten()
+               .map(|friendly_desc|
+                   match friendly_description { ...

feat: adding gauge to track total peers connected #80

Are you sure you want to change the base?

feat: adding gauge to track total peers connected #80

Conversation

gausnes commented Oct 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gausnes Oct 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gausnes Oct 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BroderickCarlin Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gausnes commented Oct 12, 2021 •

edited

Loading

gausnes Oct 12, 2021 •

edited

Loading

gausnes Oct 12, 2021 •

edited

Loading

BroderickCarlin Oct 13, 2021 •

edited

Loading