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.

Sign up for GitHub

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 remoteUser (from http basic auth) and remoteAddr to Thanos slow query logs #6153

Merged
merged 1 commit into from
Feb 24, 2023
Merged

Add remoteUser (from http basic auth) and remoteAddr to Thanos slow query logs #6153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
- [#6104](https://github.com/thanos-io/thanos/pull/6104) Objstore: Support S3 session token.
- [#5548](https://github.com/thanos-io/thanos/pull/5548) Query: Added experimental support for load balancing across multiple Store endpoints.
- [#6148](https://github.com/thanos-io/thanos/pull/6148) Query-frontend: add traceID to slow query detected log line
- [#6153](https://github.com/thanos-io/thanos/pull/6153) Query-frontend: add remote_user (from http basic auth) and remote_addr to slow query detected log line

### Fixed

Expand Down
7 changes: 7 additions & 0 deletions internal/cortex/frontend/transport/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,15 @@ func (f *Handler) reportSlowQuery(r *http.Request, responseHeaders http.Header,
thanosTraceID = traceID
}

remoteUser, _, _ := r.BasicAuth()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the only way we can get the remote user? Are there maybe some Grafana headers we can use as fallback when basic auth is not used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Grafana can be but one client. It's highly possible that users may use custom clients to query some data from Thanos.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we indicate in the changelog that this only works for basic auth?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added mention of basic auth


logMessage := append([]interface{}{
"msg", "slow query detected",
"method", r.Method,
"host", r.Host,
"path", r.URL.Path,
"remote_user", remoteUser,
"remote_addr", r.RemoteAddr,
"time_taken", queryResponseTime.String(),
"grafana_dashboard_uid", grafanaDashboardUID,
"grafana_panel_id", grafanaPanelID,
Expand All @@ -200,6 +204,7 @@ func (f *Handler) reportQueryStats(r *http.Request, queryString url.Values, quer
wallTime := stats.LoadWallTime()
numSeries := stats.LoadFetchedSeries()
numBytes := stats.LoadFetchedChunkBytes()
remoteUser, _, _ := r.BasicAuth()

// Track stats.
f.querySeconds.WithLabelValues(userID).Add(wallTime.Seconds())
Expand All @@ -213,6 +218,8 @@ func (f *Handler) reportQueryStats(r *http.Request, queryString url.Values, quer
"component", "query-frontend",
"method", r.Method,
"path", r.URL.Path,
"remote_user", remoteUser,
"remote_addr", r.RemoteAddr,
"response_time", queryResponseTime,
"query_wall_time_seconds", wallTime.Seconds(),
"fetched_series_count", numSeries,
Expand Down