Skip to content

Commit

Permalink
Add visibility when subqueries are rewritten (#10502)
Browse files Browse the repository at this point in the history
* Add visibility when subqueries are rewritten

Follow up to #10445, add a metric when queries are rewritten for
compatibility with Prometheus 3 range selector semantics.

Signed-off-by: Nick Pillitteri <[email protected]>

* Skip TestSampleTracker_Concurrency since it is not reliable

Signed-off-by: Nick Pillitteri <[email protected]>

---------

Signed-off-by: Nick Pillitteri <[email protected]>
  • Loading branch information
56quarters authored Jan 23, 2025
1 parent 77194ac commit c849fd5
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 10 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* [BUGFIX] PromQL: Fix <aggr_over_time> functions with histograms https://github.com/prometheus/prometheus/pull/15711 #10400
* [BUGFIX] MQE: Fix <aggr_over_time> functions with histograms #10400
* [BUGFIX] Distributor: return HTTP status 415 Unsupported Media Type instead of 200 Success for Remote Write 2.0 until we support it. #10423
* [BUGFIX] Query-frontend: Add flag `-query-frontend.prom2-range-compat` and corresponding YAML to rewrite queries with ranges that worked in Prometheus 2 but are invalid in Prometheus 3. #10445 #10461
* [BUGFIX] Query-frontend: Add flag `-query-frontend.prom2-range-compat` and corresponding YAML to rewrite queries with ranges that worked in Prometheus 2 but are invalid in Prometheus 3. #10445 #10461 #10502
* [BUGFIX] Distributor: Fix edge case at the HA-tracker with memberlist as KVStore, where when a replica in the KVStore is marked as deleted but not yet removed, it fails to update the KVStore. #10443

### Mixin
Expand Down
3 changes: 3 additions & 0 deletions pkg/costattribution/sample_tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ func TestSampleTracker_inactiveObservations(t *testing.T) {
}

func TestSampleTracker_Concurrency(t *testing.T) {
// Disabled due to spurious failures. See https://github.com/grafana/mimir/issues/10482
t.Skip()

m := newTestManager()
st := m.SampleTracker("user1")

Expand Down
26 changes: 19 additions & 7 deletions pkg/frontend/querymiddleware/prom2_range_compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (

"github.com/go-kit/log"
"github.com/grafana/dskit/tenant"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/prometheus/promql/parser"

apierror "github.com/grafana/mimir/pkg/api/error"
Expand All @@ -17,20 +19,27 @@ import (

// newProm2RangeCompatMiddleware creates a new query middleware that adjusts range and resolution
// selectors in subqueries in some cases for compatibility with Prometheus 3.
func newProm2RangeCompatMiddleware(limits Limits, logger log.Logger) MetricsQueryMiddleware {
func newProm2RangeCompatMiddleware(limits Limits, logger log.Logger, reg prometheus.Registerer) MetricsQueryMiddleware {
rewritten := promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
Name: "cortex_frontend_prom2_range_compat_rewritten_total",
Help: "The number of times a query was rewritten for Prometheus 2/3 range selector compatibility",
}, []string{"user"})

return MetricsQueryMiddlewareFunc(func(next MetricsQueryHandler) MetricsQueryHandler {
return &prom2RangeCompatHandler{
next: next,
limits: limits,
logger: logger,
next: next,
limits: limits,
logger: logger,
rewritten: rewritten,
}
})
}

type prom2RangeCompatHandler struct {
next MetricsQueryHandler
limits Limits
logger log.Logger
next MetricsQueryHandler
limits Limits
logger log.Logger
rewritten *prometheus.CounterVec
}

func (c *prom2RangeCompatHandler) Do(ctx context.Context, r MetricsQueryRequest) (Response, error) {
Expand All @@ -52,8 +61,11 @@ func (c *prom2RangeCompatHandler) Do(ctx context.Context, r MetricsQueryRequest)

rewrittenQuery := rewritten.String()
if origQuery != rewrittenQuery {
tenantIDString := tenant.JoinTenantIDs(tenantIDs)
c.rewritten.WithLabelValues(tenantIDString).Inc()
spanLog.DebugLog(
"msg", "modified subquery for compatibility with Prometheus 3 range selectors",
"tenants", tenantIDString,
"original", origQuery,
"rewritten", rewrittenQuery,
)
Expand Down
3 changes: 2 additions & 1 deletion pkg/frontend/querymiddleware/prom2_range_compat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/go-kit/log"
"github.com/grafana/dskit/user"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)
Expand All @@ -23,7 +24,7 @@ func TestProm2RangeCompat_Do(t *testing.T) {
}

runHandler := func(ctx context.Context, inner MetricsQueryHandler, limits Limits, req MetricsQueryRequest) (Response, error) {
middleware := newProm2RangeCompatMiddleware(limits, log.NewNopLogger())
middleware := newProm2RangeCompatMiddleware(limits, log.NewNopLogger(), prometheus.NewPedanticRegistry())
handler := middleware.Wrap(inner)
return handler.Do(ctx, req)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/frontend/querymiddleware/roundtrip.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ func newQueryMiddlewares(
metrics := newInstrumentMiddlewareMetrics(registerer)
queryBlockerMiddleware := newQueryBlockerMiddleware(limits, log, registerer)
queryStatsMiddleware := newQueryStatsMiddleware(registerer, engine)
prom2CompatMiddleware := newProm2RangeCompatMiddleware(limits, log)
prom2CompatMiddleware := newProm2RangeCompatMiddleware(limits, log, registerer)

remoteReadMiddleware = append(remoteReadMiddleware,
// Track query range statistics. Added first before any subsequent middleware modifies the request.
Expand Down

0 comments on commit c849fd5

Please sign in to comment.