From 42863522e50aa5c3a88043d1b068ccec083264a8 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 12 Jan 2023 16:01:51 -0800 Subject: [PATCH] Update ClientRequest HTTPS determination (#3577) * Update ClientRequest HTTPS determination The ClientRequest function will only report a peer port attribute if that peer port differs from the standard 80 for HTTP and 443 for HTTPS. In determining if the request is for HTTPS use the request URL scheme. This is not perfect. If a user doesn't provide a scheme this will not be correctly detected. However, the current approach of checking if the `TLS` field is non-nil will always be wrong, requests made by client ignore this field and it is always nil. Therefore, switching to using the URL field is the best we can do without having already made the request. * Test HTTPS detection for ClientRequest --- semconv/internal/v2/http.go | 2 +- semconv/internal/v2/http_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/semconv/internal/v2/http.go b/semconv/internal/v2/http.go index 6989391cfbd..0f84a905b04 100644 --- a/semconv/internal/v2/http.go +++ b/semconv/internal/v2/http.go @@ -84,7 +84,7 @@ func (c *HTTPConv) ClientRequest(req *http.Request) []attribute.KeyValue { h = req.URL.Host } peer, p := firstHostPort(h, req.Header.Get("Host")) - port := requiredHTTPPort(req.TLS != nil, p) + port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", p) if port > 0 { n++ } diff --git a/semconv/internal/v2/http_test.go b/semconv/internal/v2/http_test.go index edfc7b50f27..73b9aa9b147 100644 --- a/semconv/internal/v2/http_test.go +++ b/semconv/internal/v2/http_test.go @@ -59,6 +59,31 @@ func TestHTTPClientResponse(t *testing.T) { }, got) } +func TestHTTPSClientRequest(t *testing.T) { + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "https", + Host: "127.0.0.1:443", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + } + + assert.Equal( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.flavor", "1.0"), + attribute.String("http.url", "https://127.0.0.1:443/resource"), + attribute.String("net.peer.name", "127.0.0.1"), + }, + hc.ClientRequest(req), + ) +} + func TestHTTPClientRequest(t *testing.T) { const ( user = "alice"