feat(process): ProcessCollector use IntGauge to provide better perfor… #430
Conversation
…mance, close tikv#429 Signed-off-by: wuaoxiang <wuaoxiang@stargraph.cn>
| rss: Gauge, | ||
| start_time: Gauge, | ||
| threads: Gauge, | ||
| cpu_total: IntCounter, |
There was a problem hiding this comment.
I totally get the performance argument, but could we please keep the cpu_total metric as f64? All the other metrics (file descriptor counts, memory usage, threads) are actual integers, but processes that don't use a lot of CPU will now have 1 second CPU usage spikes every so often instead of a more or less flat usage with actual spikes occurring when they actually do.
E.g. a process using 1% CPU on average, with a 10% spike every minute will now look as if it is using 0% CPU most of the time and spike every 2 minutes out of 3.
I really don't think the marginal performance improvement is worth it. Please keep using Counter for cpu_total.
| @@ -90,7 +88,7 @@ impl ProcessCollector { | |||
| .unwrap(); | |||
| descs.extend(rss.desc().into_iter().cloned()); | |||
|
|
|||
| let start_time = Gauge::with_opts( | |||
| let start_time = IntGauge::with_opts( | |||
There was a problem hiding this comment.
Actually, this is also not an integer. Although not as critical as CPU usage (because it is essentially a constant) the process start time has fractional seconds (and they may be useful).
similar change like this PR in Tikv: tikv/tikv#11308
close #429
Signed-off-by: wuaoxiang wuaoxiang@stargraph.cn