From 21ca388dd09d8306268f717c0d2c28196b085d6a Mon Sep 17 00:00:00 2001 From: Kassio Borges Date: Fri, 11 Mar 2022 05:08:48 +0000 Subject: Add correlation_id to all exception captures --- internal/artifact/artifact.go | 10 +++---- internal/auth/auth.go | 53 ++++++++++++--------------------- internal/domain/domain.go | 9 +++--- internal/errortracking/errortracking.go | 27 +++++++++++++++++ internal/httperrors/httperrors.go | 5 ++-- internal/serving/disk/reader.go | 5 ++-- internal/vfs/serving/serving.go | 6 ++-- 7 files changed, 63 insertions(+), 52 deletions(-) create mode 100644 internal/errortracking/errortracking.go (limited to 'internal') diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 053ea940..0a4f0ab4 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -12,8 +12,7 @@ import ( "strings" "time" - "gitlab.com/gitlab-org/labkit/errortracking" - + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/httptransport" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" @@ -80,7 +79,7 @@ func (a *Artifact) makeRequest(w http.ResponseWriter, r *http.Request, reqURL *u req, err := http.NewRequestWithContext(r.Context(), "GET", reqURL.String(), nil) if err != nil { logging.LogRequest(r).WithError(err).Error(createArtifactRequestErrMsg) - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return } @@ -89,10 +88,9 @@ func (a *Artifact) makeRequest(w http.ResponseWriter, r *http.Request, reqURL *u req.Header.Add("Authorization", "Bearer "+token) } resp, err := a.client.Do(req) - if err != nil { logging.LogRequest(r).WithError(err).Error(artifactRequestErrMsg) - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve502(w) return } @@ -110,7 +108,7 @@ func (a *Artifact) makeRequest(w http.ResponseWriter, r *http.Request, reqURL *u if resp.StatusCode == http.StatusInternalServerError { logging.LogRequest(r).Error(errArtifactResponse) - errortracking.Capture(errArtifactResponse, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(errArtifactResponse, r) httperrors.Serve500(w) return } diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 98bbf49d..45b77b4f 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -17,10 +17,10 @@ import ( "github.com/gorilla/securecookie" "github.com/gorilla/sessions" "github.com/sirupsen/logrus" - "gitlab.com/gitlab-org/labkit/errortracking" "gitlab.com/gitlab-org/labkit/log" "golang.org/x/crypto/hkdf" + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/httptransport" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" @@ -108,7 +108,7 @@ func (a *Auth) checkSession(w http.ResponseWriter, r *http.Request) (*sessions.S errsave := session.Save(r, w) if errsave != nil { logRequest(r).WithError(errsave).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(errsave, r) + errortracking.CaptureErrWithReqAndStackTrace(errsave, r) httperrors.Serve500(w) return nil, errsave } @@ -178,7 +178,7 @@ func (a *Auth) checkAuthenticationResponse(session *sessions.Session, w http.Res decryptedCode, err := a.DecryptCode(r.URL.Query().Get("code"), getRequestDomain(r)) if err != nil { logRequest(r).WithError(err).Error("failed to decrypt secure code") - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return } @@ -190,12 +190,7 @@ func (a *Auth) checkAuthenticationResponse(session *sessions.Session, w http.Res logRequest(r).WithError(err).WithField( "redirect_uri", redirectURI, ).Error(fetchAccessTokenErrMsg) - errortracking.Capture( - err, - errortracking.WithRequest(r), - errortracking.WithField("redirect_uri", redirectURI), - errortracking.WithStackTrace(), - ) + errortracking.CaptureErrWithReqAndStackTrace(err, r, errortracking.WithField("redirect_uri", redirectURI)) httperrors.Serve503(w) return @@ -206,7 +201,7 @@ func (a *Auth) checkAuthenticationResponse(session *sessions.Session, w http.Res err = session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return @@ -242,11 +237,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit proxyurl, err := url.Parse(domain) if err != nil { logRequest(r).WithField("domain", domain).Error(queryParameterErrMsg) - errortracking.Capture(err, - errortracking.WithRequest(r), - errortracking.WithField("domain", domain), - errortracking.WithStackTrace(), - ) + errortracking.CaptureErrWithReqAndStackTrace(err, r, errortracking.WithField("domain", domain)) httperrors.Serve500(w) return true @@ -269,7 +260,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit err = session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return true @@ -300,7 +291,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit err := session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return true @@ -314,7 +305,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit signedCode, err := a.EncryptAndSignCode(proxyDomain, query.Get("code")) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve503(w) return true @@ -402,7 +393,6 @@ func (a *Auth) fetchAccessToken(ctx context.Context, code string) (tokenResponse // Request token resp, err := a.apiClient.Do(req) - if err != nil { return token, err } @@ -411,7 +401,7 @@ func (a *Auth) fetchAccessToken(ctx context.Context, code string) (tokenResponse if resp.StatusCode != 200 { err = errResponseNotOk - captureErrWithReqAndStackTrace(err, req) + errortracking.CaptureErrWithReqAndStackTrace(err, req) return token, err } @@ -454,7 +444,7 @@ func (a *Auth) checkTokenExists(session *sessions.Session, w http.ResponseWriter err := session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return true @@ -481,7 +471,7 @@ func destroySession(session *sessions.Session, w http.ResponseWriter, r *http.Re err := session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return @@ -510,10 +500,9 @@ func (a *Auth) checkAuthentication(w http.ResponseWriter, r *http.Request, domai url = fmt.Sprintf(apiURLUserTemplate, a.internalGitlabServer) } req, err := http.NewRequestWithContext(r.Context(), "GET", url, nil) - if err != nil { logRequest(r).WithError(err).Error(failAuthErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return true @@ -521,10 +510,9 @@ func (a *Auth) checkAuthentication(w http.ResponseWriter, r *http.Request, domai req.Header.Add("Authorization", "Bearer "+session.Values["access_token"].(string)) resp, err := a.apiClient.Do(req) - if err != nil { logRequest(r).WithError(err).Error("Failed to retrieve info with token") - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) // call serve404 handler when auth fails domain.ServeNotFoundAuthFailed(w, r) return true @@ -543,7 +531,7 @@ func (a *Auth) checkAuthentication(w http.ResponseWriter, r *http.Request, domai "status": resp.StatusCode, "status_text": resp.Status, }).Error("Unexpected response fetching access token") - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) domain.ServeNotFoundAuthFailed(w, r) return true } @@ -591,7 +579,7 @@ func (a *Auth) CheckAuthentication(w http.ResponseWriter, r *http.Request, domai if a == nil { logRequest(r).Error(errAuthNotConfigured) - captureErrWithReqAndStackTrace(errAuthNotConfigured, r) + errortracking.CaptureErrWithReqAndStackTrace(errAuthNotConfigured, r) httperrors.Serve500(w) return true @@ -602,7 +590,8 @@ func (a *Auth) CheckAuthentication(w http.ResponseWriter, r *http.Request, domai // CheckResponseForInvalidToken checks response for invalid token and destroys session if it was invalid func (a *Auth) CheckResponseForInvalidToken(w http.ResponseWriter, r *http.Request, - resp *http.Response) bool { + resp *http.Response, +) bool { if a == nil { // No auth supported return false @@ -628,7 +617,7 @@ func checkResponseForInvalidToken(resp *http.Response, session *sessions.Session defer resp.Body.Close() err := json.NewDecoder(resp.Body).Decode(&errResp) if err != nil { - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) return false } @@ -697,7 +686,3 @@ func New(pagesDomain, storeSecret, clientID, clientSecret, redirectURI, internal now: time.Now, }, nil } - -func captureErrWithReqAndStackTrace(err error, r *http.Request) { - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) -} diff --git a/internal/domain/domain.go b/internal/domain/domain.go index bf202cd3..76a1cf25 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -7,8 +7,7 @@ import ( "net/http" "sync" - "gitlab.com/gitlab-org/labkit/errortracking" - + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/serving" ) @@ -131,7 +130,7 @@ func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool { return true } - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve503(w) return true } @@ -149,7 +148,7 @@ func (d *Domain) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) { return } - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve503(w) return } @@ -173,7 +172,7 @@ func (d *Domain) ServeNamespaceNotFound(w http.ResponseWriter, r *http.Request) return } - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve503(w) return } diff --git a/internal/errortracking/errortracking.go b/internal/errortracking/errortracking.go new file mode 100644 index 00000000..d82674cc --- /dev/null +++ b/internal/errortracking/errortracking.go @@ -0,0 +1,27 @@ +package errortracking + +import ( + "net/http" + + "gitlab.com/gitlab-org/labkit/errortracking" +) + +// CaptureOption alias to avoid importing labkit/errortracking in internal packages +type CaptureOption = errortracking.CaptureOption + +// WithField alias to avoid importing labkit/errortracking in internal packages +func WithField(key, value string) CaptureOption { + return errortracking.WithField(key, value) +} + +// CaptureErrWithReqAndStackTrace calls labkit's errortracking function and attaches the request, stack trace and any additional fields +func CaptureErrWithReqAndStackTrace(err error, r *http.Request, fields ...errortracking.CaptureOption) { + opts := append( + fields, + errortracking.WithContext(r.Context()), + errortracking.WithRequest(r), + errortracking.WithStackTrace(), + ) + + errortracking.Capture(err, opts...) +} diff --git a/internal/httperrors/httperrors.go b/internal/httperrors/httperrors.go index 7de3b6bc..d6a9a860 100644 --- a/internal/httperrors/httperrors.go +++ b/internal/httperrors/httperrors.go @@ -5,8 +5,9 @@ import ( "net/http" "gitlab.com/gitlab-org/labkit/correlation" - "gitlab.com/gitlab-org/labkit/errortracking" "gitlab.com/gitlab-org/labkit/log" + + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" ) type content struct { @@ -214,7 +215,7 @@ func Serve500WithRequest(w http.ResponseWriter, r *http.Request, reason string, "host": r.Host, "path": r.URL.Path, }).WithError(err).Error(reason) - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) serveErrorPage(w, content500) } diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index 4078d5ef..f250a0d6 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -12,8 +12,8 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" - "gitlab.com/gitlab-org/labkit/errortracking" + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" "gitlab.com/gitlab-org/gitlab-pages/internal/redirects" @@ -53,7 +53,8 @@ func (reader *Reader) tryRedirects(h serving.Handler) bool { if !errors.Is(err, redirects.ErrNoRedirect) { // We assume that rewrite failure is not fatal // and we only capture the error - errortracking.Capture(err, errortracking.WithRequest(h.Request), errortracking.WithStackTrace()) + + errortracking.CaptureErrWithReqAndStackTrace(err, h.Request) } return false } diff --git a/internal/vfs/serving/serving.go b/internal/vfs/serving/serving.go index 6f45e1d5..4d8b7d90 100644 --- a/internal/vfs/serving/serving.go +++ b/internal/vfs/serving/serving.go @@ -12,8 +12,7 @@ import ( "strings" "time" - "gitlab.com/gitlab-org/labkit/errortracking" - + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" @@ -40,7 +39,8 @@ func serveContent(w http.ResponseWriter, r *http.Request, modtime time.Time, con _, haveType := w.Header()["Content-Type"] if !haveType { // this shouldn't happen - errortracking.Capture(errUnknownContentType, errortracking.WithRequest(r), errortracking.WithStackTrace()) + + errortracking.CaptureErrWithReqAndStackTrace(errUnknownContentType, r) logging.LogRequest(r).WithError(errUnknownContentType).Error("could not serve content") httperrors.Serve500(w) -- cgit v1.2.3