From 5884ee8bdc320dbf0b532a1c45b20b7740092060 Mon Sep 17 00:00:00 2001 From: Matthew Walster Date: Sun, 12 May 2024 06:55:43 +0000 Subject: [PATCH 1/9] otelhttptrace: add TLS info to the "http.tls" span --- .../httptrace/otelhttptrace/clienttrace.go | 35 ++++++++++++++--- .../httptrace/otelhttptrace/httptrace_test.go | 13 ++++--- .../otelhttptrace/test/clienttrace_test.go | 39 +++++++++++++++++-- 3 files changed, 73 insertions(+), 14 deletions(-) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go index 67e03f24810..2b97c700963 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go @@ -5,16 +5,20 @@ package otelhttptrace // import "go.opentelemetry.io/contrib/instrumentation/net import ( "context" + "crypto/sha256" "crypto/tls" + "encoding/base64" + "fmt" "net/http/httptrace" "net/textproto" "strings" "sync" + "time" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" - semconv "go.opentelemetry.io/otel/semconv/v1.20.0" + semconv "go.opentelemetry.io/otel/semconv/v1.24.0" "go.opentelemetry.io/otel/trace" ) @@ -262,7 +266,7 @@ func (ct *clientTracer) span(hook string) trace.Span { } func (ct *clientTracer) getConn(host string) { - ct.start("http.getconn", "http.getconn", semconv.NetHostName(host)) + ct.start("http.getconn", "http.getconn", attribute.Key("net.host.name").String(host)) } func (ct *clientTracer) gotConn(info httptrace.GotConnInfo) { @@ -287,7 +291,7 @@ func (ct *clientTracer) gotFirstResponseByte() { } func (ct *clientTracer) dnsStart(info httptrace.DNSStartInfo) { - ct.start("http.dns", "http.dns", semconv.NetHostName(info.Host)) + ct.start("http.dns", "http.dns", attribute.Key("net.host.name").String(info.Host)) } func (ct *clientTracer) dnsDone(info httptrace.DNSDoneInfo) { @@ -316,8 +320,29 @@ func (ct *clientTracer) tlsHandshakeStart() { ct.start("http.tls", "http.tls") } -func (ct *clientTracer) tlsHandshakeDone(_ tls.ConnectionState, err error) { - ct.end("http.tls", err) +func (ct *clientTracer) tlsHandshakeDone(state tls.ConnectionState, err error) { + attrs := make([]attribute.KeyValue, 0, 7) + attrs = append(attrs, + semconv.TLSCipher(tls.CipherSuiteName(state.CipherSuite)), + semconv.TLSProtocolVersion(tls.VersionName(state.Version)), + semconv.TLSResumed(state.DidResume), + ) + + if len(state.PeerCertificates) > 0 { + certChain := make([]string, len(state.PeerCertificates)) + for i, cert := range state.PeerCertificates { + certChain[i] = base64.StdEncoding.EncodeToString(cert.Raw) + } + + leafCert := state.PeerCertificates[0] + attrs = append(attrs, + semconv.TLSServerCertificateChain(certChain...), + semconv.TLSServerHashSha256(fmt.Sprintf("%X", sha256.Sum256(leafCert.Raw))), + semconv.TLSServerNotAfter(leafCert.NotAfter.UTC().Format(time.RFC3339)), + semconv.TLSServerNotBefore(leafCert.NotBefore.UTC().Format(time.RFC3339)), + ) + } + ct.end("http.tls", err, attrs...) } func (ct *clientTracer) wroteHeaderField(k string, v []string) { diff --git a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go index faf81b85640..f724c484726 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go @@ -5,6 +5,7 @@ package otelhttptrace_test import ( "context" + "net" "net/http" "net/http/httptest" "strings" @@ -30,7 +31,7 @@ func TestRoundtrip(t *testing.T) { props := otelhttptrace.WithPropagators(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) // Mock http server - ts := httptest.NewServer( + ts := httptest.NewTLSServer( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { attrs, corrs, span := otelhttptrace.Extract(r.Context(), r, props) @@ -69,16 +70,16 @@ func TestRoundtrip(t *testing.T) { defer ts.Close() address := ts.Listener.Addr() - hp := strings.Split(address.String(), ":") + host, port, _ := net.SplitHostPort(address.String()) expectedAttrs = map[attribute.Key]string{ - semconv.NetHostNameKey: hp[0], - semconv.NetHostPortKey: hp[1], + semconv.NetHostNameKey: host, + semconv.NetHostPortKey: port, semconv.NetProtocolVersionKey: "1.1", semconv.HTTPMethodKey: "GET", - semconv.HTTPSchemeKey: "http", + semconv.HTTPSchemeKey: "https", semconv.HTTPTargetKey: "/", semconv.HTTPRequestContentLengthKey: "3", - semconv.NetSockPeerAddrKey: hp[0], + semconv.NetSockPeerAddrKey: host, semconv.NetTransportKey: "ip_tcp", semconv.UserAgentOriginalKey: "Go-http-client/1.1", } diff --git a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go index 846f4fd128f..0e070f91696 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go @@ -6,20 +6,24 @@ package test import ( "bytes" "context" + "crypto/sha256" + "encoding/base64" + "fmt" "net/http" "net/http/httptest" "net/http/httptrace" + "slices" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" + semconv "go.opentelemetry.io/otel/semconv/v1.24.0" ) func getSpanFromRecorder(sr *tracetest.SpanRecorder, name string) (trace.ReadOnlySpan, bool) { @@ -48,7 +52,7 @@ func TestHTTPRequestWithClientTrace(t *testing.T) { tr := tp.Tracer("httptrace/client") // Mock http server - ts := httptest.NewServer( + ts := httptest.NewTLSServer( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { }), ) @@ -79,6 +83,21 @@ func TestHTTPRequestWithClientTrace(t *testing.T) { attributes []attribute.KeyValue parent string }{ + { + name: "http.tls", + attributes: []attribute.KeyValue{ + attribute.Key("tls.server.certificate_chain").StringSlice( + []string{base64.StdEncoding.EncodeToString(ts.Certificate().Raw)}, + ), + attribute.Key("tls.server.hash.sha256"). + String(fmt.Sprintf("%X", sha256.Sum256(ts.Certificate().Raw))), + attribute.Key("tls.server.not_after"). + String(ts.Certificate().NotAfter.UTC().Format(time.RFC3339)), + attribute.Key("tls.server.not_before"). + String(ts.Certificate().NotBefore.UTC().Format(time.RFC3339)), + }, + parent: "http.getconn", + }, { name: "http.connect", attributes: []attribute.KeyValue{ @@ -115,7 +134,7 @@ func TestHTTPRequestWithClientTrace(t *testing.T) { name: "test", }, } - for _, tl := range testLen { + for i, tl := range testLen { span, ok := getSpanFromRecorder(sr, tl.name) if !assert.True(t, ok) { continue @@ -142,6 +161,20 @@ func TestHTTPRequestWithClientTrace(t *testing.T) { } assert.True(t, contains, "missing http.local attribute") } + if tl.name == "http.tls" { + if i == 0 { + tl.attributes = append(tl.attributes, attribute.Key("tls.resumed").Bool(false)) + } else { + tl.attributes = append(tl.attributes, attribute.Key("tls.resumed").Bool(true)) + } + attrs = slices.DeleteFunc(attrs, func(a attribute.KeyValue) bool { + // Skip keys that are unable to be detected beforehand. + if a.Key == semconv.TLSCipherKey || a.Key == semconv.TLSProtocolVersionKey { + return true + } + return false + }) + } assert.ElementsMatch(t, tl.attributes, attrs) } } From 444054a9051558bbe02440f3199122578e28c774 Mon Sep 17 00:00:00 2001 From: Matthew Walster Date: Sun, 12 May 2024 07:23:55 +0000 Subject: [PATCH 2/9] otelhttptrace: bump semconv versions This makes all the files in this package import the same version. --- instrumentation/net/http/httptrace/otelhttptrace/httptrace.go | 2 +- .../net/http/httptrace/otelhttptrace/httptrace_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/httptrace.go b/instrumentation/net/http/httptrace/otelhttptrace/httptrace.go index a1230c36abc..f4cac854d36 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/httptrace.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/httptrace.go @@ -12,7 +12,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/baggage" "go.opentelemetry.io/otel/propagation" - semconv "go.opentelemetry.io/otel/semconv/v1.20.0" + semconv "go.opentelemetry.io/otel/semconv/v1.24.0" "go.opentelemetry.io/otel/trace" ) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go index f724c484726..763f004bcb9 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go @@ -17,7 +17,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/baggage" "go.opentelemetry.io/otel/propagation" - semconv "go.opentelemetry.io/otel/semconv/v1.20.0" + semconv "go.opentelemetry.io/otel/semconv/v1.24.0" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" ) From 7945bc6b114f82f4299a214abac5e2a9d2223772 Mon Sep 17 00:00:00 2001 From: Matthew Walster Date: Sun, 19 May 2024 04:29:14 +0000 Subject: [PATCH 3/9] otelhttptrace: remove references to semconv v1.24 --- .../httptrace/otelhttptrace/clienttrace.go | 28 +++++++++++++------ .../http/httptrace/otelhttptrace/httptrace.go | 2 +- .../httptrace/otelhttptrace/httptrace_test.go | 2 +- .../otelhttptrace/test/clienttrace_test.go | 4 +-- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go index 2b97c700963..b0319a0458d 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go @@ -18,7 +18,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" - semconv "go.opentelemetry.io/otel/semconv/v1.24.0" "go.opentelemetry.io/otel/trace" ) @@ -40,6 +39,19 @@ var ( HTTPDNSAddrs = attribute.Key("http.dns.addrs") ) +// TLS attributes. +// +// From https://opentelemetry.io/docs/specs/semconv/attributes-registry/tls/ but only present from semconv v1.24. +var ( + TLSCipher = attribute.Key("tls.cipher") + TLSProtocolVersion = attribute.Key("tls.protocol.version") + TLSResumed = attribute.Key("tls.resumed") + TLSServerCertificateChain = attribute.Key("tls.server.certificate_chain") + TLSServerHashSha256 = attribute.Key("tls.server.hash.sha256") + TLSServerNotAfter = attribute.Key("tls.server.not_after") + TLSServerNotBefore = attribute.Key("tls.server.not_before") +) + var hookMap = map[string]string{ "http.dns": "http.getconn", "http.connect": "http.getconn", @@ -323,9 +335,9 @@ func (ct *clientTracer) tlsHandshakeStart() { func (ct *clientTracer) tlsHandshakeDone(state tls.ConnectionState, err error) { attrs := make([]attribute.KeyValue, 0, 7) attrs = append(attrs, - semconv.TLSCipher(tls.CipherSuiteName(state.CipherSuite)), - semconv.TLSProtocolVersion(tls.VersionName(state.Version)), - semconv.TLSResumed(state.DidResume), + TLSCipher.String(tls.CipherSuiteName(state.CipherSuite)), + TLSProtocolVersion.String(tls.VersionName(state.Version)), + TLSResumed.Bool(state.DidResume), ) if len(state.PeerCertificates) > 0 { @@ -336,10 +348,10 @@ func (ct *clientTracer) tlsHandshakeDone(state tls.ConnectionState, err error) { leafCert := state.PeerCertificates[0] attrs = append(attrs, - semconv.TLSServerCertificateChain(certChain...), - semconv.TLSServerHashSha256(fmt.Sprintf("%X", sha256.Sum256(leafCert.Raw))), - semconv.TLSServerNotAfter(leafCert.NotAfter.UTC().Format(time.RFC3339)), - semconv.TLSServerNotBefore(leafCert.NotBefore.UTC().Format(time.RFC3339)), + TLSServerCertificateChain.StringSlice(certChain), + TLSServerHashSha256.String(fmt.Sprintf("%X", sha256.Sum256(leafCert.Raw))), + TLSServerNotAfter.String(leafCert.NotAfter.UTC().Format(time.RFC3339)), + TLSServerNotBefore.String(leafCert.NotBefore.UTC().Format(time.RFC3339)), ) } ct.end("http.tls", err, attrs...) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/httptrace.go b/instrumentation/net/http/httptrace/otelhttptrace/httptrace.go index f4cac854d36..a1230c36abc 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/httptrace.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/httptrace.go @@ -12,7 +12,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/baggage" "go.opentelemetry.io/otel/propagation" - semconv "go.opentelemetry.io/otel/semconv/v1.24.0" + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" ) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go index 763f004bcb9..f724c484726 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go @@ -17,7 +17,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/baggage" "go.opentelemetry.io/otel/propagation" - semconv "go.opentelemetry.io/otel/semconv/v1.24.0" + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" ) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go index 0e070f91696..1a944d55102 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go @@ -18,12 +18,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" - semconv "go.opentelemetry.io/otel/semconv/v1.24.0" ) func getSpanFromRecorder(sr *tracetest.SpanRecorder, name string) (trace.ReadOnlySpan, bool) { @@ -169,7 +169,7 @@ func TestHTTPRequestWithClientTrace(t *testing.T) { } attrs = slices.DeleteFunc(attrs, func(a attribute.KeyValue) bool { // Skip keys that are unable to be detected beforehand. - if a.Key == semconv.TLSCipherKey || a.Key == semconv.TLSProtocolVersionKey { + if a.Key == otelhttptrace.TLSCipher || a.Key == otelhttptrace.TLSProtocolVersion { return true } return false From e30784e3e08f448c3e65031c81d1f4d828b1dc4f Mon Sep 17 00:00:00 2001 From: Matthew Walster Date: Tue, 21 May 2024 12:32:46 +0000 Subject: [PATCH 4/9] otelhttptrace: re-introduce semconv v1.20.0 --- .../net/http/httptrace/otelhttptrace/clienttrace.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go index b0319a0458d..09b7427f961 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go @@ -18,6 +18,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" ) @@ -278,7 +279,7 @@ func (ct *clientTracer) span(hook string) trace.Span { } func (ct *clientTracer) getConn(host string) { - ct.start("http.getconn", "http.getconn", attribute.Key("net.host.name").String(host)) + ct.start("http.getconn", "http.getconn", semconv.NetHostName(host)) } func (ct *clientTracer) gotConn(info httptrace.GotConnInfo) { @@ -303,7 +304,7 @@ func (ct *clientTracer) gotFirstResponseByte() { } func (ct *clientTracer) dnsStart(info httptrace.DNSStartInfo) { - ct.start("http.dns", "http.dns", attribute.Key("net.host.name").String(info.Host)) + ct.start("http.dns", "http.dns", semconv.NetHostName(info.Host)) } func (ct *clientTracer) dnsDone(info httptrace.DNSDoneInfo) { From 3469bf831819b24922f7136afc4aeeca4f4ef850 Mon Sep 17 00:00:00 2001 From: Matthew Walster Date: Tue, 21 May 2024 12:37:00 +0000 Subject: [PATCH 5/9] Changelog: otelhttptrace now has more TLS info --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73f285c97db..e6ea3b2994f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `WithSchemaURL` option function in `go.opentelemetry.io/contrib/bridges/otelslog`. This option function is used as a replacement of `WithInstrumentationScope` to specify the semantic convention schema URL for the logged records. (#5588) - Add support for Cloud Run jobs in `go.opentelemetry.io/contrib/detectors/gcp`. (#5559) +- Add TLS information to otelhttptrace http.tls attributes, providing information on the cipher and protocol version used, whether the session was resumed, the SHA256 hash of the leaf certificate, "not before"/"not after" dates, and verified certificate chains. ### Changed From aa211d31e5a93b76df0966b9bd232cde2a5d59fb Mon Sep 17 00:00:00 2001 From: Matthew Walster Date: Tue, 21 May 2024 13:06:17 +0000 Subject: [PATCH 6/9] otelhttptrace: revert change to httptrace_test --- .../net/http/httptrace/otelhttptrace/httptrace_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go index f724c484726..bd04f9c6aae 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go @@ -31,7 +31,7 @@ func TestRoundtrip(t *testing.T) { props := otelhttptrace.WithPropagators(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) // Mock http server - ts := httptest.NewTLSServer( + ts := httptest.NewServer( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { attrs, corrs, span := otelhttptrace.Extract(r.Context(), r, props) @@ -76,7 +76,7 @@ func TestRoundtrip(t *testing.T) { semconv.NetHostPortKey: port, semconv.NetProtocolVersionKey: "1.1", semconv.HTTPMethodKey: "GET", - semconv.HTTPSchemeKey: "https", + semconv.HTTPSchemeKey: "http", semconv.HTTPTargetKey: "/", semconv.HTTPRequestContentLengthKey: "3", semconv.NetSockPeerAddrKey: host, From 427c5c4bba12d3df32410e72b641193103f87942 Mon Sep 17 00:00:00 2001 From: Matthew Walster Date: Tue, 21 May 2024 13:39:04 +0000 Subject: [PATCH 7/9] otelhttptrace: repeat tests for http and https --- CHANGELOG.md | 2 +- .../otelhttptrace/test/clienttrace_test.go | 240 ++++++++++-------- 2 files changed, 130 insertions(+), 112 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6ea3b2994f..eaf466f00bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `WithSchemaURL` option function in `go.opentelemetry.io/contrib/bridges/otelslog`. This option function is used as a replacement of `WithInstrumentationScope` to specify the semantic convention schema URL for the logged records. (#5588) - Add support for Cloud Run jobs in `go.opentelemetry.io/contrib/detectors/gcp`. (#5559) -- Add TLS information to otelhttptrace http.tls attributes, providing information on the cipher and protocol version used, whether the session was resumed, the SHA256 hash of the leaf certificate, "not before"/"not after" dates, and verified certificate chains. +- Add TLS information from semantic conventions v1.24.0 to `go.opentelemetry.io/contrib/instrumentation/net/http/httptrace`. (#5563) ### Changed diff --git a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go index 1a944d55102..eef2d42d530 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go @@ -12,13 +12,13 @@ import ( "net/http" "net/http/httptest" "net/http/httptrace" + "net/url" "slices" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -46,136 +46,154 @@ func getSpansFromRecorder(sr *tracetest.SpanRecorder, name string) []trace.ReadO } func TestHTTPRequestWithClientTrace(t *testing.T) { - sr := tracetest.NewSpanRecorder() - tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) - otel.SetTracerProvider(tp) - tr := tp.Tracer("httptrace/client") - - // Mock http server - ts := httptest.NewTLSServer( + // Mock http server, one without TLS and another with TLS. + tsHTTP := httptest.NewServer( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { }), ) - defer ts.Close() - address := ts.Listener.Addr() + defer tsHTTP.Close() - client := ts.Client() - err := func(ctx context.Context) error { - ctx, span := tr.Start(ctx, "test") - defer span.End() - req, _ := http.NewRequest("GET", ts.URL, nil) - _, req = otelhttptrace.W3C(ctx, req) + tsHTTPS := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + }), + ) + defer tsHTTPS.Close() + + for _, ts := range []*httptest.Server{tsHTTP, tsHTTPS} { + sr := tracetest.NewSpanRecorder() + tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) + otel.SetTracerProvider(tp) + tr := tp.Tracer("httptrace/client") + + err := func(ctx context.Context) error { + ctx, span := tr.Start(ctx, "test") + defer span.End() + req, _ := http.NewRequest("GET", ts.URL, nil) + _, req = otelhttptrace.W3C(ctx, req) + + res, err := ts.Client().Do(req) + if err != nil { + t.Fatalf("Request failed: %s", err.Error()) + } + _ = res.Body.Close() - res, err := client.Do(req) + return nil + }(context.Background()) if err != nil { - t.Fatalf("Request failed: %s", err.Error()) + panic("unexpected error in http request: " + err.Error()) } - _ = res.Body.Close() - return nil - }(context.Background()) - if err != nil { - panic("unexpected error in http request: " + err.Error()) - } + type tc struct { + name string + attributes []attribute.KeyValue + parent string + onlyTLS bool + } - testLen := []struct { - name string - attributes []attribute.KeyValue - parent string - }{ - { - name: "http.tls", - attributes: []attribute.KeyValue{ - attribute.Key("tls.server.certificate_chain").StringSlice( - []string{base64.StdEncoding.EncodeToString(ts.Certificate().Raw)}, - ), - attribute.Key("tls.server.hash.sha256"). - String(fmt.Sprintf("%X", sha256.Sum256(ts.Certificate().Raw))), - attribute.Key("tls.server.not_after"). - String(ts.Certificate().NotAfter.UTC().Format(time.RFC3339)), - attribute.Key("tls.server.not_before"). - String(ts.Certificate().NotBefore.UTC().Format(time.RFC3339)), + testLen := []tc{ + { + name: "http.connect", + attributes: []attribute.KeyValue{ + attribute.Key("http.conn.done.addr").String(ts.Listener.Addr().String()), + attribute.Key("http.conn.done.network").String("tcp"), + attribute.Key("http.conn.start.network").String("tcp"), + attribute.Key("http.remote").String(ts.Listener.Addr().String()), + }, + parent: "http.getconn", }, - parent: "http.getconn", - }, - { - name: "http.connect", - attributes: []attribute.KeyValue{ - attribute.Key("http.conn.done.addr").String(address.String()), - attribute.Key("http.conn.done.network").String("tcp"), - attribute.Key("http.conn.start.network").String("tcp"), - attribute.Key("http.remote").String(address.String()), + { + name: "http.getconn", + attributes: []attribute.KeyValue{ + attribute.Key("http.remote").String(ts.Listener.Addr().String()), + attribute.Key("net.host.name").String(ts.Listener.Addr().String()), + attribute.Key("http.conn.reused").Bool(false), + attribute.Key("http.conn.wasidle").Bool(false), + }, + parent: "test", }, - parent: "http.getconn", - }, - { - name: "http.getconn", - attributes: []attribute.KeyValue{ - attribute.Key("http.remote").String(address.String()), - attribute.Key("net.host.name").String(address.String()), - attribute.Key("http.conn.reused").Bool(false), - attribute.Key("http.conn.wasidle").Bool(false), + { + name: "http.receive", + parent: "test", + }, + { + name: "http.headers", + parent: "test", + }, + { + name: "http.send", + parent: "test", + }, + { + name: "test", }, - parent: "test", - }, - { - name: "http.receive", - parent: "test", - }, - { - name: "http.headers", - parent: "test", - }, - { - name: "http.send", - parent: "test", - }, - { - name: "test", - }, - } - for i, tl := range testLen { - span, ok := getSpanFromRecorder(sr, tl.name) - if !assert.True(t, ok) { - continue } - if tl.parent != "" { - parent, ok := getSpanFromRecorder(sr, tl.parent) - if assert.True(t, ok) { - assert.Equal(t, span.Parent().SpanID(), parent.SpanContext().SpanID()) - } + u, err := url.Parse(ts.URL) + if err != nil { + panic("unexpected error in parsing httptest server URL: " + err.Error()) } - if len(tl.attributes) > 0 { - attrs := span.Attributes() - if tl.name == "http.getconn" { - // http.local attribute uses a non-deterministic port. - local := attribute.Key("http.local") - var contains bool - for i, a := range attrs { - if a.Key == local { - attrs = append(attrs[:i], attrs[i+1:]...) - contains = true - break - } + // http.tls only exists on HTTPS connections. + if u.Scheme == "https" { + testLen = append([]tc{{ + name: "http.tls", + attributes: []attribute.KeyValue{ + attribute.Key("tls.server.certificate_chain").StringSlice( + []string{base64.StdEncoding.EncodeToString(ts.Certificate().Raw)}, + ), + attribute.Key("tls.server.hash.sha256"). + String(fmt.Sprintf("%X", sha256.Sum256(ts.Certificate().Raw))), + attribute.Key("tls.server.not_after"). + String(ts.Certificate().NotAfter.UTC().Format(time.RFC3339)), + attribute.Key("tls.server.not_before"). + String(ts.Certificate().NotBefore.UTC().Format(time.RFC3339)), + }, + parent: "http.getconn", + }}, testLen...) + } + + for i, tl := range testLen { + span, ok := getSpanFromRecorder(sr, tl.name) + if !assert.True(t, ok) { + continue + } + + if tl.parent != "" { + parent, ok := getSpanFromRecorder(sr, tl.parent) + if assert.True(t, ok) { + assert.Equal(t, span.Parent().SpanID(), parent.SpanContext().SpanID()) } - assert.True(t, contains, "missing http.local attribute") } - if tl.name == "http.tls" { - if i == 0 { - tl.attributes = append(tl.attributes, attribute.Key("tls.resumed").Bool(false)) - } else { - tl.attributes = append(tl.attributes, attribute.Key("tls.resumed").Bool(true)) + if len(tl.attributes) > 0 { + attrs := span.Attributes() + if tl.name == "http.getconn" { + // http.local attribute uses a non-deterministic port. + local := attribute.Key("http.local") + var contains bool + for i, a := range attrs { + if a.Key == local { + attrs = append(attrs[:i], attrs[i+1:]...) + contains = true + break + } + } + assert.True(t, contains, "missing http.local attribute") } - attrs = slices.DeleteFunc(attrs, func(a attribute.KeyValue) bool { - // Skip keys that are unable to be detected beforehand. - if a.Key == otelhttptrace.TLSCipher || a.Key == otelhttptrace.TLSProtocolVersion { - return true + if tl.name == "http.tls" { + if i == 0 { + tl.attributes = append(tl.attributes, attribute.Key("tls.resumed").Bool(false)) + } else { + tl.attributes = append(tl.attributes, attribute.Key("tls.resumed").Bool(true)) } - return false - }) + attrs = slices.DeleteFunc(attrs, func(a attribute.KeyValue) bool { + // Skip keys that are unable to be detected beforehand. + if a.Key == otelhttptrace.TLSCipher || a.Key == otelhttptrace.TLSProtocolVersion { + return true + } + return false + }) + } + assert.ElementsMatch(t, tl.attributes, attrs) } - assert.ElementsMatch(t, tl.attributes, attrs) } } } From f7116c066342712adb226e90634c5086911b5242 Mon Sep 17 00:00:00 2001 From: Matthew Walster Date: Tue, 21 May 2024 13:42:57 +0000 Subject: [PATCH 8/9] otelhttptrace: Amend attribute keys to have suffix "Key" --- .../httptrace/otelhttptrace/clienttrace.go | 28 +++++++++---------- .../otelhttptrace/test/clienttrace_test.go | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go index 09b7427f961..353b9b33225 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go @@ -44,13 +44,13 @@ var ( // // From https://opentelemetry.io/docs/specs/semconv/attributes-registry/tls/ but only present from semconv v1.24. var ( - TLSCipher = attribute.Key("tls.cipher") - TLSProtocolVersion = attribute.Key("tls.protocol.version") - TLSResumed = attribute.Key("tls.resumed") - TLSServerCertificateChain = attribute.Key("tls.server.certificate_chain") - TLSServerHashSha256 = attribute.Key("tls.server.hash.sha256") - TLSServerNotAfter = attribute.Key("tls.server.not_after") - TLSServerNotBefore = attribute.Key("tls.server.not_before") + TLSCipherKey = attribute.Key("tls.cipher") + TLSProtocolVersionKey = attribute.Key("tls.protocol.version") + TLSResumedKey = attribute.Key("tls.resumed") + TLSServerCertificateChainKey = attribute.Key("tls.server.certificate_chain") + TLSServerHashSha256Key = attribute.Key("tls.server.hash.sha256") + TLSServerNotAfterKey = attribute.Key("tls.server.not_after") + TLSServerNotBeforeKey = attribute.Key("tls.server.not_before") ) var hookMap = map[string]string{ @@ -336,9 +336,9 @@ func (ct *clientTracer) tlsHandshakeStart() { func (ct *clientTracer) tlsHandshakeDone(state tls.ConnectionState, err error) { attrs := make([]attribute.KeyValue, 0, 7) attrs = append(attrs, - TLSCipher.String(tls.CipherSuiteName(state.CipherSuite)), - TLSProtocolVersion.String(tls.VersionName(state.Version)), - TLSResumed.Bool(state.DidResume), + TLSCipherKey.String(tls.CipherSuiteName(state.CipherSuite)), + TLSProtocolVersionKey.String(tls.VersionName(state.Version)), + TLSResumedKey.Bool(state.DidResume), ) if len(state.PeerCertificates) > 0 { @@ -349,10 +349,10 @@ func (ct *clientTracer) tlsHandshakeDone(state tls.ConnectionState, err error) { leafCert := state.PeerCertificates[0] attrs = append(attrs, - TLSServerCertificateChain.StringSlice(certChain), - TLSServerHashSha256.String(fmt.Sprintf("%X", sha256.Sum256(leafCert.Raw))), - TLSServerNotAfter.String(leafCert.NotAfter.UTC().Format(time.RFC3339)), - TLSServerNotBefore.String(leafCert.NotBefore.UTC().Format(time.RFC3339)), + TLSServerCertificateChainKey.StringSlice(certChain), + TLSServerHashSha256Key.String(fmt.Sprintf("%X", sha256.Sum256(leafCert.Raw))), + TLSServerNotAfterKey.String(leafCert.NotAfter.UTC().Format(time.RFC3339)), + TLSServerNotBeforeKey.String(leafCert.NotBefore.UTC().Format(time.RFC3339)), ) } ct.end("http.tls", err, attrs...) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go index eef2d42d530..573973264de 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go @@ -186,7 +186,7 @@ func TestHTTPRequestWithClientTrace(t *testing.T) { } attrs = slices.DeleteFunc(attrs, func(a attribute.KeyValue) bool { // Skip keys that are unable to be detected beforehand. - if a.Key == otelhttptrace.TLSCipher || a.Key == otelhttptrace.TLSProtocolVersion { + if a.Key == otelhttptrace.TLSCipherKey || a.Key == otelhttptrace.TLSProtocolVersionKey { return true } return false From de7bef7ba41ff9e6e397ea540ee3dbf5cde0e165 Mon Sep 17 00:00:00 2001 From: Matthew Walster Date: Tue, 21 May 2024 13:56:21 +0000 Subject: [PATCH 9/9] otelhttptrace: remove superfluous struct member --- .../net/http/httptrace/otelhttptrace/test/clienttrace_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go index 573973264de..4d8b65e7b51 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -87,7 +88,6 @@ func TestHTTPRequestWithClientTrace(t *testing.T) { name string attributes []attribute.KeyValue parent string - onlyTLS bool } testLen := []tc{