-
Notifications
You must be signed in to change notification settings - Fork 342
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
set user agent on scrape requests to nginx. #70
Conversation
@asherf thanks for the PR! I suggest implementing this feature using a custom RoundTripper -- https://godoc.org/net/http#RoundTripper. There is an example implementation of a similar feature here https://github.com/kubernetes/kubernetes/blob/bc1d8c6d61078269540f807248a455cba09e672c/staging/src/k8s.io/client-go/transport/round_trippers.go#L144 Note that it is important to clone requests by doing a deep copy to adhere of the contract of the RoundTripper. With a RoundTripper, we will only have to change the httpClient in httpClient := &http.Client{
Timeout: timeout.Duration,
Transport: userAgentRT,
} where userAgentRT is a variable that holds a RoundTripper for setting the user-agent header As the result, both clients for NGINX and NGINX Plus will set the custom user-agent header. |
@pleshakov Thanks for that useful feedback! It took me a while (busy at work + I'm new to Go) |
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.
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
+1 to @pleshakov suggestions about readability and best practices.
Additionally, please rebase against master, there are conflicts in exporter.go
due to a PR merged recently.
Thanks for the contribution !
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.
@asherf thanks for making the changes. there a bug appeared after the PR was rebased against the master branch. otherwise, looks good!
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.
👍
Proposed changes
This changes the User-Agent header that is being sent when calling nginx stats endpoint.
Currently, the User-Agent is the default Go Http client user agent string (
Go-http-client/1.1
), which is not very useful when looking at nginx logs.This will change it to be
NGINX-Prometheus-Exporter/v0.5.0
(with the version we get when building the docker image)I have tested locally (with a custom docker image) in my k8s cluster (faked the version number when calling docker build).
(ignore the red color, this is just kubetail choosing random colors)
Checklist
Before creating a PR, run through this checklist and mark each as complete.