Merge pull request #364 from bookingcom/emadolsky/refactor-log-levels

Refactor log levels

Author: emadolsky

app/carbonapi/app.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"expvar"
 	"fmt"
+	"go.uber.org/zap/zapcore"
 	"net"
 	"net/http"
 	"os"
@@ -383,24 +384,27 @@ func setUpConfig(app *App, logger *zap.Logger) {
 
 }
 
-func (app *App) deferredAccessLogging(accessLogger *zap.Logger, r *http.Request, accessLogDetails *carbonapipb.AccessLogDetails, t time.Time, logAsError bool) {
+func (app *App) deferredAccessLogging(accessLogger *zap.Logger, r *http.Request, accessLogDetails *carbonapipb.AccessLogDetails, t time.Time, level zapcore.Level) {
 	accessLogDetails.Runtime = time.Since(t).Seconds()
 	accessLogDetails.RequestMethod = r.Method
 
 	fields, err := accessLogDetails.GetLogFields()
 	if err != nil {
 		accessLogger.Error("could not marshal access log details", zap.Error(err))
 	}
-
-	// TODO (grzkv) This logic is not obvious for the user
-	if logAsError {
-		accessLogger.Error("request failed", fields...)
-		apiMetrics.Errors.Add(1)
-	} else {
-		// TODO (grzkv) The code can differ from the real one. Clean up
-		// accessLogDetails.HttpCode = http.StatusOK
-		accessLogger.Info("request served", fields...)
+	var logMsg string
+	if accessLogDetails.HttpCode/100 < 4 {
+		logMsg = "request served"
+		apiMetrics.Responses.Add(1)
+	} else if accessLogDetails.HttpCode/100 == 4 {
+		logMsg = "request failed with client error"
 		apiMetrics.Responses.Add(1)
+	} else {
+		logMsg = "request failed with server error"
+		apiMetrics.Errors.Add(1)
+	}
+	if ce := accessLogger.Check(level, logMsg); ce != nil {
+		ce.Write(fields...)
 	}
 
 	if app != nil {