WIP: query: Expose query API request counts

[kakkoyun]

Thanos Query API now exposes thanos_query_api_instant_query_total, thanos_query_api_instant_query_failures_total, thanos_query_api_range_query_total and thanos_query_api_range_query_failures_total metrics to track error rates of query API.

It also attempts to change the behavior of thanos_query_api_instant_query_duration_seconds and thanos_query_api_range_query_duration_seconds, to track the duration of failed queries as well. (Still not sure about this one, since it'd change existing behavior.

Changes

• Expose new query API metrics: thanos_query_api_instant_query_total, thanos_query_api_instant_query_failures_total, thanos_query_api_range_query_total and thanos_query_api_range_query_failures_total.

Verification

None yet.

[bwplotka]

Thanks!
How this relates to standard HTTP metrics defined for each API call here:

thanos/cmd/thanos/query.go

Line 401 in 865d5ec

ins := extpromhttp.NewInstrumentationMiddleware(reg)

Wonder if it is not duplication bit ): If it is definitely there is some work on docs / example alerts (:

[kakkoyun]

@bwplotka You're definitely right. Apparently, I have overlooked it. In this sense, all the metrics in this component can be replaced by middleware metrics.

http_requests_total{job="thanos-querier", handler="query"}
http_requests_total{job="thanos-querier", handler="query"}
http_request_duration_seconds_bucket{job="thanos-querier", handler="query"}
http_request_duration_seconds_bucket{job="thanos-querier", handler="query_range"}

If you don't have backwards compatibility concerns, could I remove thanos_query_api_instant_query_duration_seconds and thanos_query_api_range_query_duration_second, since they are also duplicates?

Maybe it's not the right place when I was checking code base for this PR, I have realized we have similar metrics for receive component

thanos/pkg/receive/handler.go

Line 76 in 865d5ec

requestDuration: prometheus.NewHistogramVec(

.
Should we use

thanos/pkg/extprom/http/instrument_server.go

Line 38 in 865d5ec

func NewInstrumentationMiddleware(reg *prometheus.Registry) InstrumentationMiddleware {

for receive as well?

I could also help with documentation of metrics and dashboards if it's needed. I recently worked on https://github.com/metalmatze/kube-thanos/tree/master/jsonnet/thanos-mixin, I can help while my caches are hot.

cc @brancz

[brancz]

All of the above sounds good to me! (no compatibility concerns)

[bwplotka]

Fully agree here. We are still 0.x releases so it's actually the best moment to define the proper consistent approach here! Makes sense @kakkoyun

[kakkoyun]

Cool, then I'll close this one and focus on consistent instrumentation in another PR.