Add first opentelemetry metric #393
Conversation
|
@erebe first draft PR for the metrics provider. This is mostly to make sure the general style fits your expectations, though could already be merged as is. This is the ~MVP of getting some prometheus metrics with the opentelemetry tooling. The main potentially contentious point I see: This tooling is quite beta/alpha stage in the rust ecosystem. |
| verbatim_doc_comment, | ||
| default_value = None, | ||
| )] | ||
| metrics_provider_address: Option<String>, |
There was a problem hiding this comment.
You can directly set an IpAddress in instead of a string to directly refuse invalid bind address
| let fut = conn | ||
| .serve_connection( | ||
| stream, | ||
| service_fn(|req: Request<body::Incoming>| { |
There was a problem hiding this comment.
you need to check that the path is /metrics to return the metrics
| self.metrics.connections.add( | ||
| 1, | ||
| &[ | ||
| KeyValue::new("remote_host", format!("{}", remote.host)), |
There was a problem hiding this comment.
We should avoid setting remote_host and port as label as it will make too much cardinality for prometheus, and create a data leak for us.
If a user request random host / port our metrics are going to fill the available memory of wstunnel until it OOM as they are never cleaned.
metrics should not jeopardize the stability of the system
https://stackoverflow.com/a/69167162
|
Overall, looks ok 👍 |
No description provided.