log: improve logging

- add a panic recovery handler
        - add logs on unexpected error paths
        - use logger's panic method

WORKSPACE

@@ -1113,3 +1113,10 @@ go_repository(
     sum = "h1:yU/FENpkHYISWsQrbr3pcZOBj0EuRjPzNc1+dTCLu44=",
     version = "v0.0.0-20160317154340-7f45deb8130a",
 )
+
+go_repository(
+    name = "com_github_gorilla_handlers",
+    importpath = "github.com/gorilla/handlers",
+    sum = "h1:0QniY0USkHQ1RGCLfKxeNHK9bkDHGRYGNDFBCS+YARg=",
+    version = "v1.4.2",
+)

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/getlantern/deepcopy v0.0.0-20160317154340-7f45deb8130a
 	github.com/go-chi/chi v4.0.2+incompatible // indirect
 	github.com/gofrs/uuid v3.2.0+incompatible
+	github.com/gorilla/handlers v1.4.2
 	github.com/gorilla/mux v1.7.3
 	github.com/json-iterator/go v1.1.6
 	github.com/jtblin/go-ldap-client v0.0.0-20170223121919-b73f66626b33
@@ -33,4 +34,5 @@ require (
 	gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect
 	gopkg.in/ldap.v2 v2.5.1
 	gopkg.in/resty.v1 v1.12.0
+	honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099
 )

go.sum

@@ -62,6 +62,8 @@ github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
 github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1 h1:EGx4pi6eqNxGaHF6qqu48+N2wcFQ5qg5FXgOdqsJ5d8=
 github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
+github.com/gorilla/handlers v1.4.2 h1:0QniY0USkHQ1RGCLfKxeNHK9bkDHGRYGNDFBCS+YARg=
+github.com/gorilla/handlers v1.4.2/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ=
 github.com/gorilla/mux v1.7.3 h1:gnP5JzjVOuiZD07fKKToCAOjS0yOpj/qPETTXCCS6hw=
 github.com/gorilla/mux v1.7.3/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
 github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
@@ -244,4 +246,5 @@ gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bl
 gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099 h1:XJP7lxbSxWLOMNdBE4B/STaqVy6L73o0knwj2vIlxnw=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

pkg/api/BUILD.bazel

@@ -8,7 +8,6 @@ go_library(
         "controller.go",
         "errors.go",
         "ldap.go",
-        "log.go",
         "regexp.go",
         "routes.go",
     ],
@@ -17,14 +16,15 @@ go_library(
     deps = [
         "//docs:go_default_library",
         "//errors:go_default_library",
+        "//pkg/log:go_default_library",
         "//pkg/storage:go_default_library",
         "@com_github_getlantern_deepcopy//:go_default_library",
+        "@com_github_gorilla_handlers//:go_default_library",
         "@com_github_gorilla_mux//:go_default_library",
         "@com_github_json_iterator_go//:go_default_library",
         "@com_github_jtblin_go_ldap_client//:go_default_library",
         "@com_github_opencontainers_distribution_spec//:go_default_library",
         "@com_github_opencontainers_image_spec//specs-go/v1:go_default_library",
-        "@com_github_rs_zerolog//:go_default_library",
         "@com_github_swaggo_http_swagger//:go_default_library",
         "@in_gopkg_ldap_v2//:go_default_library",
         "@org_golang_x_crypto//bcrypt:go_default_library",

pkg/api/config.go

@@ -2,9 +2,9 @@ package api
 
 import (
 	"github.com/anuvu/zot/errors"
+	"github.com/anuvu/zot/pkg/log"
 	"github.com/getlantern/deepcopy"
 	dspec "github.com/opencontainers/distribution-spec"
-	"github.com/rs/zerolog"
 )
 
 //nolint (gochecknoglobals)
@@ -92,7 +92,7 @@ func (c *Config) Sanitize() *Config {
 	return c
 }
 
-func (c *Config) Validate(log zerolog.Logger) error {
+func (c *Config) Validate(log log.Logger) error {
 	// LDAP configuration
 	if c.HTTP.Auth != nil && c.HTTP.Auth.LDAP != nil {
 		l := c.HTTP.Auth.LDAP

pkg/api/controller.go

@@ -9,21 +9,22 @@ import (
 	"net/http"
 
 	"github.com/anuvu/zot/errors"
+	"github.com/anuvu/zot/pkg/log"
 	"github.com/anuvu/zot/pkg/storage"
+	"github.com/gorilla/handlers"
 	"github.com/gorilla/mux"
-	"github.com/rs/zerolog"
 )
 
 type Controller struct {
 	Config     *Config
 	Router     *mux.Router
 	ImageStore *storage.ImageStore
-	Log        zerolog.Logger
+	Log        log.Logger
 	Server     *http.Server
 }
 
 func NewController(config *Config) *Controller {
-	return &Controller{Config: config, Log: NewLogger(config)}
+	return &Controller{Config: config, Log: log.NewLogger(config.Log.Level, config.Log.Output)}
 }
 
 func (c *Controller) Run() error {
@@ -37,7 +38,8 @@ func (c *Controller) Run() error {
 	c.Log.Info().Interface("params", c.Config.Sanitize()).Msg("configuration settings")
 
 	engine := mux.NewRouter()
-	engine.Use(Logger(c.Log))
+	engine.Use(log.SessionLogger(c.Log), handlers.RecoveryHandler(handlers.RecoveryLogger(c.Log),
+		handlers.PrintRecoveryStack(false)))
 	c.Router = engine
 	_ = NewRouteHandler(c)
 

pkg/api/ldap.go

@@ -7,8 +7,8 @@ import (
 	"time"
 
 	"github.com/anuvu/zot/errors"
+	"github.com/anuvu/zot/pkg/log"
 	"github.com/jtblin/go-ldap-client"
-	"github.com/rs/zerolog"
 	goldap "gopkg.in/ldap.v2"
 )
 
@@ -18,7 +18,7 @@ type LDAPClient struct {
 	ldap.LDAPClient
 	subtreeSearch bool
 	clientCAs     *x509.CertPool
-	log           zerolog.Logger
+	log           log.Logger
 }
 
 // Connect connects to the ldap backend.

pkg/api/routes.go

@@ -162,6 +162,7 @@ func (rh *RouteHandler) CheckManifest(w http.ResponseWriter, r *http.Request) {
 		case errors.ErrManifestNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			WriteJSON(w, http.StatusInternalServerError, NewError(MANIFEST_INVALID, map[string]string{"reference": reference}))
 		}
 		return
@@ -213,6 +214,7 @@ func (rh *RouteHandler) GetManifest(w http.ResponseWriter, r *http.Request) {
 		case errors.ErrManifestNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -273,6 +275,7 @@ func (rh *RouteHandler) UpdateManifest(w http.ResponseWriter, r *http.Request) {
 		case errors.ErrBlobNotFound:
 			WriteJSON(w, http.StatusBadRequest, NewError(BLOB_UNKNOWN, map[string]string{"blob": digest}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -314,6 +317,7 @@ func (rh *RouteHandler) DeleteManifest(w http.ResponseWriter, r *http.Request) {
 		case errors.ErrManifestNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -358,6 +362,7 @@ func (rh *RouteHandler) CheckBlob(w http.ResponseWriter, r *http.Request) {
 		case errors.ErrBlobNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(BLOB_UNKNOWN, map[string]string{"digest": digest}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -409,6 +414,7 @@ func (rh *RouteHandler) GetBlob(w http.ResponseWriter, r *http.Request) {
 		case errors.ErrBlobNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(BLOB_UNKNOWN, map[string]string{"digest": digest}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -453,6 +459,7 @@ func (rh *RouteHandler) DeleteBlob(w http.ResponseWriter, r *http.Request) {
 		case errors.ErrBlobNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(BLOB_UNKNOWN, map[string]string{"digest": digest}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -487,6 +494,7 @@ func (rh *RouteHandler) CreateBlobUpload(w http.ResponseWriter, r *http.Request)
 		case errors.ErrRepoNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(NAME_UNKNOWN, map[string]string{"name": name}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -536,6 +544,7 @@ func (rh *RouteHandler) GetBlobUpload(w http.ResponseWriter, r *http.Request) {
 		case errors.ErrUploadNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"uuid": uuid}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -613,6 +622,7 @@ func (rh *RouteHandler) PatchBlobUpload(w http.ResponseWriter, r *http.Request)
 		case errors.ErrUploadNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"uuid": uuid}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -708,6 +718,7 @@ func (rh *RouteHandler) UpdateBlobUpload(w http.ResponseWriter, r *http.Request)
 			case errors.ErrUploadNotFound:
 				WriteJSON(w, http.StatusNotFound, NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"uuid": uuid}))
 			default:
+				rh.c.Log.Error().Err(err).Msg("unexpected error")
 				w.WriteHeader(http.StatusInternalServerError)
 			}
 			return
@@ -726,6 +737,7 @@ func (rh *RouteHandler) UpdateBlobUpload(w http.ResponseWriter, r *http.Request)
 		case errors.ErrUploadNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"uuid": uuid}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return
@@ -769,6 +781,7 @@ func (rh *RouteHandler) DeleteBlobUpload(w http.ResponseWriter, r *http.Request)
 		case errors.ErrUploadNotFound:
 			WriteJSON(w, http.StatusNotFound, NewError(BLOB_UPLOAD_UNKNOWN, map[string]string{"uuid": uuid}))
 		default:
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		return

pkg/log/BUILD.bazel

@@ -0,0 +1,12 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["log.go"],
+    importpath = "github.com/anuvu/zot/pkg/log",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@com_github_gorilla_mux//:go_default_library",
+        "@com_github_rs_zerolog//:go_default_library",
+    ],
+)

pkg/log/log.go

@@ -1,4 +1,4 @@
-package api
+package log
 
 import (
 	"net/http"
@@ -9,24 +9,33 @@ import (
 	"github.com/rs/zerolog"
 )
 
-func NewLogger(config *Config) zerolog.Logger {
+// Logger extends zerolog's Logger
+type Logger struct {
+	zerolog.Logger
+}
+
+func (l Logger) Println(v ...interface{}) {
+	l.Logger.Error().Msg("panic recovered")
+}
+
+func NewLogger(level string, output string) Logger {
 	zerolog.TimeFieldFormat = time.RFC3339Nano
-	lvl, err := zerolog.ParseLevel(config.Log.Level)
+	lvl, err := zerolog.ParseLevel(level)
 	if err != nil {
 		panic(err)
 	}
 	zerolog.SetGlobalLevel(lvl)
 	var log zerolog.Logger
-	if config.Log.Output == "" {
+	if output == "" {
 		log = zerolog.New(os.Stdout)
 	} else {
-		file, err := os.OpenFile(config.Log.Output, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600)
+		file, err := os.OpenFile(output, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600)
 		if err != nil {
 			panic(err)
 		}
 		log = zerolog.New(file)
 	}
-	return log.With().Timestamp().Logger()
+	return Logger{Logger: log.With().Timestamp().Logger()}
 }
 
 type statusWriter struct {
@@ -49,7 +58,7 @@ func (w *statusWriter) Write(b []byte) (int, error) {
 	return n, err
 }
 
-func Logger(log zerolog.Logger) mux.MiddlewareFunc {
+func SessionLogger(log Logger) mux.MiddlewareFunc {
 	l := log.With().Str("module", "http").Logger()
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

pkg/storage/BUILD.bazel

@@ -7,6 +7,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//errors:go_default_library",
+        "//pkg/log:go_default_library",
         "@com_github_gofrs_uuid//:go_default_library",
         "@com_github_opencontainers_go_digest//:go_default_library",
         "@com_github_opencontainers_image_spec//specs-go/v1:go_default_library",
@@ -21,6 +22,7 @@ go_test(
     embed = [":go_default_library"],
     race = "on",
     deps = [
+        "//pkg/log:go_default_library",
         "@com_github_opencontainers_go_digest//:go_default_library",
         "@com_github_opencontainers_image_spec//specs-go/v1:go_default_library",
         "@com_github_rs_zerolog//:go_default_library",

pkg/storage/storage.go

@@ -10,6 +10,7 @@ import (
 	"sync"
 
 	"github.com/anuvu/zot/errors"
+	zlog "github.com/anuvu/zot/pkg/log"
 	guuid "github.com/gofrs/uuid"
 	godigest "github.com/opencontainers/go-digest"
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -32,7 +33,7 @@ type ImageStore struct {
 	log         zerolog.Logger
 }
 
-func NewImageStore(rootDir string, log zerolog.Logger) *ImageStore {
+func NewImageStore(rootDir string, log zlog.Logger) *ImageStore {
 	is := &ImageStore{rootDir: rootDir,
 		lock:        &sync.Mutex{},
 		blobUploads: make(map[string]BlobUpload),
@@ -69,11 +70,10 @@ func (is *ImageStore) InitRepo(name string) error {
 		il := ispec.ImageLayout{Version: ispec.ImageLayoutVersion}
 		buf, err := json.Marshal(il)
 		if err != nil {
-			panic(err)
+			is.log.Panic().Err(err).Msg("unable to marshal JSON")
 		}
 		if err := ioutil.WriteFile(ilPath, buf, 0644); err != nil {
-			is.log.Error().Err(err).Str("file", ilPath).Msg("unable to write file")
+			is.log.Panic().Err(err).Str("file", ilPath).Msg("unable to write file")
-			panic(err)
 		}
 	}
 
@@ -84,11 +84,10 @@ func (is *ImageStore) InitRepo(name string) error {
 		index.SchemaVersion = 2
 		buf, err := json.Marshal(index)
 		if err != nil {
-			panic(err)
+			is.log.Panic().Err(err).Msg("unable to marshal JSON")
 		}
 		if err := ioutil.WriteFile(indexPath, buf, 0644); err != nil {
-			is.log.Error().Err(err).Str("file", indexPath).Msg("unable to write file")
+			is.log.Panic().Err(err).Str("file", indexPath).Msg("unable to write file")
-			panic(err)
 		}
 	}
 

pkg/storage/storage_test.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/anuvu/zot/pkg/log"
 	"github.com/anuvu/zot/pkg/storage"
 	godigest "github.com/opencontainers/go-digest"
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -22,7 +23,7 @@ func TestAPIs(t *testing.T) {
 	}
 	defer os.RemoveAll(dir)
 
-	il := storage.NewImageStore(dir, zerolog.New(os.Stdout))
+	il := storage.NewImageStore(dir, log.Logger{Logger: zerolog.New(os.Stdout)})
 
 	Convey("Repo layout", t, func(c C) {
 		repoName := "test"