Skip to content

fix(query-frontend): ensure slow query and stats logging on request failures#8618

Merged
GiedriusS merged 2 commits intothanos-io:mainfrom
pvlltvk:fix/ensure-log-slow-query
Jan 22, 2026
Merged

fix(query-frontend): ensure slow query and stats logging on request failures#8618
GiedriusS merged 2 commits intothanos-io:mainfrom
pvlltvk:fix/ensure-log-slow-query

Conversation

@pvlltvk
Copy link
Copy Markdown
Contributor

@pvlltvk pvlltvk commented Jan 3, 2026

This PR fix the query-frontend handler to guarantee that slow query logging and query statistics are reported even when requests fail or panic.

Fixes: #6219

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

All changes in internal/cortex/frontend/transport/handler.go:

  • Extract reporting logic into a dedicated reportQueryStatsAndSlowQueries function for clarity
  • Wrap reportQueryStatsAndSlowQueries in a defer to ensure it runs regardless of how the request completes (success, error, or panic)
  • Fix bug in reportQueryStats where return values from addStatsToLogMessage and addQueryRangeToLogMessage were ignored, causing stats and query range to be missing from log output
  • Add a test for this case

Verification

  • make lint
  • make format
  • built and ran on a test env with some real cases - works as expected

Ensure we log slow queries and query statistics regardless of how
the request completes

Signed-off-by: Pavel Litvyak <pvlltvk@gmail.com>
@pvlltvk
Copy link
Copy Markdown
Contributor Author

pvlltvk commented Jan 16, 2026

@MichaHoffmann @GiedriusS
Sorry to mention you in a comment, but it's been a while since the PR was opened. Perhaps you could take a look at it? 🙂

@GiedriusS
Copy link
Copy Markdown
Member

@MichaHoffmann @GiedriusS Sorry to mention you in a comment, but it's been a while since the PR was opened. Perhaps you could take a look at it? 🙂

Thanks for the ping and sorry for the delay. It's on my todo list.

GiedriusS
GiedriusS previously approved these changes Jan 22, 2026
Copy link
Copy Markdown
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thank you 🙇

…uery

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
@GiedriusS GiedriusS merged commit 9e0fc13 into thanos-io:main Jan 22, 2026
21 of 25 checks passed
pvlltvk added a commit to pvlltvk/thanos that referenced this pull request Apr 9, 2026
PR thanos-io#8618 moved slow query logging into a defer and switched from
using w.Header() (ResponseWriter headers) to resp.Header (round-trip
response headers). The X-Thanos-Trace-Id header is set on w.Header()
by the tracing middleware before the handler runs, so it was never
present in resp.Header, causing trace IDs to be missing from slow
query logs.

Pass w.Header() to reportQueryStatsAndSlowQueries instead of the
response object so the trace ID is available on both success and
error paths.

Fixes: thanos-io#8760

Signed-off-by: Pavel Litvyak <pvlltvk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Thanos query logging skips failed queries.

2 participants