Fix duplicate metric registration for filters (#397)

Some filter currently call `register` (rather than
`register_if_not_exists`) which throws an error if the metric
has already been registered - this meant that we couldn't use
multiple instances of those filter in the filter chain

Author: iffyio

src/filters/capture_bytes/metrics.rs

@@ -32,7 +32,7 @@ impl Metrics {
                 "CaptureBytes",
                 "Total number of packets dropped due capture size being larger than the received packet",
             ))?
-            .register(registry)?,
+            .register_if_not_exists(registry)?,
         })
     }
 }

src/filters/compress/metrics.rs

@@ -38,21 +38,21 @@ impl Metrics {
             ),
             &operation_labels,
         )?
-        .register(registry)?;
+        .register_if_not_exists(registry)?;
 
         let decompressed_bytes_total = IntCounter::with_opts(filter_opts(
             "decompressed_bytes_total",
             "Compress",
             "Total number of decompressed bytes either received or sent.",
         ))?
-        .register(registry)?;
+        .register_if_not_exists(registry)?;
 
         let compressed_bytes_total = IntCounter::with_opts(filter_opts(
             "compressed_bytes_total",
             "Compress",
             "Total number of compressed bytes either received or sent.",
         ))?
-        .register(registry)?;
+        .register_if_not_exists(registry)?;
 
         Ok(Metrics {
             packets_dropped_compress: dropped_metric

src/filters/local_rate_limit/metrics.rs

@@ -31,7 +31,7 @@ impl Metrics {
                 "LocalRateLimit",
                 "Total number of packets dropped due to rate limiting",
             ))?
-            .register(registry)?,
+            .register_if_not_exists(registry)?,
         })
     }
 }

src/filters/token_router/metrics.rs

@@ -36,7 +36,7 @@ impl Metrics {
             ),
             &label_names,
         )?
-        .register(registry)?;
+        .register_if_not_exists(registry)?;
 
         Ok(Metrics {
             packets_dropped_no_token_found: metric

src/metrics.rs

@@ -43,25 +43,22 @@ pub fn filter_opts(name: &str, filter_name: &str, description: &str) -> Opts {
     opts(name, &format!("filter_{}", filter_name), description)
 }
 
-pub trait CollectorExt: Collector + Clone + Sized + 'static {
-    /// Registers the current metric collector with the provided registry.
-    /// Returns an error if a collector with the same name has already
-    /// been registered.
-    fn register(self, registry: &Registry) -> Result<Self> {
-        registry.register(Box::new(self.clone()))?;
-        Ok(self)
-    }
+/// Registers the current metric collector with the provided registry.
+/// Returns an error if a collector with the same name has already
+/// been registered.
+fn register_metric<T: Collector + Sized + 'static>(
+    registry: &Registry,
+    collector: T,
+) -> Result<()> {
+    registry.register(Box::new(collector))
+}
 
+pub trait CollectorExt: Collector + Clone + Sized + 'static {
     /// Registers the current metric collector with the provided registry
     /// if not already registered.
     fn register_if_not_exists(self, registry: &Registry) -> Result<Self> {
-        match self.clone().register(registry) {
+        match register_metric(registry, self.clone()) {
             Ok(_) | Err(prometheus::Error::AlreadyReg) => Ok(self),
-            Err(prometheus::Error::Msg(msg)) if msg.contains("already exists") => {
-                // FIXME: We should be able to remove this branch entirely if `AlreadyReg` gets fixed.
-                //  https://github.com/tikv/rust-prometheus/issues/247
-                Ok(self)
-            }
             Err(err) => Err(err),
         }
     }