sergeymakinen / ipsec_exporter

Export strongswan/libreswan IPsec stats to Prometheus
BSD 3-Clause "New" or "Revised" License
21 stars 16 forks source link

Why are UIDs included as a label? #9

Open pokab opened 1 year ago

pokab commented 1 year ago

If I understand it correctly, UIDs in libreswan are generated dynamically and are always changing.

If this is true, including UIDs in metrics, like this:

ipsec_child_sa_state{ike_sa_local_host="xxxx",ike_sa_local_id="",ike_sa_name="xxxx",ike_sa_remote_host="xxxx",ike_sa_remote_id="",ike_sa_remote_identity="",ike_sa_uid="2970",ike_sa_version="1",ike_sa_vips="",local_ts="xxxx",mode="TUNNEL",name="xxxx",protocol="ESP",remote_ts="xxxx",reqid="",uid="2961"} 17

ipsec_child_sa_bytes_out{ike_sa_local_host="xxxx",ike_sa_local_id="",ike_sa_name="xxxx",ike_sa_remote_host="xxxx",ike_sa_remote_id="",ike_sa_remote_identity="",ike_sa_uid="2970",ike_sa_version="1",ike_sa_vips="",local_ts="xxxx",mode="TUNNEL",name="xxxx",protocol="ESP",remote_ts="xxxx",reqid="",uid="2961"} 4096

ipsec_ike_sa_state{local_host="xxxx",local_id="",name="xxxx",remote_host="xxxx",remote_id="",remote_identity="",uid="2953",version="1",vips=""} 7

is a bad practice since it will cause an unbounded number of unique prometheus time series, as described in https://prometheus.io/docs/practices/naming/ :

Remember that every unique combination of key-value label pairs represents a new time series, which can dramatically increase the amount of data stored. Do not use labels to store dimensions with high cardinality (many different label values), such as user IDs, email addresses, or other unbounded sets of values.

Because of this, there should be no label where the value of the label does not come from the configuration file, or is not a fixed value.

So the labels I don't really understand are:

sergeymakinen commented 1 year ago

Hey @pokab, yeah, it looked like a simple idea to get a traffic usage across sessions at that time but it was definitely not a great idea. On the one hand, it will be nice to sort it out in v2 while keeping an ability to monitor a traffic but on the other hand, I've completely switched to WireGuard and thinking on archiving this because I have too many other things to focus on (and hopefully - to share). At least I hope this repository should be useful to others with a working codebase with unit and integration tests...

pokab commented 1 year ago

I've got a working solution pushed in my repository, but not yet in the master branch. Will clean it up later. No problem if you don't want a pull request or maintain this application.