Skip to content
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

Enable HTTP/2 again #261

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions config/http_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ func newClient(rt http.RoundTripper) *http.Client {

// NewClientFromConfig returns a new HTTP client configured for the
// given config.HTTPClientConfig. The name is used as go-conntrack metric label.
func NewClientFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives, enableHTTP2 bool) (*http.Client, error) {
rt, err := NewRoundTripperFromConfig(cfg, name, disableKeepAlives, enableHTTP2)
func NewClientFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives bool) (*http.Client, error) {
rt, err := NewRoundTripperFromConfig(cfg, name, disableKeepAlives)
if err != nil {
return nil, err
}
Expand All @@ -151,7 +151,7 @@ func NewClientFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives, e

// NewRoundTripperFromConfig returns a new HTTP RoundTripper configured for the
// given config.HTTPClientConfig. The name is used as go-conntrack metric label.
func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives, enableHTTP2 bool) (http.RoundTripper, error) {
func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives bool) (http.RoundTripper, error) {
newRT := func(tlsConfig *tls.Config) (http.RoundTripper, error) {
// The only timeout we care about is the configured scrape timeout.
// It is applied on request. So we leave out any timings here.
Expand All @@ -172,19 +172,18 @@ func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAli
conntrack.DialWithName(name),
),
}
if enableHTTP2 {
// HTTP/2 support is golang has many problematic cornercases where
// dead connections would be kept and used in connection pools.
// https://github.com/golang/go/issues/32388
// https://github.com/golang/go/issues/39337
// https://github.com/golang/go/issues/39750
// TODO: Re-Enable HTTP/2 once upstream issue is fixed.
// TODO: use ForceAttemptHTTP2 when we move to Go 1.13+.
err := http2.ConfigureTransport(rt.(*http.Transport))
if err != nil {
return nil, err
}
// TODO: use ForceAttemptHTTP2 when we move to Go 1.13+.
rt2, err := http2.ConfigureTransports(rt.(*http.Transport))
if err != nil {
return nil, err
}
// ReadIdleTimeout is the timeout after which a health check using ping
// frame will be carried out if no frame is received on the
// connection. 5 minutes is above maximum sane scrape interval,
// we should not have this small overhead on the scrape connections.
// For other cases, this is used to validate that the connection can
// still be used.
rt2.ReadIdleTimeout = 5 * time.Minute

// If a bearer token is provided, create a round tripper that will set the
// Authorization header correctly on each request.
Expand Down
16 changes: 8 additions & 8 deletions config/http_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func TestNewClientFromConfig(t *testing.T) {
}
defer testServer.Close()

client, err := NewClientFromConfig(validConfig.clientConfig, "test", false, true)
client, err := NewClientFromConfig(validConfig.clientConfig, "test", false)
if err != nil {
t.Errorf("Can't create a client from this config: %+v", validConfig.clientConfig)
continue
Expand Down Expand Up @@ -255,7 +255,7 @@ func TestNewClientFromInvalidConfig(t *testing.T) {
}

for _, invalidConfig := range newClientInvalidConfig {
client, err := NewClientFromConfig(invalidConfig.clientConfig, "test", false, true)
client, err := NewClientFromConfig(invalidConfig.clientConfig, "test", false)
if client != nil {
t.Errorf("A client instance was returned instead of nil using this config: %+v", invalidConfig.clientConfig)
}
Expand Down Expand Up @@ -294,7 +294,7 @@ func TestMissingBearerAuthFile(t *testing.T) {
}
defer testServer.Close()

client, err := NewClientFromConfig(cfg, "test", false, true)
client, err := NewClientFromConfig(cfg, "test", false)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -483,7 +483,7 @@ func TestBasicAuthNoPassword(t *testing.T) {
if err != nil {
t.Fatalf("Error loading HTTP client config: %v", err)
}
client, err := NewClientFromConfig(*cfg, "test", false, true)
client, err := NewClientFromConfig(*cfg, "test", false)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand All @@ -509,7 +509,7 @@ func TestBasicAuthNoUsername(t *testing.T) {
if err != nil {
t.Fatalf("Error loading HTTP client config: %v", err)
}
client, err := NewClientFromConfig(*cfg, "test", false, true)
client, err := NewClientFromConfig(*cfg, "test", false)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand All @@ -535,7 +535,7 @@ func TestBasicAuthPasswordFile(t *testing.T) {
if err != nil {
t.Fatalf("Error loading HTTP client config: %v", err)
}
client, err := NewClientFromConfig(*cfg, "test", false, true)
client, err := NewClientFromConfig(*cfg, "test", false)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand Down Expand Up @@ -686,7 +686,7 @@ func TestTLSRoundTripper(t *testing.T) {
writeCertificate(bs, tc.cert, cert)
writeCertificate(bs, tc.key, key)
if c == nil {
c, err = NewClientFromConfig(cfg, "test", false, true)
c, err = NewClientFromConfig(cfg, "test", false)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand Down Expand Up @@ -758,7 +758,7 @@ func TestTLSRoundTripperRaces(t *testing.T) {
writeCertificate(bs, TLSCAChainPath, ca)
writeCertificate(bs, ClientCertificatePath, cert)
writeCertificate(bs, ClientKeyNoPassPath, key)
c, err = NewClientFromConfig(cfg, "test", false, true)
c, err = NewClientFromConfig(cfg, "test", false)
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ require (
github.com/prometheus/client_golang v1.7.1
github.com/prometheus/client_model v0.2.0
github.com/sirupsen/logrus v1.6.0
golang.org/x/net v0.0.0-20200625001655-4c5254603344
golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae
golang.org/x/net v0.0.0-20201024042810-be3efd7ff127
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f
gopkg.in/alecthomas/kingpin.v2 v2.2.6
gopkg.in/yaml.v2 v2.3.0
)
Expand Down
Loading