Add latest changes from gitlab-org/gitlab@master

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