-
-
Notifications
You must be signed in to change notification settings - Fork 60
WIP: add regex replacements #6
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -203,6 +203,9 @@ use futures::future::{ok, FutureResult}; | |
| use futures::{Async, Future, Poll}; | ||
| use prometheus::{opts, Encoder, HistogramVec, IntCounterVec, Registry, TextEncoder}; | ||
|
|
||
| #[cfg(feature = "regex-replace")] | ||
| use regex::Regex; | ||
|
|
||
| #[derive(Clone)] | ||
| #[must_use = "must be set up as middleware for actix-web"] | ||
| /// By default two metrics are tracked (this assumes the namespace `actix_web_prom`): | ||
|
|
@@ -221,11 +224,15 @@ pub struct PrometheusMetrics { | |
| pub registry: Registry, | ||
| pub(crate) namespace: String, | ||
| pub(crate) endpoint: Option<String>, | ||
|
|
||
| #[cfg(feature = "regex-replace")] | ||
| pub(crate) replacements: Vec<(Regex, String)>, | ||
| } | ||
|
|
||
| impl PrometheusMetrics { | ||
| /// Create a new PrometheusMetrics. You set the namespace and the metrics endpoint | ||
| /// through here. | ||
| #[cfg(not(feature = "regex-replace"))] | ||
| pub fn new(namespace: &str, endpoint: Option<&str>) -> Self { | ||
| let registry = Registry::new(); | ||
|
|
||
|
|
@@ -234,20 +241,65 @@ impl PrometheusMetrics { | |
| } | ||
|
|
||
| /// Create a new PrometheusMetrics. | ||
| /// Throws error if "<`namespace`>_http_requests_total" already registered | ||
| /// Returns an error if "<`namespace`>_http_requests_total" already registered. | ||
| /// Request paths are adjusted by executing the supplied replacements before storing the metrics. | ||
| #[cfg(feature = "regex-replace")] | ||
| pub fn new_with_registry( | ||
| registry: Registry, | ||
| namespace: &str, | ||
| endpoint: Option<&str>, | ||
| replacements: Vec<(Regex, String)>, | ||
| ) -> Result<Self, Box<dyn std::error::Error>> { | ||
| let (http_requests_total, http_requests_duration_seconds) = | ||
| Self::register_metrics(®istry, namespace)?; | ||
| Ok(PrometheusMetrics { | ||
| http_requests_total, | ||
| http_requests_duration_seconds, | ||
| registry, | ||
| namespace: namespace.to_string(), | ||
| endpoint: if let Some(url) = endpoint { | ||
| Some(url.to_string()) | ||
| } else { | ||
| None | ||
| }, | ||
| replacements, | ||
| }) | ||
| } | ||
|
|
||
| /// Create a new PrometheusMetrics. | ||
| /// Returns an error if "<`namespace`>_http_requests_total" already registered | ||
| #[cfg(not(feature = "regex-replace"))] | ||
| pub fn new_with_registry( | ||
| registry: Registry, | ||
| namespace: &str, | ||
| endpoint: Option<&str>, | ||
| ) -> Result<Self, Box<dyn std::error::Error>> { | ||
| let (http_requests_total, http_requests_duration_seconds) = | ||
| Self::register_metrics(®istry, namespace)?; | ||
| Ok(PrometheusMetrics { | ||
| http_requests_total, | ||
| http_requests_duration_seconds, | ||
| registry, | ||
| namespace: namespace.to_string(), | ||
| endpoint: if let Some(url) = endpoint { | ||
| Some(url.to_string()) | ||
| } else { | ||
| None | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| fn register_metrics( | ||
| registry: &Registry, | ||
| namespace: &str, | ||
| ) -> prometheus::Result<(IntCounterVec, HistogramVec)> { | ||
| let http_requests_total_opts = | ||
| opts!("http_requests_total", "Total number of HTTP requests").namespace(namespace); | ||
| let http_requests_total = | ||
| IntCounterVec::new(http_requests_total_opts, &["endpoint", "method", "status"]) | ||
| .unwrap(); | ||
| registry | ||
| .register(Box::new(http_requests_total.clone())) | ||
| .unwrap(); | ||
|
|
||
| registry.register(Box::new(http_requests_total.clone()))?; | ||
|
|
||
| let http_requests_duration_seconds_opts = opts!( | ||
| "http_requests_duration_seconds", | ||
|
|
@@ -259,19 +311,10 @@ impl PrometheusMetrics { | |
| &["endpoint", "method", "status"], | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| registry.register(Box::new(http_requests_duration_seconds.clone()))?; | ||
|
|
||
| Ok(PrometheusMetrics { | ||
| http_requests_total, | ||
| http_requests_duration_seconds, | ||
| registry, | ||
| namespace: namespace.to_string(), | ||
| endpoint: if let Some(url) = endpoint { | ||
| Some(url.to_string()) | ||
| } else { | ||
| None | ||
| }, | ||
| }) | ||
| Ok((http_requests_total, http_requests_duration_seconds)) | ||
| } | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally a cfg attribute on the parameter would make duplicating this method unnecessary, but that would require rust nightly, since attributes on parameters are not stabilized yet (proposed to be stabilized soon though!)
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it's now in 1.39 so this can be changed.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It requires that we bump the minimum rust version to 1.39, if that's okay we can do that. |
||
| fn metrics(&self) -> String { | ||
|
|
@@ -290,7 +333,29 @@ impl PrometheusMetrics { | |
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "regex-replace")] | ||
| fn update_metrics(&self, path: &str, method: &Method, status: StatusCode, clock: SystemTime) { | ||
| log::trace!("About to replacing path capture groups: {}", path); | ||
| let mut path = String::from(path); | ||
| for (exp, repl) in self.replacements.clone() { | ||
| path = String::from(exp.replace_all(&path, &repl as &str)) | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not really happy with this as it is, because each time we're updating metrics, we copy all replacements, the path once, and then the path again for each replacement. I couldn't find a way to do this with less copies though that also makes the borrow checker happy. |
||
| log::trace!("Replaced path capture groups: {}", path); | ||
| self.update_metrics_inner(&path, method, status, clock) | ||
| } | ||
|
|
||
| #[cfg(not(feature = "regex-replace"))] | ||
| fn update_metrics(&self, path: &str, method: &Method, status: StatusCode, clock: SystemTime) { | ||
| self.update_metrics_inner(path, method, status, clock) | ||
| } | ||
|
|
||
| fn update_metrics_inner( | ||
| &self, | ||
| path: &str, | ||
| method: &Method, | ||
| status: StatusCode, | ||
| clock: SystemTime, | ||
| ) { | ||
| let method = method.to_string(); | ||
| let status = status.as_u16().to_string(); | ||
|
|
||
|
|
@@ -304,6 +369,7 @@ impl PrometheusMetrics { | |
| self.http_requests_total | ||
| .with_label_values(&[&path, &method, &status]) | ||
| .inc(); | ||
| log::trace!("updated metrics"); | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be more logging in this crate IMO, added the log crate to enable logging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed.