logging: fix the HTTP logger

Fix the HTTP logger so that it would take a look at the `url.URL` as
well if `r.Host` is empty. Without this, the logging functionality doesn't quite work
as Cortex uses `url.URL`:

https://github.com/cortexproject/cortex/blob/faf92e8f3dbedfa769fbd6a3ff6e0fbd127694f5/pkg/frontend/downstream_roundtripper.go#L16-L23

Add a test to ensure that this works as expected in both ways.

Signed-off-by: Giedrius Statkevičius <[email protected]>

pkg/logging/http.go

@@ -39,7 +39,11 @@ func (m *HTTPServerMiddleware) HTTPMiddleware(name string, next http.Handler) ht
 	return func(w http.ResponseWriter, r *http.Request) {
 		wrapped := httputil.WrapResponseWriterWithStatus(w)
 		start := time.Now()
-		_, port, err := net.SplitHostPort(r.Host)
+		hostPort := r.Host
+		if hostPort == "" {
+			hostPort = r.URL.Host
+		}
+		_, port, err := net.SplitHostPort(hostPort)
 		if err != nil {
 			level.Error(m.logger).Log("msg", "failed to parse host port for http log decision", "err", err)
 			next.ServeHTTP(w, r)

pkg/logging/http_test.go

@@ -4,34 +4,64 @@
 package logging
 
 import (
+	"bytes"
 	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/go-kit/kit/log"
 	"github.com/thanos-io/thanos/pkg/testutil"
 )
 
 func TestHTTPServerMiddleware(t *testing.T) {
-	m := NewHTTPServerMiddleware(log.NewNopLogger())
+	b := bytes.Buffer{}
+
+	m := NewHTTPServerMiddleware(log.NewLogfmtLogger(io.Writer(&b)))
 	handler := func(w http.ResponseWriter, r *http.Request) {
 		_, err := io.WriteString(w, "Test Works")
 		if err != nil {
-			t.Log(err)
+			testutil.Ok(t, err)
 		}
 	}
 	hm := m.HTTPMiddleware("test", http.HandlerFunc(handler))
 
-	req := httptest.NewRequest("GET", "http://example.com/foo", nil)
+	// Cortex way:
+	u, err := url.Parse("http://example.com:5555/foo")
+	testutil.Ok(t, err)
+	req := &http.Request{
+		Method: "GET",
+		URL:    u,
+		Body:   nil,
+	}
+
 	w := httptest.NewRecorder()
 
 	hm(w, req)
 
 	resp := w.Result()
-	body, _ := ioutil.ReadAll(resp.Body)
+	body, err := ioutil.ReadAll(resp.Body)
+	testutil.Ok(t, err)
+
+	testutil.Equals(t, 200, resp.StatusCode)
+	testutil.Equals(t, "Test Works", string(body))
+	testutil.Assert(t, !strings.Contains(b.String(), "err="))
+
+	// Typical way:
+	req = httptest.NewRequest("GET", "http://example.com:5555/foo", nil)
+	b.Reset()
+
+	w = httptest.NewRecorder()
+	hm(w, req)
+
+	resp = w.Result()
+	body, err = ioutil.ReadAll(resp.Body)
+	testutil.Ok(t, err)
 
 	testutil.Equals(t, 200, resp.StatusCode)
 	testutil.Equals(t, "Test Works", string(body))
+	testutil.Assert(t, !strings.Contains(b.String(), "err="))
 }