nhcb: Add global config option for convert_classic_histograms_to_nhcb #16226
Conversation
3511e1f to
e6b3479
Compare
df1d683 to
818a191
Compare
|
@krajorama or @beorn7 can either of you help review this? |
There was a problem hiding this comment.
Looking good, I have some further suggestions.
Bartek asked if we should use *bool , but I don't see how we could avoid it. The other parameters that have global defaults are numbers (like scrape interval, timeout) so it's easy to just take the 0 value as not-set, but this is a bool.
The thing that has tripped us up before was making changes effective on reload of configuration. I don't have the time to check that now, but maybe you could try it out? See #13846
818a191 to
36b9695
Compare
There was a problem hiding this comment.
The thing that has tripped us up before was making changes effective on reload of configuration. I don't have the time to check that now, but maybe you could try it out? See #13846
Thanks for pointing this out @krajorama. I tested reloading prometheus with a config change to the convert classic hist to nhcb and the change was picked up. Did this by sending a SIGHUP to prometheus. Is there further considerations for this?
Regarding the sync part, I saw sync is only called here
Line 202 in 7d73c1d
Addresses the global part of prometheus#13532 Signed-off-by: chardch <otwordsne@gmail.com>
36b9695 to
7c13bed
Compare
Signed-off-by: chardch <otwordsne@gmail.com>
7c13bed to
357b5ed
Compare
Addresses the global part of #13532