native-histograms: Fixed PrometheusProto scrape format preference.#13010
native-histograms: Fixed PrometheusProto scrape format preference.#13010
Conversation
Broken by #12738. We have to update both global variables (as GlobalConfig is not a pointer here). DefaultConfig is used when no global: section is provided, whereas DefaultGlobalConfig is used when it's provided and for individual scrape configs. Reported on #prometheus-dev (thanks to @beorn7): https://cloud-native.slack.com/archives/C01AUBA4PFE/p1697733267205649 Tested manually, it would be nice to add test at some point (quick fix for now). Signed-off-by: bwplotka <bwplotka@gmail.com>
684fb3b to
e319da0
Compare
|
Let's first base this on top of the release-2.48 branch because we need it in the next RC. |
beorn7
left a comment
There was a problem hiding this comment.
Thank you very much for the quick fix. Approved modulo merge into release-2.48 first.
|
PR that broke it was specifically excluded from the release-2.48 (see #12985 description). I don't know why, probably @LeviHarrison and @roidelapluie don't like me. Or they had instinct it was broken 🙃 So merging it into main. |
|
That would be the foresight of @bboreham. Thanks for the quick fix though! |
|
Yea, no worries, just let's be explicit why not in the PR description next time on the reasoning (I still don't know why not). 🤗 |
|
From our conversation I said "It's a judgement call. [...] 12738 is changing the way configuration works. I think the old way is marked experimental, but still this is the one I would worry about." [and all the others that were cherry-picked were bug-fixes or very simple] |
|
Yes and I had technically cut 2.48 a week earlier, before the PR had been merged, but there was a hold up with dependency updates. |
Broken by #12738. We have to update both global variables (as GlobalConfig is not a pointer here). DefaultConfig is used when no
global:section is provided, whereas DefaultGlobalConfig is used when it's provided and for individual scrape configs.Reported on #prometheus-dev (thanks to @beorn7): https://cloud-native.slack.com/archives/C01AUBA4PFE/p1697733267205649
Tested manually, it would be nice to add test at some point (quick fix for now).