Fixing prom v3 upgrade issues - #4044
Conversation
|
💻 Deploy preview deleted. |
| send_native_histograms = true | ||
| metadata_config { | ||
| send_interval = "1s" | ||
| prometheus.write.queue "scrape_prom_metrics" { |
There was a problem hiding this comment.
As a follow up, I want to expand our integration tests to cover more cases and protocols and use old remote_write component too. But for now we should switch to the one we use most commonly: new WAL with (mostly) default settings. The only settings we're giving here are to make the test go faster.
1698f0f to
51cd328
Compare
| replace github.com/prometheus/prometheus => github.com/thampiotr/prometheus v0.0.0-20250731131506-a28a7d9c5b3b // staleness_disabling_v3.4.2 branch | ||
|
|
||
| // TODO: remove this replace directive once the upstream issue is fixed: https://github.com/prometheus/common/pull/809 | ||
| replace github.com/prometheus/common => github.com/thampiotr/common v0.0.0-20250801132732-61b386b68d79 |
There was a problem hiding this comment.
We're almost there! My latest tests have resolved all the issues I know about. The only work remaining is to get the fixes merged upstream:
- Corrupted metrics names: pending [BUGFIX] Scraping: Fix memory corruption in name and unit labels under -tags slicelabels prometheus/prometheus#16957 getting merged upstream, after which this will need cherry-picking to our branch here: fix corrupted name label prometheus#44
- Incorrect metric escaping in protobuf protocol: pending Fix delimited proto not escaped correctly prometheus/common#809 getting merged in common.
@ptodev it would be great if we can review the rest and I will just replace these dependencies as soon as upstreams are merged 🤞
| // * There is a release of Prometheus which addresses https://github.com/prometheus/prometheus/issues/14049, | ||
| // for example, via this implementation: https://github.com/grafana/prometheus/pull/34 | ||
| replace github.com/prometheus/prometheus => github.com/grafana/prometheus v1.8.2-0.20250709144109-5551df55271b | ||
| replace github.com/prometheus/prometheus => github.com/thampiotr/prometheus v0.0.0-20250731131506-a28a7d9c5b3b // staleness_disabling_v3.4.2 branch |
There was a problem hiding this comment.
As far as I can tell we use fix-corrupted-name-label in your fork, that includes the staleness fix. Are we planning to use your fork here or is this only temporary?
There was a problem hiding this comment.
This is temporary while we get this fix prometheus/prometheus#16957 merged upstream. I mentioned on Slack, sorry if you missed this.
There was a problem hiding this comment.
I'm happy to merge the PR as long as we don't release the next version with this fork.
| // * There is a release of Prometheus which addresses https://github.com/prometheus/prometheus/issues/14049, | ||
| // for example, via this implementation: https://github.com/grafana/prometheus/pull/34 | ||
| replace github.com/prometheus/prometheus => github.com/grafana/prometheus v1.8.2-0.20250709144109-5551df55271b | ||
| replace github.com/prometheus/prometheus => github.com/thampiotr/prometheus v0.0.0-20250731131506-a28a7d9c5b3b // staleness_disabling_v3.4.2 branch |
There was a problem hiding this comment.
I'm happy to merge the PR as long as we don't release the next version with this fork.
Thanks! I'll merge now to avoid further toil with merge conflicts and so I can get the dev deployment pipeline working again. I am watching for my upstream merge fixed and will follow-up with fixing the go.mod soon. |
3 participants
PR Description
Fixes several issues that the upgrade to prom v3 has introduced:
scrape_native_histogramsdefault tofalseas it was incorrectly set totruein the upgrade.Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist