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. |
| 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
There was a problem hiding this comment.
I'll have to think about this a bit.
I see why you say that, and agree with the point you make.
OTOH, I need the cardinality to get the information I need. So I'll have to think about some way to customize this.
In my case, k8s and multiple replicas make an OOM less serious.
|
Overall, looks ok 👍 |
79824d2 to
ae52331
Compare
a3375f3 to
d065bfa
Compare
|
I discovered recently that this rust crate for prometheus is managing the expiration of old metrics. If that can help you |
d28409f to
b36e8a4
Compare
ef0eb24 to
c5ac5fb
Compare
cbecd19 to
977c385
Compare
a8c4240 to
73c53b0
Compare
5ce3fb8 to
c5e2981
Compare
9b03594 to
84e6786
Compare
No description provided.