-
Notifications
You must be signed in to change notification settings - Fork 487
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.
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 metadata to User-Agent header #5774
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just a few small comments about style
pkg/useragent/useragent.go
Outdated
// package useragent provides a consistent way to get a user agent for outbound http requests from Grafana Agent. | ||
// The default User-Agent is `GrafanaAgent/$VERSION($MODE)` | ||
// Where version is the build version of the agent and MODE is one of "static" or "flow". | ||
package useragent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this in internal/useragent
instead of pkg/useragent
? Since we're heading towards 1.0 where we're unlikely to have a large public API (or any public API at all), I'd like to get in the habit of only exposing what absolutely needs to be made public.
pkg/useragent/useragent.go
Outdated
metadata = append(metadata, op) | ||
} | ||
if len(metadata) > 0 { | ||
parenthesis = fmt.Sprintf(" (%s)", strings.Join(metadata, ";")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about using strings.Join(metadata, "; ")
here (with the space after the semicolon)? That would help make the metadata slightly more human-readable:
GrafanaAgent/v1.2.3 (static; linux; operator)
vs
GrafanaAgent/v1.2.3 (static;linux;operator)
pkg/useragent/useragent.go
Outdated
// settable by tests | ||
var goos = runtime.GOOS | ||
|
||
func UserAgent() string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling this Get()
might be less repetitive so callers use useragent.Get()
instead of useragent.UserAgent()
flow
orstatic
)operator
if running in operator generated pod.AGENT_OPERATOR=1
to agent pods it creates to facilitate the above.Example User Agents:
GrafanaAgent/v1.2.3 (static; linux; operator)
GrafanaAgent/v1.2.3 (flow; windows)
GrafanaAgent/v1.2.3 (static; linux)
Why?
This will allow database operators (mimir,loki,pyroscope, etc.) to better understand agent traffic patterns sending them data.