Support Prometheus /api/v1/status/buildinfo API on Querier/QFE #4978
Conversation
|
An open question: From the Cortex project's point view, shall we support this API on all Cortex components? Thanos support this API for all. But from Grafana's perspective, exposing this on the query component should be good enough. |
yeya24
force-pushed
the
support-buildinfo-api
branch
from
08b97b2 to
3340c90
Compare
There was a problem hiding this comment.
Possible to write an unit test for this?
Also, should the API path make it clear that it's the underlying Prometheus build info and not the Cortex build info? How about a build info for Thanos? I guess my point is - should the build info API include information about Cortex, Thanos, and Prometheus (also Alertmanager)?
|
In terms of the additional information in the build info API, I think we could add some cortex specific info to so that we know it is Cortex. But I don't think Prometheus or Thanos would do the same thing right now, at least for Thanos. The usecase is not very clear and no client really needs it right now, even Grafana it relies on user input to decide whether it is Thanos, Cortex, etc. |
yeya24
force-pushed
the
support-buildinfo-api
branch
3 times, most recently
from
ca6f19e to
e08a33f
Compare
|
Tested working using my local Grafana and the version can be automatically detected. |
| @@ -211,7 +212,11 @@ func NewQuerierHandler( | |||
| false, | |||
| regexp.MustCompile(".*"), | |||
| func() (v1.RuntimeInfo, error) { return v1.RuntimeInfo{}, errors.New("not implemented") }, | |||
| &v1.PrometheusVersion{}, | |||
| &v1.PrometheusVersion{ | |||
There was a problem hiding this comment.
Why we decided not to support the following?
BuildUser string `json:"buildUser"`
BuildDate string `json:"buildDate"`
GoVersion string `json:"goVersion"`
Not that I want to support them, but I thought it do be nice to have explicit discussion on this as documentation purposes.
There was a problem hiding this comment.
I could add GoVersion for sure. Thanks for catching.
But for other variables, right now we don't propagate these variables right now from our build. I will add them here first and when needed we can propagate these info through makefile
There was a problem hiding this comment.
Can we create an issue to remember to update https://cortexmetrics.io/docs/api/ for the next issue after this PR is merged?
|
@alvinlin123 I updated the API doc. |
Sorry that mislead you with my previous comment. But, problem with updating it now is that people using non-main brach may get confused. I think we should update the API doc while doing 1.15 release. I have created a release 1.15 milestone can you add an issue to update the API doc in that milestone? Thanks I really hope we can have a version selector in our API doc ... |
|
@alvinlin123 I see that makes sense... |
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
support-buildinfo-api
branch
2 times, most recently
from
e83ce77 to
0497704
Compare
yeya24
force-pushed
the
support-buildinfo-api
branch
from
0497704 to
79cf003
Compare
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
support-buildinfo-api
branch
from
79cf003 to
a405e9c
Compare
|
Hey @alvinlin123, I reverted the doc update. Could you please take another look? |
Signed-off-by: Ben Ye [email protected]
What this PR does:
Grafana uses this API to detect Cortex version and decides whether to use series API with matchers or the label values API. Support it will benefit Grafana & Cortex users to use an API with better performance.
This is a new feature in Grafana grafana/grafana#56496.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]