Skip to content

Commit d0cf961

Browse files
committed
QFE: only log slow query, if it is a query endpoint
Signed-off-by: Pedro Tanaka <[email protected]>
1 parent 731e460 commit d0cf961

File tree

2 files changed

+106
-1
lines changed

2 files changed

+106
-1
lines changed

internal/cortex/frontend/transport/handler.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,9 @@ func (f *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
148148
}
149149

150150
// Check whether we should parse the query string.
151-
shouldReportSlowQuery := f.cfg.LogQueriesLongerThan != 0 && queryResponseTime > f.cfg.LogQueriesLongerThan
151+
shouldReportSlowQuery := f.cfg.LogQueriesLongerThan != 0 &&
152+
queryResponseTime > f.cfg.LogQueriesLongerThan &&
153+
isQueryEndpoint(r.URL.Path)
152154
if shouldReportSlowQuery || f.cfg.QueryStatsEnabled {
153155
queryString = f.parseRequestQueryString(r, buf)
154156
}
@@ -161,6 +163,13 @@ func (f *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
161163
}
162164
}
163165

166+
// isQueryEndpoint returns true if the path is any of the Prometheus HTTP API,
167+
// query-related endpoints.
168+
// Example: /api/v1/query, /api/v1/query_range, /api/v1/series, /api/v1/label, /api/v1/labels
169+
func isQueryEndpoint(path string) bool {
170+
return strings.HasPrefix(path, "/api/v1/query") || strings.HasPrefix(path, "/api/v1/series") || strings.HasPrefix(path, "/api/v1/label")
171+
}
172+
164173
// reportSlowQuery reports slow queries.
165174
func (f *Handler) reportSlowQuery(r *http.Request, responseHeaders http.Header, queryString url.Values, queryResponseTime time.Duration) {
166175
// NOTE(GiedriusS): see https://github.com/grafana/grafana/pull/60301 for more info.
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package transport
2+
3+
import (
4+
"bytes"
5+
"io"
6+
"net/http"
7+
"net/http/httptest"
8+
"testing"
9+
"time"
10+
11+
"github.com/go-kit/log"
12+
"github.com/prometheus/client_golang/prometheus"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
type fakeRoundTripper struct {
17+
response *http.Response
18+
err error
19+
requestLatency time.Duration
20+
}
21+
22+
func (f *fakeRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) {
23+
time.Sleep(f.requestLatency)
24+
return f.response, f.err
25+
}
26+
27+
func TestHandler_SlowQueryLog(t *testing.T) {
28+
cfg := HandlerConfig{
29+
QueryStatsEnabled: true,
30+
LogQueriesLongerThan: 1 * time.Second,
31+
}
32+
fakeRoundTripper := &fakeRoundTripper{
33+
requestLatency: 2 * time.Second,
34+
response: &http.Response{
35+
StatusCode: http.StatusOK,
36+
Header: http.Header{
37+
"Content-Type": []string{"application/json"},
38+
"Server-Timing": []string{"querier;dur=1.23"},
39+
},
40+
Body: io.NopCloser(bytes.NewBufferString(`{}`)),
41+
},
42+
}
43+
44+
tests := []struct {
45+
name string
46+
url string
47+
logParts []string
48+
}{
49+
{
50+
name: "Basic query",
51+
url: "/api/v1/query?query=absent(up)&start=1714262400&end=1714266000",
52+
logParts: []string{
53+
"slow query detected",
54+
"time_taken=2",
55+
"path=/api/v1/query",
56+
"param_query=absent(up)",
57+
"param_start=1714262400",
58+
"param_end=1714266000",
59+
},
60+
},
61+
{
62+
name: "Query with different parameters",
63+
url: "/api/v1/query_range?query=rate(http_requests_total[5m])&start=1714262400&end=1714266000&step=15",
64+
logParts: []string{
65+
"slow query detected",
66+
"time_taken=2",
67+
"path=/api/v1/query_range",
68+
"param_query=rate(http_requests_total[5m])",
69+
"param_start=1714262400",
70+
"param_end=1714266000",
71+
"param_step=15",
72+
},
73+
},
74+
{
75+
name: "Non-query endpoint",
76+
url: "/api/v1/status/config",
77+
// No slow query log for non-query endpoints
78+
logParts: []string{},
79+
},
80+
}
81+
82+
for _, tt := range tests {
83+
t.Run(tt.name, func(t *testing.T) {
84+
logWriter := &bytes.Buffer{}
85+
logger := log.NewLogfmtLogger(log.NewSyncWriter(logWriter))
86+
87+
handler := NewHandler(cfg, fakeRoundTripper, logger, prometheus.NewRegistry())
88+
89+
handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", tt.url, nil))
90+
91+
for _, part := range tt.logParts {
92+
require.Contains(t, logWriter.String(), part)
93+
}
94+
})
95+
}
96+
}

0 commit comments

Comments
 (0)