-
Notifications
You must be signed in to change notification settings - Fork 492
cmd/scollector: Set custom UserAgent and use facebookgo/httpcontrol for 60s RequestTimeout #1878
Conversation
|
Not ready to merge yet, as I plan on testing on a few systems for a while first. |
|
This is working now and ready for review. I am going to test it on a few systems to make sure the new 60s timeout doesn't cause any issues. Fixes #1879 |
|
I added another commit that will set the UA for the collect package when used in tsdbrelay and bosun. I'll also test these on my VM. |
collect/collect.go
Outdated
| UseNtlm bool = false | ||
|
|
||
| // DefaultClient can be used to override the HTTP client that will be used to make requests. | ||
| DefaultClient *http.Client |
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.
This is kinda wierd to me. Collect already has an odd client with a ghetto timeout thing (that looks buggy to me). Can we just have a central utility that sets http.DefaultClient?
Something like util.SetupHttpClient(appName, msg, features...)?
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.
I'd rather collect never had to know anything about http clients, and can just Get and Post to its heart's content.
|
In general, it looks pretty good. I'd just like to reduce some of the duplicated code if possible. |
|
After speaking with Craig the new commits remove the client code from collect (just use collect.DefaultClient to override) and update the scollector/docs.go file. We decided against a util function, since bosun already requires a custom transport and we may need to add additional custom headers in scollector/tsdbrelay the future. I'll test on a few systems then merge this to master. |
No description provided.