*: update Cortex (and Prometheus) dependency #4586
Conversation
Update Cortex dependency (and Prometheus together) to include cortexproject/cortex@70dddb6. storage.Querier finally has support for label.Matchers so now we can fully pass them down. Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
GiedriusS
force-pushed
the
update_cortex_prometheus
branch
from
6d283c0
to
614e1b1
Compare
|
Nice! Is this pr ready for review now? |
No, still need to fix the tests hence it is a draft |
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
GiedriusS
force-pushed
the
update_cortex_prometheus
branch
from
600c169
to
8f9a57f
Compare
| } | ||
| sort.Strings(res) | ||
| } | ||
|
|
There was a problem hiding this comment.
This is probably the biggest change - in this function 😄 We have to add external labels here now so that the behaviour would be the same as previously. We were doing Select() and then picking label names out of all series so naturally, they would have external labels there.
|
@yeya24 fixed the tests, PTAL 🍻 |
pkg/api/query/v1.go
Outdated
| @@ -657,13 +657,13 @@ func (qapi *QueryAPI) labelNames(r *http.Request) (interface{}, []error, *api.Ap | |||
| return nil, nil, apiErr | |||
| } | |||
|
|
|||
| var matcherSets [][]*labels.Matcher | |||
| var matcherSets []*labels.Matcher | |||
There was a problem hiding this comment.
The behavior changes here. We need to support multiple matchers, same as the label values API.
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
|
@yeya24 thanks for the very quick review! ❤️ |
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
| @@ -702,7 +702,7 @@ func (c *Client) SeriesInGRPC(ctx context.Context, base *url.URL, matchers []*la | |||
| return m.Data, c.get2xxResultWithGRPCErrors(ctx, "/prom_series HTTP[client]", &u, &m) | |||
| } | |||
|
|
|||
| // LabelNames returns all known label names. It uses gRPC errors. | |||
| // LabelNames returns all known label names constrained by the given matchers. It uses gRPC errors. | |||
| // NOTE: This method is tested in pkg/store/prometheus_test.go against Prometheus. | |||
| func (c *Client) LabelNamesInGRPC(ctx context.Context, base *url.URL, matchers []storepb.LabelMatcher, startTime, endTime int64) ([]string, error) { | |||
There was a problem hiding this comment.
This is still not sufficient I think. If the Prometheus server doesn't support label name matchers, then all label names will be returned. What about doing the same as https://github.com/thanos-io/thanos/blob/main/pkg/store/prometheus.go#L533-L567? Checking Prometheus version seems safer here.
@GiedriusS @bwplotka
There was a problem hiding this comment.
Yea, some compatibility code has to be there, thanks for being careful. I am on PTO until Monday, will catch on this unless someone else does 🤗
* *: update Cortex (and Prometheus) dependency Update Cortex dependency (and Prometheus together) to include cortexproject/cortex@70dddb6. storage.Querier finally has support for label.Matchers so now we can fully pass them down. Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com> * *: fix tests by passing EnableExemplarStorage Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com> * CHANGELOG: update with note about LabelNames() pushdown Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com> * store: pass along matchers in proxy Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com> * store: fix e2e labelname tests Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com> * Fix according to Ben's comments Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com> * test/e2e: fix grammar issue in a comment Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Update Cortex dependency (and Prometheus together) to include
cortexproject/cortex@70dddb6.
storage.Querier finally has support for label.Matchers so now we can
fully pass them down. This is covered by tests. Plus, I've played around a bit
with these changes locally.
Signed-off-by: Giedrius Statkevičius giedrius.statkevicius@vinted.com