Skip to content

Commit

Permalink
Update ClientRequest HTTPS determination (#3577)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
MrAlias committed Jan 13, 2023
1 parent 640a0cd commit 4286352
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
2 changes: 1 addition & 1 deletion semconv/internal/v2/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++
}
Expand Down
25 changes: 25 additions & 0 deletions semconv/internal/v2/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 4286352

Please sign in to comment.