-
Notifications
You must be signed in to change notification settings - Fork 93
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 http collector timeouts; fixes #1064 #1310
Conversation
68396c5
to
c9fbb97
Compare
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.
Your changes LGTM. I added one comment to address an improvement to some of the old code
I suggest we add some tests here. There are no tests for this collector unfortunately, so its a start from scratch
Probably on your radar already, either way, remember to create a PR with the docs change. The collector parameters do not mention the HTTP methods (again, legacy issue). I suggest we add this to the docs as we add this new parameter so that the documentation represents such a hierachy
I did something like this replicatedhq/troubleshoot.sh#522 |
My change led to the docs looking like https://troubleshoot.sh/docs/collect/custom-metrics/#parameters |
@banjoh I'll make the changes as per our discussions but really would've written it like this: collectors:
- http:
collectorName: healthz
url: https://example.com/
method: get
headers: ... ..for these purposes:
Not sure what this breaks though (support bundle analysis, breaking changes in customer envs etc), so I wouldn't do it right away. |
e58f8b9
to
4c058c9
Compare
86c9de2
to
6122a93
Compare
Agreed. It would look cleaner this way but the migration effort to adopt this outweighs the benefits IMHO |
6122a93
to
5b35b43
Compare
Signed-off-by: Archit Sharma <archit@replicated.com>
…#1064) Signed-off-by: Archit Sharma <archit@replicated.com>
Signed-off-by: Archit Sharma <archit@replicated.com>
a054d83
to
200d155
Compare
200d155
to
8cf0758
Compare
420a5d1
to
c39e7f7
Compare
Signed-off-by: Archit Sharma <archit@replicated.com>
c39e7f7
to
4c18d19
Compare
Description, Motivation and Context
add http collector timeouts
Fixes #1064
Checklist
Does this PR introduce a breaking change?