-
Notifications
You must be signed in to change notification settings - Fork 321
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
Disable HTTP/2 #249
Disable HTTP/2 #249
Conversation
How will the blackbox exporter turn it back on via this code? It doesn't do persistent connections, so those issues likely don't affect it. |
Signed-off-by: Julien Pivotto <[email protected]>
What about this? |
Let's not start hiding things in the YAML, we currently use function arguments for this - and the one that's there is already only used by blackbox. |
Signed-off-by: Julien Pivotto <[email protected]>
@brian-brazil I have adapted it. |
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.
👍
err := http2.ConfigureTransport(rt.(*http.Transport)) | ||
if err != nil { | ||
return nil, err | ||
if enableHTTP2 { |
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 add a comment that this should go away once Go is fixed?
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.
Actually if we add a comment, could we tight the disabling to the disableKeepAlives arg?
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.
In theory yes, but it feels pretty hacky.
Signed-off-by: Julien Pivotto <[email protected]>
I have put the golang issues in comment. |
- Disable HTTP2: prometheus/common#249 - Composite duration: prometheus/common#246 Signed-off-by: Julien Pivotto <[email protected]>
- Disable HTTP2: prometheus/common#249 - Composite duration: prometheus/common#246 Signed-off-by: Julien Pivotto <[email protected]>
- Disable HTTP2: prometheus/common#249 - Composite duration: prometheus/common#246 Signed-off-by: Julien Pivotto <[email protected]>
* Update changelog * Bump Go modules. * Update NewClientFromConfig use prometheus/common#249 added option to disable HTTP/2. Signed-off-by: Ben Kochie <[email protected]>
* Update changelog * Bump Go modules. * Update NewClientFromConfig use * Bump Go to 1.15. prometheus/common#249 added option to disable HTTP/2. Signed-off-by: Ben Kochie <[email protected]>
…ner-addr server: Expose `http` and `grpc` listen addresses.
HTTP/2 support is golang has many problematic cornercases where dead
connections would be kept.
golang/go#32388
golang/go#39337
golang/go#39750
I suggest we disable HTTP/2 for now and enable it manually on the
blackbox exporter.
Signed-off-by: Julien Pivotto [email protected]