logging: fix the HTTP logger

CHANGELOG.md

@@ -30,6 +30,7 @@ We use _breaking :warning:_ to mark changes that are not backward compatible (re
 - [#3960](https://github.com/thanos-io/thanos/pull/3960) fix deduplication of equal alerts with different labels
 - [#3937](https://github.com/thanos-io/thanos/pull/3937) Store: Fix race condition in chunk pool.
 - [#4017](https://github.com/thanos-io/thanos/pull/4017) Query Frontend: fix downsampling iterator returning duplicate samples.
+- [#4017](https://github.com/thanos-io/thanos/pull/4041) Logging: fix the HTTP logger
 
 ### Changed
 - [#3929](https://github.com/thanos-io/thanos/pull/3929) Store: Adds the name of the instantiated memcached client to log info

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="))
 }