Skip to content

Commit f1619db

Browse files
authored
Merge pull request #8762 from pvlltvk/fix/query-frontend-slow-query-trace-id
fix(query-frontend): restore trace ID in slow query logs
2 parents 349bc1b + 5e79de0 commit f1619db

3 files changed

Lines changed: 16 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ It is recommend to upgrade the storage components first (Receive, Store, etc.) a
2121
- [#8714](https://github.com/thanos-io/thanos/pull/8714): Tracing: Fix `tls_config` fields (`ca_file`, `cert_file`, `key_file`) being silently ignored when using the OTLP gRPC exporter. Previously, deployments using a private CA or mTLS client certificates had to work around this via `OTEL_EXPORTER_OTLP_CERTIFICATE` and related environment variables.
2222
- [#8128](https://github.com/thanos-io/thanos/issues/8128): Query-Frontend: Fix panic in `AnalyzesMerge` caused by indexing the wrong slice variable, leading to an out-of-range access when merging more than two query analyses.
2323
- [#8720](https://github.com/thanos-io/thanos/issues/8720): Receive: Fix 503 errors during restarts in some cases.
24+
- [#8762](https://github.com/thanos-io/thanos/pull/8762): Query-Frontend: Fix trace ID missing from slow query logs, regression from #8618.
2425
- [#8799](https://github.com/thanos-io/thanos/pull/8799): *: Set a `KeepaliveEnforcementPolicy` with `MinTime: 10s` on all gRPC servers, matching the client keepalive interval.
2526
- [#8806](https://github.com/thanos-io/thanos/pull/8806): Receive: Validate tenant IDs extracted from split-tenant labels to prevent path traversal.
2627
- [#8810](https://github.com/thanos-io/thanos/pull/8810): Ruler: correctly pass query partial response for gRPC.

internal/cortex/frontend/transport/handler.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func (f *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
127127

128128
// Ensure we log slow queries and query statistics regardless of how the request completes.
129129
defer func() {
130-
f.reportQueryStatsAndSlowQueries(r, resp, buf, startTime, stats)
130+
f.reportQueryStatsAndSlowQueries(r, w.Header(), buf, startTime, stats)
131131
}()
132132

133133
var err error
@@ -153,7 +153,7 @@ func (f *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
153153
}
154154

155155
// reportQueryStatsAndSlowQueries handles logging of slow queries and query statistics.
156-
func (f *Handler) reportQueryStatsAndSlowQueries(r *http.Request, resp *http.Response, buf bytes.Buffer, startTime time.Time, stats *querier_stats.Stats) {
156+
func (f *Handler) reportQueryStatsAndSlowQueries(r *http.Request, responseHeaders http.Header, buf bytes.Buffer, startTime time.Time, stats *querier_stats.Stats) {
157157
queryResponseTime := time.Since(startTime)
158158

159159
shouldReportSlowQuery := f.cfg.LogQueriesLongerThan != 0 &&
@@ -164,10 +164,6 @@ func (f *Handler) reportQueryStatsAndSlowQueries(r *http.Request, resp *http.Res
164164
queryString := f.parseRequestQueryString(r, buf)
165165

166166
if shouldReportSlowQuery {
167-
var responseHeaders http.Header
168-
if resp != nil {
169-
responseHeaders = resp.Header
170-
}
171167
f.reportSlowQuery(r, responseHeaders, queryString, queryResponseTime, stats)
172168
}
173169
if f.cfg.QueryStatsEnabled {

internal/cortex/frontend/transport/handler_test.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ func TestHandler_SlowQueryLog(t *testing.T) {
5050
"param_query=absent(up)",
5151
"param_start=1714262400",
5252
"param_end=1714266000",
53+
"trace_id=test-trace-id-123",
5354
},
5455
},
5556
{
@@ -114,7 +115,9 @@ func TestHandler_SlowQueryLog(t *testing.T) {
114115

115116
handler := NewHandler(cfg, fakeRoundTripper, logger, prometheus.NewRegistry())
116117

117-
handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", tt.url, nil))
118+
recorder := httptest.NewRecorder()
119+
recorder.Header().Set("X-Thanos-Trace-Id", "test-trace-id-123")
120+
handler.ServeHTTP(recorder, httptest.NewRequest("GET", tt.url, nil))
118121

119122
for _, part := range tt.logParts {
120123
require.Contains(t, logWriter.String(), part)
@@ -141,11 +144,19 @@ func TestHandler_SlowQueryLogOnError(t *testing.T) {
141144
logger := log.NewLogfmtLogger(log.NewSyncWriter(logWriter))
142145

143146
handler := NewHandler(cfg, fakeRT, logger, prometheus.NewRegistry())
144-
handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/api/v1/query?query=up", nil))
147+
148+
// Simulate tracing middleware setting the trace ID on the ResponseWriter
149+
// before the handler runs. The slow query log must include this trace ID
150+
// even when the round-tripper returns an error (resp is nil).
151+
recorder := httptest.NewRecorder()
152+
recorder.Header().Set("X-Thanos-Trace-Id", "test-trace-abc123")
153+
154+
handler.ServeHTTP(recorder, httptest.NewRequest("GET", "/api/v1/query?query=up", nil))
145155

146156
// Verify slow query is logged even when round-tripper returns an error
147157
require.Contains(t, logWriter.String(), "slow query detected")
148158
require.Contains(t, logWriter.String(), "time_taken=")
149159
require.Contains(t, logWriter.String(), "path=/api/v1/query")
150160
require.Contains(t, logWriter.String(), "param_query=up")
161+
require.Contains(t, logWriter.String(), "test-trace-abc123")
151162
}

0 commit comments

Comments
 (0)