Implement federated Status API#8484
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
2eb699d to
c15f7bc
Compare
saswatamcode
left a comment
saswatamcode
left a comment
There was a problem hiding this comment.
Thanks for working on this! I think this is in a great direction already!
Would be great to see some e2e test
| enableExemplarPartialResponse := cmd.Flag("exemplar.partial-response", "Enable partial response for exemplar endpoint. --no-exemplar.partial-response for disabling."). | ||
| Hidden().Default("true").Bool() | ||
|
|
||
| enableStatusPartialResponse := cmd.Flag("status.partial-response", "Enable partial response for status endpoint. --no-exemplar.partial-response for disabling."). |
There was a problem hiding this comment.
| enableStatusPartialResponse := cmd.Flag("status.partial-response", "Enable partial response for status endpoint. --no-exemplar.partial-response for disabling."). | |
| enableStatusPartialResponse := cmd.Flag("status.partial-response", "Enable partial response for status endpoint. --no-status.partial-response for disabling."). |
c15f7bc to
453b465
Compare
This is the initial implementation of a federated TSDB Status API. Right now, the change covers: * New `/api/v1/status/tsdb` endpoint in Thanos Query (compatible with the existing Prometheus API). * Definition of a new gRPC Status API. * Implementation of the gRPC Status server for Query and Receive. Relates to thanos-io#5395 Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
3ffabb5 to
afb0e0a
Compare
The new tests check the TSDB status API without and with tenancy. Signed-off-by: Simon Pasquier <[email protected]>
afb0e0a to
3df8742
Compare
|
End-to-end tests added. I think that the failures are unrelated to my change. |
simonpasquier
changed the title
Signed-off-by: Simon Pasquier <[email protected]>
saswatamcode
left a comment
saswatamcode
left a comment
There was a problem hiding this comment.
Thanks a lot for the effort on this! 💪🏻
LGTM already, just one small comment!
| } | ||
|
|
||
| // Merge statistics from all tenants. | ||
| aggregatedStats := &statuspb.TSDBStatisticsEntry{} |
There was a problem hiding this comment.
So I thought about this a bit more.
Right now, by default, we get the aggregate stats merged from all TSDBs in multitsdb. And we can pass in a tenant to get specific tenant stats.
But there's also some utility in getting the map of tenants and tenant stats directly (powering a thanos tenant health dashboard)!
Maybe it might be useful to have another bool URL query param here called aggregate?
There was a problem hiding this comment.
I agree that it would useful. But I'm not sure if it should be the same endpoint or if we need a dedicated one (given that the response data will look quite different).
There was a problem hiding this comment.
Hmm good point, wouldn't be great. So in that case, maybe some separate thanos specific endpoint.
Happy to have it in a separate PR, rest looks good!
saswatamcode
left a comment
saswatamcode
left a comment
There was a problem hiding this comment.
LGTM! Let's merge and iterate
Failing test is unrelated
3 participants
Changes
This is a first draft towards an implementation of a federated TSDB Status API. The goal is to kick-off the discussion.The change covers:
/api/v1/status/tsdbendpoint in Thanos Query (compatible with the existing Prometheus API).Relates to #5395
Verification
Manual verification + unit tests + end-to-end tests.