Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add visibility when subqueries are rewritten #10502

Merged
merged 2 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading