vendor: Add dependency on prometheus#70325
Conversation
|
This change is |
Release note: None
41c8d9b to
f4a0a00
Compare
rimadeodhar
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @rimadeodhar)
go.mod, line 141 at r1 (raw file):
github.com/prometheus/common v0.10.0 github.com/prometheus/procfs v0.6.0 // indirect github.com/prometheus/prometheus v2.5.0+incompatible
We have to use this old dependency because of our dependency on the an old zipkin version https://github.com/cockroachdb/cockroach/blob/master/go.mod#L131. Using later versions of prometheus upgrades the zipkin version which breaks our build. I think our zipkin version should be upgraded but that is out of scope for this PR.
|
NB: it's rather unusual to create a PR to import a dependency before it's actually used in the code. |
dhartunian
left a comment
There was a problem hiding this comment.
NB: it's rather unusual to create a PR to import a dependency before it's actually used in the code.
@knz do you know the reason for this? In the case of this PR we have an RFC which clearly outlines the need. I like having a separate dep PR so you can test vendor changes in isolation and make the code review for the actual work easier to look at.
Reviewable status:
go.mod, line 141 at r1 (raw file):
Previously, rimadeodhar (Rima Deodhar) wrote…
We have to use this old dependency because of our dependency on the an old zipkin version https://github.com/cockroachdb/cockroach/blob/master/go.mod#L131. Using later versions of prometheus upgrades the zipkin version which breaks our build. I think our zipkin version should be upgraded but that is out of scope for this PR.
Gotcha. Thanks for clarifying. We should consider upgrading Zipkin at some point as well, if you run into any issues w/ this old prom dependency.
|
TFTR! |
|
Build succeeded: |
This PR adds an external dependency on prometheus. We need
the promql library in order to enforce validity of promql
expressions which will be contained in upcoming alerting
and aggregation rules. These rule implementations are
upcoming as a part of the new metrics upgrade.
Resolves #69796
Release note: None