search

oliver006 / redis_exporter

Prometheus Exporter for ValKey & Redis Metrics. Supports ValKey and Redis 2.x, 3.x, 4.x, 5.x, 6.x, and 7.x
https://github.com/oliver006/redis_exporter
MIT License
3.18k stars 877 forks source link

`sentinel_master_setting_ckquorum` metric is incorrect? #930

Open jdheyburn opened 3 months ago

jdheyburn commented 3 months ago

Describe the problem

I recently upgraded our nonprod exporters from 1.55.0 to 1.62.0 and wanted to verify the new metrics that are being exported.

It seems that sentinel_master_setting_ckquorum is always reporting 0 because the field the code is scraping from does not exist. Link to line.

masterCkquorum, _ := strconv.ParseFloat(masterDetailMap["ckquorum"], 64)

There is no ckquorum field from the output of redis-cli -p 26379 sentinel masters - there is however a quorum field.

/data $ redis-cli -p 26379 sentinel masters | grep ckquorum
/data $ redis-cli -p 26379 sentinel masters | grep quorum -A1
quorum
2

I think this is a typo in the code from when it was added in below PR. In the PR the author even quotes as the field being quorum over ckquorum.

What version of redis_exporter are you running? Please run redis_exporter --version if you're not sure what version you're running. [ ] 0.3x.x [x] 1.x.x

Running the exporter N/A

N/A

Expected behavior

I believe the code should be:

masterCkquorum, _ := strconv.ParseFloat(masterDetailMap["quorum"], 64)

Screenshots N/A

Additional context It would then make sense for the metric exported to instead be sentinel_master_setting_quorum, but this may cause a breaking change. But then if it was never reporting correctly in the first place then it may not be breaking.

oliver006 commented 3 months ago

Not really familiar with the sentinel stuff (I don't use it) but sounds like you're right and this should be quorum instead of ckquorum

I wouldn't worry about the breaking change as it's broken right now and this will fix it.