Merge branch 'fix/log-consistency' into 'master'

fix: improve consistency of log fields

Closes #785

See merge request gitlab-org/gitlab-pages!805

7 files changed, 18 insertions, 30 deletions

diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index 59dd0749..3014936b 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -205,8 +205,8 @@ func (a *Auth) handleProxyingAuth(session *hostSession, w http.ResponseWriter, r
 
 		proxyurl, err := url.Parse(domain)
 		if err != nil {
-			logRequest(r).WithField("domain", domain).Error(queryParameterErrMsg)
-			errortracking.CaptureErrWithReqAndStackTrace(err, r, errortracking.WithField("domain", domain))
+			logRequest(r).WithField("domain_query", domain).Error(queryParameterErrMsg)
+			errortracking.CaptureErrWithReqAndStackTrace(err, r, errortracking.WithField("domain_query", domain))
 
 			httperrors.Serve500(w)
 			return true
@@ -217,12 +217,15 @@ func (a *Auth) handleProxyingAuth(session *hostSession, w http.ResponseWriter, r
 		}
 
 		if !a.domainAllowed(r.Context(), host, domains) {
-			logRequest(r).WithField("domain", host).Warn("Domain is not configured")
+			logRequest(r).WithFields(logrus.Fields{
+				"domain_query": domain,
+				"domain_host":  host,
+			}).Warn("Domain is not configured")
 			httperrors.Serve401(w)
 			return true
 		}
 
-		logRequest(r).WithField("domain", domain).Info("User is authenticating via domain")
+		logRequest(r).WithField("domain_query", domain).Info("User is authenticating via domain")
 
 		session.Values["proxy_auth_domain"] = domain
 
@@ -239,7 +242,7 @@ func (a *Auth) handleProxyingAuth(session *hostSession, w http.ResponseWriter, r
 
 		logRequest(r).WithFields(logrus.Fields{
 			"public_gitlab_server": a.publicGitlabServer,
-			"pages_domain":         domain,
+			"domain_query":         domain,
 		}).Info("Redirecting user to gitlab for oauth")
 
 		http.Redirect(w, r, url, http.StatusFound)
@@ -253,7 +256,7 @@ func (a *Auth) handleProxyingAuth(session *hostSession, w http.ResponseWriter, r
 		// Get domain started auth process
 		proxyDomain := session.Values["proxy_auth_domain"].(string)
 
-		logRequest(r).WithField("domain", proxyDomain).Info("Redirecting auth callback to custom domain")
+		logRequest(r).WithField("proxy_auth_domain", proxyDomain).Info("Redirecting auth callback to custom domain")
 
 		// Clear proxying from session
 		delete(session.Values, "proxy_auth_domain")
diff --git a/internal/httperrors/httperrors.go b/internal/httperrors/httperrors.go
index d6a9a860..b5991b6e 100644
--- a/internal/httperrors/httperrors.go
+++ b/internal/httperrors/httperrors.go
@@ -4,10 +4,8 @@ import (
 	"fmt"
 	"net/http"
 
-	"gitlab.com/gitlab-org/labkit/correlation"
-	"gitlab.com/gitlab-org/labkit/log"
-
 	"gitlab.com/gitlab-org/gitlab-pages/internal/errortracking"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/logging"
 )
 
 type content struct {
@@ -210,11 +208,7 @@ func Serve500(w http.ResponseWriter) {
 
 // Serve500WithRequest returns a 500 error response / HTML page to the http.ResponseWriter
 func Serve500WithRequest(w http.ResponseWriter, r *http.Request, reason string, err error) {
-	log.WithFields(log.Fields{
-		"correlation_id": correlation.ExtractFromContext(r.Context()),
-		"host":           r.Host,
-		"path":           r.URL.Path,
-	}).WithError(err).Error(reason)
+	logging.LogRequest(r).WithError(err).Error(reason)
 	errortracking.CaptureErrWithReqAndStackTrace(err, r)
 	serveErrorPage(w, content500)
 }
diff --git a/internal/logging/logging.go b/internal/logging/logging.go
index 6e8533d8..2faaddc5 100644
--- a/internal/logging/logging.go
+++ b/internal/logging/logging.go
@@ -67,9 +67,7 @@ func BasicAccessLogger(handler http.Handler, format string) (http.Handler, error
 
 func extraFields(r *http.Request) log.Fields {
 	return log.Fields{
-		"correlation_id": correlation.ExtractFromContext(r.Context()),
-		"pages_https":    request.IsHTTPS(r),
-		"pages_host":     r.Host,
+		"pages_https": request.IsHTTPS(r),
 	}
 }
 
diff --git a/internal/ratelimiter/middleware.go b/internal/ratelimiter/middleware.go
index 1be6f642..d710b193 100644
--- a/internal/ratelimiter/middleware.go
+++ b/internal/ratelimiter/middleware.go
@@ -5,10 +5,9 @@ import (
 	"strconv"
 
 	"github.com/sirupsen/logrus"
-	"gitlab.com/gitlab-org/labkit/correlation"
-	"gitlab.com/gitlab-org/labkit/log"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/logging"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
 )
 
@@ -46,13 +45,9 @@ func (rl *RateLimiter) Middleware(handler http.Handler) http.Handler {
 }
 
 func (rl *RateLimiter) logRateLimitedRequest(r *http.Request) {
-	log.WithFields(logrus.Fields{
+	logging.LogRequest(r).WithFields(logrus.Fields{
 		"rate_limiter_name":             rl.name,
-		"correlation_id":                correlation.ExtractFromContext(r.Context()),
-		"req_scheme":                    r.URL.Scheme,
-		"req_host":                      r.Host,
-		"req_path":                      r.URL.Path,
-		"pages_domain":                  request.GetHostWithoutPort(r),
+		"scheme":                        r.URL.Scheme,
 		"remote_addr":                   r.RemoteAddr,
 		"source_ip":                     request.GetRemoteAddrWithoutPort(r),
 		"x_forwarded_proto":             r.Header.Get(headerXForwardedProto),
diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go
index 75868cce..de8ae2a5 100644
--- a/internal/source/gitlab/client/client.go
+++ b/internal/source/gitlab/client/client.go
@@ -113,7 +113,7 @@ func (gc *Client) GetLookup(ctx context.Context, host string) api.Lookup {
 		log.WithError(domain.ErrDomainDoesNotExist).WithFields(
 			log.Fields{
 				"correlation_id": correlation.ExtractFromContext(ctx),
-				"host":           host,
+				"lookup_name":    host,
 			}).Error("unexpected nil response from gitlab")
 		return api.Lookup{Name: host, Error: domain.ErrDomainDoesNotExist}
 	}
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index e1087ddb..abe645fe 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -102,10 +102,8 @@ func (g *Gitlab) Resolve(r *http.Request) (*serving.Request, error) {
 
 	logging.LogRequest(r).WithError(domain.ErrDomainDoesNotExist).WithFields(
 		log.Fields{
-			"host":               host,
 			"lookup_paths_count": size,
 			"lookup_paths":       response.Domain.LookupPaths,
-			"url_path":           urlPath,
 		}).Error("could not find project lookup path")
 
 	return nil, domain.ErrDomainDoesNotExist
diff --git a/internal/vfs/zip/archive.go b/internal/vfs/zip/archive.go
index 49ae2719..9f01794b 100644
--- a/internal/vfs/zip/archive.go
+++ b/internal/vfs/zip/archive.go
@@ -122,7 +122,7 @@ func (a *zipArchive) readArchive(url string) {
 	a.resource, a.err = httprange.NewResource(ctx, url, a.fs.httpClient)
 	if a.err != nil {
 		log.WithFields(log.Fields{
-			"url": url,
+			"archive_url": url,
 		}).WithError(a.err).Infoln("read zip archive request failed")
 		metrics.ZipOpened.WithLabelValues("error").Inc()
 		return
@@ -136,7 +136,7 @@ func (a *zipArchive) readArchive(url string) {
 
 	if a.archive == nil || a.err != nil {
 		log.WithFields(log.Fields{
-			"url": url,
+			"archive_url": url,
 		}).WithError(a.err).Infoln("loading zip archive files into memory failed")
 		metrics.ZipOpened.WithLabelValues("error").Inc()
 		return