diff options
Diffstat (limited to 'workhorse/internal')
-rw-r--r-- | workhorse/internal/errortracker/sentry.go | 60 | ||||
-rw-r--r-- | workhorse/internal/helper/helpers.go | 43 | ||||
-rw-r--r-- | workhorse/internal/helper/raven.go | 58 | ||||
-rw-r--r-- | workhorse/internal/imageresizer/image_resizer.go | 10 | ||||
-rw-r--r-- | workhorse/internal/log/logging.go | 6 | ||||
-rw-r--r-- | workhorse/internal/upstream/upstream.go | 3 |
6 files changed, 96 insertions, 84 deletions
diff --git a/workhorse/internal/errortracker/sentry.go b/workhorse/internal/errortracker/sentry.go deleted file mode 100644 index 72a32c8d349..00000000000 --- a/workhorse/internal/errortracker/sentry.go +++ /dev/null @@ -1,60 +0,0 @@ -package errortracker - -import ( - "fmt" - "net/http" - "os" - "runtime/debug" - - "gitlab.com/gitlab-org/labkit/errortracking" - - "gitlab.com/gitlab-org/labkit/log" -) - -// NewHandler allows us to handle panics in upstreams gracefully, by logging them -// using structured logging and reporting them into Sentry as `error`s with a -// proper correlation ID attached. -func NewHandler(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - defer func() { - if p := recover(); p != nil { - fields := log.ContextFields(r.Context()) - log.WithFields(fields).Error(p) - debug.PrintStack() - // A panic isn't always an `error`, so we may have to convert it into one. - e, ok := p.(error) - if !ok { - e = fmt.Errorf("%v", p) - } - TrackFailedRequest(r, e, fields) - } - }() - - next.ServeHTTP(w, r) - }) -} - -func TrackFailedRequest(r *http.Request, err error, fields log.Fields) { - captureOpts := []errortracking.CaptureOption{ - errortracking.WithContext(r.Context()), - errortracking.WithRequest(r), - } - for k, v := range fields { - captureOpts = append(captureOpts, errortracking.WithField(k, fmt.Sprintf("%v", v))) - } - - errortracking.Capture(err, captureOpts...) -} - -func Initialize(version string) error { - // Use a custom environment variable (not SENTRY_DSN) to prevent - // clashes with gitlab-rails. - sentryDSN := os.Getenv("GITLAB_WORKHORSE_SENTRY_DSN") - sentryEnvironment := os.Getenv("GITLAB_WORKHORSE_SENTRY_ENVIRONMENT") - - return errortracking.Initialize( - errortracking.WithSentryDSN(sentryDSN), - errortracking.WithSentryEnvironment(sentryEnvironment), - errortracking.WithVersion(version), - ) -} diff --git a/workhorse/internal/helper/helpers.go b/workhorse/internal/helper/helpers.go index 2e23f50b913..f9b46181579 100644 --- a/workhorse/internal/helper/helpers.go +++ b/workhorse/internal/helper/helpers.go @@ -14,31 +14,50 @@ import ( "syscall" "github.com/sebest/xff" - - "gitlab.com/gitlab-org/gitlab-workhorse/internal/log" + "gitlab.com/gitlab-org/labkit/log" + "gitlab.com/gitlab-org/labkit/mask" ) const NginxResponseBufferHeader = "X-Accel-Buffering" -func CaptureAndFail(w http.ResponseWriter, r *http.Request, err error, msg string, code int, loggerCallbacks ...log.ConfigureLogger) { - http.Error(w, msg, code) - logger := log.WithRequest(r).WithError(err) - - for _, cb := range loggerCallbacks { - logger = cb(logger) +func logErrorWithFields(r *http.Request, err error, fields log.Fields) { + if err != nil { + CaptureRavenError(r, err, fields) } - logger.Error(msg) + printError(r, err, fields) +} + +func CaptureAndFail(w http.ResponseWriter, r *http.Request, err error, msg string, code int) { + http.Error(w, msg, code) + logErrorWithFields(r, err, nil) +} + +func Fail500(w http.ResponseWriter, r *http.Request, err error) { + CaptureAndFail(w, r, err, "Internal server error", http.StatusInternalServerError) } -func Fail500(w http.ResponseWriter, r *http.Request, err error, loggerCallbacks ...log.ConfigureLogger) { - CaptureAndFail(w, r, err, "Internal server error", http.StatusInternalServerError, loggerCallbacks...) +func Fail500WithFields(w http.ResponseWriter, r *http.Request, err error, fields log.Fields) { + http.Error(w, "Internal server error", http.StatusInternalServerError) + logErrorWithFields(r, err, fields) } func RequestEntityTooLarge(w http.ResponseWriter, r *http.Request, err error) { CaptureAndFail(w, r, err, "Request Entity Too Large", http.StatusRequestEntityTooLarge) } +func printError(r *http.Request, err error, fields log.Fields) { + if r != nil { + entry := log.WithContextFields(r.Context(), log.Fields{ + "method": r.Method, + "uri": mask.URL(r.RequestURI), + }) + entry.WithFields(fields).WithError(err).Error() + } else { + log.WithFields(fields).WithError(err).Error("unknown error") + } +} + func SetNoCacheHeaders(header http.Header) { header.Set("Cache-Control", "no-cache, no-store, max-age=0, must-revalidate") header.Set("Pragma", "no-cache") @@ -78,7 +97,7 @@ func OpenFile(path string) (file *os.File, fi os.FileInfo, err error) { func URLMustParse(s string) *url.URL { u, err := url.Parse(s) if err != nil { - log.WithError(err).WithFields(log.Fields{"url": s}).Error("urlMustParse") + log.WithError(err).WithField("url", s).Fatal("urlMustParse") } return u } diff --git a/workhorse/internal/helper/raven.go b/workhorse/internal/helper/raven.go new file mode 100644 index 00000000000..898e8ec85f8 --- /dev/null +++ b/workhorse/internal/helper/raven.go @@ -0,0 +1,58 @@ +package helper + +import ( + "net/http" + "reflect" + + raven "github.com/getsentry/raven-go" + + //lint:ignore SA1019 this was recently deprecated. Update workhorse to use labkit errortracking package. + correlation "gitlab.com/gitlab-org/labkit/correlation/raven" + + "gitlab.com/gitlab-org/labkit/log" +) + +var ravenHeaderBlacklist = []string{ + "Authorization", + "Private-Token", +} + +func CaptureRavenError(r *http.Request, err error, fields log.Fields) { + client := raven.DefaultClient + extra := raven.Extra{} + + for k, v := range fields { + extra[k] = v + } + + interfaces := []raven.Interface{} + if r != nil { + CleanHeadersForRaven(r) + interfaces = append(interfaces, raven.NewHttp(r)) + + //lint:ignore SA1019 this was recently deprecated. Update workhorse to use labkit errortracking package. + extra = correlation.SetExtra(r.Context(), extra) + } + + exception := &raven.Exception{ + Stacktrace: raven.NewStacktrace(2, 3, nil), + Value: err.Error(), + Type: reflect.TypeOf(err).String(), + } + interfaces = append(interfaces, exception) + + packet := raven.NewPacketWithExtra(err.Error(), extra, interfaces...) + client.Capture(packet, nil) +} + +func CleanHeadersForRaven(r *http.Request) { + if r == nil { + return + } + + for _, key := range ravenHeaderBlacklist { + if r.Header.Get(key) != "" { + r.Header.Set(key, "[redacted]") + } + } +} diff --git a/workhorse/internal/imageresizer/image_resizer.go b/workhorse/internal/imageresizer/image_resizer.go index 7d423b80067..69e9496aec2 100644 --- a/workhorse/internal/imageresizer/image_resizer.go +++ b/workhorse/internal/imageresizer/image_resizer.go @@ -428,18 +428,16 @@ func logFields(startTime time.Time, params *resizeParams, outcome *resizeOutcome func handleOutcome(w http.ResponseWriter, req *http.Request, startTime time.Time, params *resizeParams, outcome *resizeOutcome) { fields := logFields(startTime, params, outcome) - logger := log.WithRequest(req).WithFields(fields) + log := log.WithRequest(req).WithFields(fields) switch outcome.status { case statusRequestFailure: if outcome.bytesWritten <= 0 { - helper.Fail500(w, req, outcome.err, func(b *log.Builder) *log.Builder { - return b.WithFields(fields) - }) + helper.Fail500WithFields(w, req, outcome.err, fields) } else { - logger.WithError(outcome.err).Error(outcome.status) + log.WithError(outcome.err).Error(outcome.status) } default: - logger.Info(outcome.status) + log.Info(outcome.status) } } diff --git a/workhorse/internal/log/logging.go b/workhorse/internal/log/logging.go index 9c19cde1395..c65ec07743a 100644 --- a/workhorse/internal/log/logging.go +++ b/workhorse/internal/log/logging.go @@ -8,13 +8,11 @@ import ( "gitlab.com/gitlab-org/labkit/mask" "golang.org/x/net/context" - "gitlab.com/gitlab-org/gitlab-workhorse/internal/errortracker" + "gitlab.com/gitlab-org/gitlab-workhorse/internal/helper" ) type Fields = log.Fields -type ConfigureLogger func(*Builder) *Builder - type Builder struct { entry *logrus.Entry fields log.Fields @@ -85,6 +83,6 @@ func (b *Builder) Error(args ...interface{}) { b.entry.Error(args...) if b.req != nil && b.err != nil { - errortracker.TrackFailedRequest(b.req, b.err, b.fields) + helper.CaptureRavenError(b.req, b.err, b.fields) } } diff --git a/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go index fd655a07679..c81a21c0ecd 100644 --- a/workhorse/internal/upstream/upstream.go +++ b/workhorse/internal/upstream/upstream.go @@ -16,7 +16,6 @@ import ( "gitlab.com/gitlab-org/labkit/correlation" "gitlab.com/gitlab-org/gitlab-workhorse/internal/config" - "gitlab.com/gitlab-org/gitlab-workhorse/internal/errortracker" "gitlab.com/gitlab-org/gitlab-workhorse/internal/helper" "gitlab.com/gitlab-org/gitlab-workhorse/internal/rejectmethods" "gitlab.com/gitlab-org/gitlab-workhorse/internal/upload" @@ -64,7 +63,7 @@ func NewUpstream(cfg config.Config, accessLogger *logrus.Logger) http.Handler { correlationOpts = append(correlationOpts, correlation.WithPropagation()) } - handler := correlation.InjectCorrelationID(errortracker.NewHandler(&up), correlationOpts...) + handler := correlation.InjectCorrelationID(&up, correlationOpts...) // TODO: move to LabKit https://gitlab.com/gitlab-org/gitlab-workhorse/-/issues/339 handler = rejectmethods.NewMiddleware(handler) return handler |