From 90ff28bf6e4109170555e9005532659d802ce0b4 Mon Sep 17 00:00:00 2001 From: Yi Shi Date: Fri, 20 Mar 2026 18:24:34 +0000 Subject: [PATCH 1/4] add log logic when protection rule triggered log --- pkg/queryfrontend/protection_middleware.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/pkg/queryfrontend/protection_middleware.go b/pkg/queryfrontend/protection_middleware.go index 8f6e9ec3234..d8fa07a865c 100644 --- a/pkg/queryfrontend/protection_middleware.go +++ b/pkg/queryfrontend/protection_middleware.go @@ -80,16 +80,26 @@ func (m *protectionMiddleware) Do(ctx context.Context, r queryrange.Request) (qu if protectionResult != nil { ctx = WithProtectionResult(ctx, protectionResult) - } - - // Return 400 Bad Request error code if a protection rule is triggered. - if protectionResult != nil && protectionResult.Action == RuleActionBlock { - return nil, httpgrpc.Errorf(http.StatusBadRequest, "query blocked by protection rule: %s", protectionResult.RuleName) + if err := m.applyProtectionResult(protectionResult, query); err != nil { + return nil, err + } } return m.next.Do(ctx, r) } +// applyProtectionResult performs the action indicated by the protection result. +// Returns an error if the query should be blocked, nil otherwise. +func (m *protectionMiddleware) applyProtectionResult(result *ProtectionResult, query string) error { + switch result.Action { + case RuleActionBlock: + return httpgrpc.Errorf(http.StatusBadRequest, "query blocked by protection rule: %s", result.RuleName) + case RuleActionLog: + level.Info(m.logger).Log("msg", "protection rule triggered", "rule", result.RuleName, "action", "log", "query", query) + } + return nil +} + // extractActor returns the actor identifier from the request headers. // Uses X-Source header as the actor identifier. func extractActor(r queryrange.Request) string { From 97de279e117006aa6c7577687bd12103c742cc92 Mon Sep 17 00:00:00 2001 From: Yi Shi Date: Fri, 20 Mar 2026 18:30:48 +0000 Subject: [PATCH 2/4] add protection triggered counter metric and log action logging --- pkg/queryfrontend/protection_middleware.go | 26 +++++++++++++++------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/pkg/queryfrontend/protection_middleware.go b/pkg/queryfrontend/protection_middleware.go index d8fa07a865c..9619a5bb095 100644 --- a/pkg/queryfrontend/protection_middleware.go +++ b/pkg/queryfrontend/protection_middleware.go @@ -19,10 +19,11 @@ import ( ) type protectionMiddleware struct { - next queryrange.Handler - engine *ProtectionEngine - logger log.Logger - totalLatency prometheus.Histogram + next queryrange.Handler + engine *ProtectionEngine + logger log.Logger + totalLatency prometheus.Histogram + triggeredTotal *prometheus.CounterVec } // NewProtectionMiddleware creates a new middleware that applies protection rules to queries. @@ -34,13 +35,20 @@ func NewProtectionMiddleware(engine *ProtectionEngine, logger log.Logger, reg pr Help: "Total duration of the protection middleware, including parsing and evaluation.", Buckets: []float64{0.0001, 0.00025, 0.0005, 0.001, 0.005, 0.01, 0.1, 1.0, 10.0}, }) + triggeredTotal := promauto.With(reg).NewCounterVec(prometheus.CounterOpts{ + Namespace: "thanos", + Subsystem: "query_frontend", + Name: "protection_triggered_total", + Help: "Total number of queries matched by a protection rule, labeled by action and rule name.", + }, []string{"action", "rule"}) return queryrange.MiddlewareFunc(func(next queryrange.Handler) queryrange.Handler { return &protectionMiddleware{ - next: next, - engine: engine, - logger: logger, - totalLatency: totalLatency, + next: next, + engine: engine, + logger: logger, + totalLatency: totalLatency, + triggeredTotal: triggeredTotal, } }) } @@ -93,8 +101,10 @@ func (m *protectionMiddleware) Do(ctx context.Context, r queryrange.Request) (qu func (m *protectionMiddleware) applyProtectionResult(result *ProtectionResult, query string) error { switch result.Action { case RuleActionBlock: + m.triggeredTotal.WithLabelValues("block", result.RuleName).Inc() return httpgrpc.Errorf(http.StatusBadRequest, "query blocked by protection rule: %s", result.RuleName) case RuleActionLog: + m.triggeredTotal.WithLabelValues("log", result.RuleName).Inc() level.Info(m.logger).Log("msg", "protection rule triggered", "rule", result.RuleName, "action", "log", "query", query) } return nil From 5978fb75671ced64347848ae36ea4cad0042fa7a Mon Sep 17 00:00:00 2001 From: Yi Shi Date: Fri, 20 Mar 2026 20:30:11 +0000 Subject: [PATCH 3/4] change block return 400 to return 403 to align with existing bad query block --- pkg/queryfrontend/protection_middleware.go | 2 +- pkg/queryfrontend/protection_middleware_test.go | 4 ++-- test/e2e/query_frontend_test.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/queryfrontend/protection_middleware.go b/pkg/queryfrontend/protection_middleware.go index 9619a5bb095..e19cee5b648 100644 --- a/pkg/queryfrontend/protection_middleware.go +++ b/pkg/queryfrontend/protection_middleware.go @@ -102,7 +102,7 @@ func (m *protectionMiddleware) applyProtectionResult(result *ProtectionResult, q switch result.Action { case RuleActionBlock: m.triggeredTotal.WithLabelValues("block", result.RuleName).Inc() - return httpgrpc.Errorf(http.StatusBadRequest, "query blocked by protection rule: %s", result.RuleName) + return httpgrpc.Errorf(http.StatusForbidden, "query blocked by protection rule: %s", result.RuleName) case RuleActionLog: m.triggeredTotal.WithLabelValues("log", result.RuleName).Inc() level.Info(m.logger).Log("msg", "protection rule triggered", "rule", result.RuleName, "action", "log", "query", query) diff --git a/pkg/queryfrontend/protection_middleware_test.go b/pkg/queryfrontend/protection_middleware_test.go index 817f317218d..004ccd39502 100644 --- a/pkg/queryfrontend/protection_middleware_test.go +++ b/pkg/queryfrontend/protection_middleware_test.go @@ -87,7 +87,7 @@ func TestProtectionMiddleware_RuleActionLog_PassesThrough(t *testing.T) { require.True(t, called) } -func TestProtectionMiddleware_RuleActionBlock_Returns400(t *testing.T) { +func TestProtectionMiddleware_RuleActionBlock_Returns403(t *testing.T) { called := false next := queryrange.HandlerFunc(func(ctx context.Context, r queryrange.Request) (queryrange.Response, error) { called = true @@ -104,7 +104,7 @@ func TestProtectionMiddleware_RuleActionBlock_Returns400(t *testing.T) { httpErr, ok := httpgrpc.HTTPResponseFromError(err) require.True(t, ok) - require.Equal(t, int32(http.StatusBadRequest), httpErr.Code) + require.Equal(t, int32(http.StatusForbidden), httpErr.Code) } func TestProtectionMiddleware_ProtectionResultInContext(t *testing.T) { diff --git a/test/e2e/query_frontend_test.go b/test/e2e/query_frontend_test.go index 55f5ed17b8f..2434db7a0ca 100644 --- a/test/e2e/query_frontend_test.go +++ b/test/e2e/query_frontend_test.go @@ -1215,7 +1215,7 @@ func TestQueryFrontendProtection(t *testing.T) { q := e2ethanos.NewQuerierBuilder(e, "1").Init() testutil.Ok(t, e2e.StartAndWaitReady(q)) - t.Run("block action returns 400", func(t *testing.T) { + t.Run("block action returns 403", func(t *testing.T) { qfe := e2ethanos.NewQueryFrontend(e, "block", "http://"+q.InternalEndpoint("http"), queryfrontend.Config{}, queryfrontend.CacheProviderConfig{Type: queryfrontend.INMEMORY}, @@ -1237,7 +1237,7 @@ rules: require.NoError(t, err) t.Cleanup(func() { require.NoError(t, resp.Body.Close()) }) - require.Equal(t, http.StatusBadRequest, resp.StatusCode) + require.Equal(t, http.StatusForbidden, resp.StatusCode) }) t.Run("log action passes through", func(t *testing.T) { From 0acf6f4742ffecc1258f51290bb239ab5f3ccfc5 Mon Sep 17 00:00:00 2001 From: Yi Shi Date: Fri, 20 Mar 2026 21:39:54 +0000 Subject: [PATCH 4/4] lint --- pkg/queryfrontend/protection_middleware.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/queryfrontend/protection_middleware.go b/pkg/queryfrontend/protection_middleware.go index e19cee5b648..ef201116916 100644 --- a/pkg/queryfrontend/protection_middleware.go +++ b/pkg/queryfrontend/protection_middleware.go @@ -19,11 +19,11 @@ import ( ) type protectionMiddleware struct { - next queryrange.Handler - engine *ProtectionEngine - logger log.Logger - totalLatency prometheus.Histogram - triggeredTotal *prometheus.CounterVec + next queryrange.Handler + engine *ProtectionEngine + logger log.Logger + totalLatency prometheus.Histogram + triggeredTotal *prometheus.CounterVec } // NewProtectionMiddleware creates a new middleware that applies protection rules to queries.