From add36525a8b349584e264c56799401c10e13d606 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 29 Mar 2022 05:02:33 +0000 Subject: [PATCH 1/4] Support HTTP/2 with TLS in OTLP HTTP receiver. --- config/confighttp/confighttp.go | 2 ++ config/confighttp/confighttp_test.go | 30 +++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 8a062efdada..ad57bcc3cf4 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -24,6 +24,7 @@ import ( "github.com/rs/cors" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel" + "golang.org/x/net/http2" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config" @@ -232,6 +233,7 @@ func (hss *HTTPServerSettings) ToListener() (net.Listener, error) { if err != nil { return nil, err } + tlsCfg.NextProtos = []string{http2.NextProtoTLS} listener = tls.NewListener(listener, tlsCfg) } return listener, nil diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index a9b81fe1fa5..c82e93e3314 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -380,6 +380,7 @@ func TestHttpReception(t *testing.T) { tlsServerCreds *configtls.TLSServerSetting tlsClientCreds *configtls.TLSClientSetting hasError bool + forceHttp1 bool }{ { name: "noTLS", @@ -404,6 +405,23 @@ func TestHttpReception(t *testing.T) { ServerName: "localhost", }, }, + { + name: "TLS (HTTP/1.1)", + tlsServerCreds: &configtls.TLSServerSetting{ + TLSSetting: configtls.TLSSetting{ + CAFile: filepath.Join("testdata", "ca.crt"), + CertFile: filepath.Join("testdata", "server.crt"), + KeyFile: filepath.Join("testdata", "server.key"), + }, + }, + tlsClientCreds: &configtls.TLSClientSetting{ + TLSSetting: configtls.TLSSetting{ + CAFile: filepath.Join("testdata", "ca.crt"), + }, + ServerName: "localhost", + }, + forceHttp1: true, + }, { name: "NoServerCertificates", tlsServerCreds: &configtls.TLSServerSetting{ @@ -505,15 +523,24 @@ func TestHttpReception(t *testing.T) { <-time.After(10 * time.Millisecond) prefix := "https://" + expectedProto := "HTTP/2.0" if tt.tlsClientCreds.Insecure { prefix = "http://" + expectedProto = "HTTP/1.1" } hcs := &HTTPClientSettings{ Endpoint: prefix + ln.Addr().String(), TLSSetting: *tt.tlsClientCreds, } - client, errClient := hcs.ToClient(map[config.ComponentID]component.Extension{}, componenttest.NewNopTelemetrySettings()) + if tt.forceHttp1 { + expectedProto = "HTTP/1.1" + hcs.CustomRoundTripper = func(rt http.RoundTripper) (http.RoundTripper, error) { + rt.(*http.Transport).ForceAttemptHTTP2 = false + return rt, nil + } + } + client, errClient := hcs.ToClient(map[config.ComponentID]component.Extension{}, component.TelemetrySettings{}) require.NoError(t, errClient) resp, errResp := client.Get(hcs.Endpoint) @@ -524,6 +551,7 @@ func TestHttpReception(t *testing.T) { body, errRead := ioutil.ReadAll(resp.Body) assert.NoError(t, errRead) assert.Equal(t, "test", string(body)) + assert.Equal(t, expectedProto, resp.Proto) } require.NoError(t, s.Close()) }) From 777d34a79645708ee096c90062b0a764a76bef13 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 29 Mar 2022 05:54:32 +0000 Subject: [PATCH 2/4] Add benchmark --- CHANGELOG.md | 1 + config/confighttp/confighttp_test.go | 112 ++++++++++++++++++++++++++- 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 241f71e7acd..e32203726b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ In case of type mismatch, they don't panic right away but return an invalid zero-initialized instance for consistency with other OneOf field accessors (#5034) - Update OTLP to v0.15.0 (#5064) +- OTLP HTTP receiver will use HTTP/2 over TLS if client supports it (#5190) ### 🧰 Bug fixes 🧰 diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index c82e93e3314..3d4b71917eb 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -380,7 +380,7 @@ func TestHttpReception(t *testing.T) { tlsServerCreds *configtls.TLSServerSetting tlsClientCreds *configtls.TLSClientSetting hasError bool - forceHttp1 bool + forceHTTP1 bool }{ { name: "noTLS", @@ -420,7 +420,7 @@ func TestHttpReception(t *testing.T) { }, ServerName: "localhost", }, - forceHttp1: true, + forceHTTP1: true, }, { name: "NoServerCertificates", @@ -533,7 +533,7 @@ func TestHttpReception(t *testing.T) { Endpoint: prefix + ln.Addr().String(), TLSSetting: *tt.tlsClientCreds, } - if tt.forceHttp1 { + if tt.forceHTTP1 { expectedProto = "HTTP/1.1" hcs.CustomRoundTripper = func(rt http.RoundTripper) (http.RoundTripper, error) { rt.(*http.Transport).ForceAttemptHTTP2 = false @@ -947,3 +947,109 @@ type mockHost struct { func (nh *mockHost) GetExtensions() map[config.ComponentID]component.Extension { return nh.ext } + +func BenchmarkHttpRequest(b *testing.B) { + tests := []struct { + name string + forceHTTP1 bool + clientPerThread bool + }{ + { + name: "HTTP/2.0, shared client (like load balancer)", + forceHTTP1: false, + clientPerThread: false, + }, + { + name: "HTTP/1.1, shared client (like load balancer)", + forceHTTP1: true, + clientPerThread: false, + }, + { + name: "HTTP/2.0, client per thread (like single app)", + forceHTTP1: false, + clientPerThread: true, + }, + { + name: "HTTP/1.1, client per thread (like single app)", + forceHTTP1: true, + clientPerThread: true, + }, + } + + tlsServerCreds := &configtls.TLSServerSetting{ + TLSSetting: configtls.TLSSetting{ + CAFile: filepath.Join("testdata", "ca.crt"), + CertFile: filepath.Join("testdata", "server.crt"), + KeyFile: filepath.Join("testdata", "server.key"), + }, + } + tlsClientCreds := &configtls.TLSClientSetting{ + TLSSetting: configtls.TLSSetting{ + CAFile: filepath.Join("testdata", "ca.crt"), + }, + ServerName: "localhost", + } + + hss := &HTTPServerSettings{ + Endpoint: "localhost:0", + TLSSetting: tlsServerCreds, + } + + s, err := hss.ToServer( + componenttest.NewNopHost(), + componenttest.NewNopTelemetrySettings(), + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, errWrite := fmt.Fprint(w, "test") + require.NoError(b, errWrite) + })) + require.NoError(b, err) + ln, err := hss.ToListener() + require.NoError(b, err) + + go func() { + _ = s.Serve(ln) + }() + defer func() { + _ = s.Close() + }() + + // Wait for the servers to start + <-time.After(10 * time.Millisecond) + + for _, bb := range tests { + hcs := &HTTPClientSettings{ + Endpoint: "https://" + ln.Addr().String(), + TLSSetting: *tlsClientCreds, + } + if bb.forceHTTP1 { + hcs.CustomRoundTripper = func(rt http.RoundTripper) (http.RoundTripper, error) { + rt.(*http.Transport).ForceAttemptHTTP2 = false + return rt, nil + } + } + b.Run(bb.name, func(b *testing.B) { + var c *http.Client + if !bb.clientPerThread { + c, err = hcs.ToClient(map[config.ComponentID]component.Extension{}, component.TelemetrySettings{}) + require.NoError(b, err) + } + b.RunParallel(func(pb *testing.PB) { + if c == nil { + c, err = hcs.ToClient(map[config.ComponentID]component.Extension{}, component.TelemetrySettings{}) + require.NoError(b, err) + } + for pb.Next() { + resp, errResp := c.Get(hcs.Endpoint) + require.NoError(b, errResp) + body, errRead := ioutil.ReadAll(resp.Body) + _ = resp.Body.Close() + require.NoError(b, errRead) + require.Equal(b, "test", string(body)) + } + c.CloseIdleConnections() + }) + // Wait for connections to close before closing server to prevent log spam + <-time.After(10 * time.Millisecond) + }) + } +} From 0acba8dfc7aded71451c7cafd537ff5806715014 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Thu, 31 Mar 2022 02:54:01 +0000 Subject: [PATCH 3/4] Explicit http/1 fallback proto --- config/confighttp/confighttp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index ad57bcc3cf4..a5465343160 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -233,7 +233,7 @@ func (hss *HTTPServerSettings) ToListener() (net.Listener, error) { if err != nil { return nil, err } - tlsCfg.NextProtos = []string{http2.NextProtoTLS} + tlsCfg.NextProtos = []string{http2.NextProtoTLS, "http/1.1"} listener = tls.NewListener(listener, tlsCfg) } return listener, nil From 34c5561a4ae3e9c94093998bdb588922ce411f4d Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Fri, 1 Apr 2022 07:45:10 -0700 Subject: [PATCH 4/4] Update CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf6afa586dd..d87b8872bcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ### 💡 Enhancements 💡 +- OTLP HTTP receiver will use HTTP/2 over TLS if client supports it (#5190) + ### 🧰 Bug fixes 🧰 ## v0.48.0 Beta @@ -77,7 +79,6 @@ instance for consistency with other OneOf field accessors (#5035) - Update OTLP to v0.15.0 (#5064) - Adding support for transition from older versions of OTLP to OTLP v0.15.0 (#5085) -- OTLP HTTP receiver will use HTTP/2 over TLS if client supports it (#5190) ### 🧰 Bug fixes 🧰