From ae9a8fb5304fca0a1dc0441cb991227320033bca Mon Sep 17 00:00:00 2001 From: Ercan Ucan Date: Tue, 23 Feb 2021 16:35:37 +0100 Subject: [PATCH] fix: use correlationID middleware As discussed within https://gitlab.com/gitlab-org/gitlab-pages/-/issues/510 this MR adds the usage of labkit's correlationID middleware. It uses a similar approach to the implemantion in gitlab-workhorse. Fixes https://gitlab.com/gitlab-org/gitlab-pages/-/issues/510 :tools: with :heart: at Siemens Changelog: fixed --- app.go | 8 ++++++++ app_config.go | 1 + internal/auth/auth.go | 8 +++++--- internal/httptransport/metered_round_tripper.go | 2 +- internal/logging/logging.go | 12 ++++++++---- internal/source/gitlab/client/client.go | 4 +++- main.go | 3 +++ 7 files changed, 29 insertions(+), 9 deletions(-) diff --git a/app.go b/app.go index 0a6f0cd4..40bfd153 100644 --- a/app.go +++ b/app.go @@ -15,6 +15,7 @@ import ( log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/go-mimedb" + "gitlab.com/gitlab-org/labkit/correlation" "gitlab.com/gitlab-org/labkit/errortracking" labmetrics "gitlab.com/gitlab-org/labkit/metrics" "gitlab.com/gitlab-org/labkit/monitoring" @@ -339,6 +340,13 @@ func (a *theApp) buildHandlerPipeline() (http.Handler, error) { // Custom response headers handler = a.customHeadersMiddleware(handler) + // Correlation ID injection middleware + var correlationOpts []correlation.InboundHandlerOption + if a.appConfig.PropagateCorrelationID { + correlationOpts = append(correlationOpts, correlation.WithPropagation()) + } + handler = correlation.InjectCorrelationID(handler, correlationOpts...) + // This MUST be the last handler! // This handler blocks unknown HTTP methods, // being the last means it will be evaluated first diff --git a/app_config.go b/app_config.go index 5b6c547a..f3048915 100644 --- a/app_config.go +++ b/app_config.go @@ -25,6 +25,7 @@ type appConfig struct { StatusPath string DisableCrossOriginRequests bool + PropagateCorrelationID bool LogFormat string LogVerbose bool diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 2fdcbeb3..da35ba51 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -18,6 +18,7 @@ import ( log "github.com/sirupsen/logrus" "golang.org/x/crypto/hkdf" + "gitlab.com/gitlab-org/labkit/correlation" "gitlab.com/gitlab-org/labkit/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" @@ -617,9 +618,10 @@ func checkResponseForInvalidToken(resp *http.Response, session *sessions.Session func logRequest(r *http.Request) *log.Entry { state := r.URL.Query().Get("state") return log.WithFields(log.Fields{ - "host": r.Host, - "path": r.URL.Path, - "state": state, + "correlation_id": correlation.ExtractFromContext(r.Context()), + "host": r.Host, + "path": r.URL.Path, + "state": state, }) } diff --git a/internal/httptransport/metered_round_tripper.go b/internal/httptransport/metered_round_tripper.go index ff79faca..471449ec 100644 --- a/internal/httptransport/metered_round_tripper.go +++ b/internal/httptransport/metered_round_tripper.go @@ -23,7 +23,7 @@ type meteredRoundTripper struct { // NewMeteredRoundTripper will create a custom http.RoundTripper that can be used with an http.Client. // The RoundTripper will report metrics based on the collectors passed. -func NewMeteredRoundTripper(transport *http.Transport, name string, tracerVec, durationsVec *prometheus. +func NewMeteredRoundTripper(transport http.RoundTripper, name string, tracerVec, durationsVec *prometheus. HistogramVec, counterVec *prometheus.CounterVec, ttfbTimeout time.Duration) http.RoundTripper { if transport == nil { transport = DefaultTransport diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 43fe65e6..769856a2 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -4,6 +4,8 @@ import ( "net/http" "github.com/sirupsen/logrus" + + "gitlab.com/gitlab-org/labkit/correlation" "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/gitlab-pages/internal/request" @@ -83,8 +85,9 @@ func BasicAccessLogger(handler http.Handler, format string, extraFields log.Extr if extraFields == nil { extraFields = func(r *http.Request) log.Fields { return log.Fields{ - "pages_https": request.IsHTTPS(r), - "pages_host": r.Host, + "correlation_id": correlation.ExtractFromContext(r.Context()), + "pages_https": request.IsHTTPS(r), + "pages_host": r.Host, } } } @@ -104,7 +107,8 @@ func AccessLogger(handler http.Handler, format string) (http.Handler, error) { // LogRequest will inject request host and path to the logged messages func LogRequest(r *http.Request) *logrus.Entry { return log.WithFields(log.Fields{ - "host": r.Host, - "path": r.URL.Path, + "correlation_id": correlation.ExtractFromContext(r.Context()), + "host": r.Host, + "path": r.URL.Path, }) } diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go index c5a37e17..99d53fb7 100644 --- a/internal/source/gitlab/client/client.go +++ b/internal/source/gitlab/client/client.go @@ -13,6 +13,8 @@ import ( "github.com/dgrijalva/jwt-go" + "gitlab.com/gitlab-org/labkit/correlation" + "gitlab.com/gitlab-org/gitlab-pages/internal/domain" "gitlab.com/gitlab-org/gitlab-pages/internal/httptransport" "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api" @@ -64,7 +66,7 @@ func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpi httpClient: &http.Client{ Timeout: connectionTimeout, Transport: httptransport.NewMeteredRoundTripper( - httptransport.DefaultTransport, + correlation.NewInstrumentedRoundTripper(httptransport.DefaultTransport), "gitlab_internal_api", metrics.DomainsSourceAPITraceDuration, metrics.DomainsSourceAPICallDuration, diff --git a/main.go b/main.go index 582d963e..6f758244 100644 --- a/main.go +++ b/main.go @@ -54,6 +54,7 @@ var ( daemonUID = flag.Uint("daemon-uid", 0, "Drop privileges to this user") daemonGID = flag.Uint("daemon-gid", 0, "Drop privileges to this group") daemonInplaceChroot = flag.Bool("daemon-inplace-chroot", false, "Fall back to a non-bind-mount chroot of -pages-root when daemonizing") + propagateCorrelationID = flag.Bool("propagate-correlation-id", false, "Reuse existing Correlation-ID from the incoming request header `X-Request-ID` if present") logFormat = flag.String("log-format", "text", "The log output format: 'text' or 'json'") logVerbose = flag.Bool("log-verbose", false, "Verbose logging") _ = flag.String("admin-secret-path", "", "DEPRECATED") @@ -167,6 +168,7 @@ func configFromFlags() appConfig { config.RedirectHTTP = *redirectHTTP config.HTTP2 = *useHTTP2 config.DisableCrossOriginRequests = *disableCrossOriginRequests + config.PropagateCorrelationID = *propagateCorrelationID config.StatusPath = *pagesStatus config.LogFormat = *logFormat config.LogVerbose = *logVerbose @@ -290,6 +292,7 @@ func loadConfig() appConfig { "pages-domain": *pagesDomain, "pages-root": *pagesRoot, "pages-status": *pagesStatus, + "propagate-correlation-id": *propagateCorrelationID, "redirect-http": config.RedirectHTTP, "root-cert": *pagesRootKey, "root-key": *pagesRootCert, -- GitLab