Skip to content

Commit

Permalink
feat: Change log middleware and inject logger in context
Browse files Browse the repository at this point in the history
BREAKING CHANGE
  • Loading branch information
oxyno-zeta committed Jul 3, 2021
1 parent 5bb59ee commit 1e4b610
Show file tree
Hide file tree
Showing 10 changed files with 107 additions and 45 deletions.
3 changes: 2 additions & 1 deletion pkg/s3-proxy/authx/authentication/basic-auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/authx/models"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/config"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/log"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/middlewares"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/utils"
"github.com/thoas/go-funk"
Expand All @@ -19,7 +20,7 @@ func (s *service) basicAuthMiddleware(res *config.Resource) func(http.Handler) h
basicConfig := s.cfg.AuthProviders.Basic[res.Provider]
basicAuthUserConfigList := res.Basic.Credentials
// Get logger from request
logEntry := middlewares.GetLogEntry(r)
logEntry := log.GetLoggerFromContext(r.Context())
path := r.URL.RequestURI()
// Get bucket request context from request
brctx := middlewares.GetBucketRequestContext(r)
Expand Down
3 changes: 2 additions & 1 deletion pkg/s3-proxy/authx/authentication/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/gobwas/glob"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/authx/models"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/config"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/log"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/metrics"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/middlewares"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/utils"
Expand All @@ -29,7 +30,7 @@ type service struct {
func (s *service) Middleware(resources []*config.Resource) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
logEntry := middlewares.GetLogEntry(r)
logEntry := log.GetLoggerFromContext(r.Context())

// Check if resources are empty
if len(resources) == 0 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/s3-proxy/authx/authentication/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (s *service) OIDCEndpoints(providerKey string, oidcCfg *config.OIDCAuthConf

mux.HandleFunc(mainRedirectURLObject.Path, func(w http.ResponseWriter, r *http.Request) {
// Get logger from request
logEntry := middlewares.GetLogEntry(r)
logEntry := log.GetLoggerFromContext(r.Context())

// ! In this particular case, no bucket request context because mounted in general and not per target

Expand Down Expand Up @@ -188,7 +188,7 @@ func (s *service) oidcAuthorizationMiddleware(res *config.Resource) func(http.Ha
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
oidcAuthCfg := s.cfg.AuthProviders.OIDC[res.Provider]
// Get logger from request
logEntry := middlewares.GetLogEntry(r)
logEntry := log.GetLoggerFromContext(r.Context())
path := r.URL.RequestURI()
// Get bucket request context from request
brctx := middlewares.GetBucketRequestContext(r)
Expand Down
4 changes: 2 additions & 2 deletions pkg/s3-proxy/authx/authorization/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/authx/authentication"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/authx/models"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/config"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/log"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/metrics"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/middlewares"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/utils"
Expand All @@ -17,7 +18,7 @@ var errAuthorizationMiddlewareNotSupported = errors.New("authorization not suppo
func Middleware(cfg *config.Config, metricsCl metrics.Client) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
logger := middlewares.GetLogEntry(r)
logger := log.GetLoggerFromContext(r.Context())

// Get request resource from request
resource := authentication.GetRequestResource(r)
Expand Down Expand Up @@ -58,7 +59,6 @@ func Middleware(cfg *config.Config, metricsCl metrics.Client) func(http.Handler)

// Get request data
requestURI := r.URL.RequestURI()
// httpMethod := r.Method

// Get bucket request context
brctx := middlewares.GetBucketRequestContext(r)
Expand Down
8 changes: 8 additions & 0 deletions pkg/s3-proxy/log/contextkey.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package log

// contextKey is a value for use with context.WithValue. It's used as
// a pointer so it fits in an interface{} without allocation. This technique
// for defining context keys was copied from Go 1.7's new use of context in net/http.
type contextKey struct {
name string
}
Original file line number Diff line number Diff line change
@@ -1,41 +1,66 @@
package middlewares
package log

import (
"context"
"fmt"
"net/http"
"time"

"github.com/go-chi/chi/v5/middleware"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/log"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/utils"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/tracing"
"github.com/sirupsen/logrus"
)

// HTTPAddLoggerToContextMiddleware HTTP Middleware that will add request logger to request context.
func HTTPAddLoggerToContextMiddleware() func(next http.Handler) http.Handler {
return func(h http.Handler) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
// Get logger from request
logger := GetLogEntry(r)
// Add logger to request context in order to keep it
ctx := context.WithValue(r.Context(), loggerContextKey, logger)
// Create new request with new context
r = r.WithContext(ctx)

// Next
h.ServeHTTP(rw, r)
})
}
}

// Copied and modified from https://github.com/go-chi/chi/blob/master/_examples/logging/main.go

// NewStructuredLogger Generate a new structured logger.
func NewStructuredLogger(logger log.Logger) func(next http.Handler) http.Handler {
return middleware.RequestLogger(&StructuredLogger{logger})
func NewStructuredLogger(
logger Logger,
getTraceID func(r *http.Request) string,
getClientIP func(r *http.Request) string,
getRequestURI func(r *http.Request) string,
) func(next http.Handler) http.Handler {
return middleware.RequestLogger(&StructuredLogger{
Logger: logger,
GetTraceID: getTraceID,
GetClientIP: getClientIP,
GetRequestURI: getRequestURI,
})
}

// StructuredLogger structured logger.
type StructuredLogger struct {
Logger log.Logger
Logger Logger
GetTraceID func(r *http.Request) string
GetClientIP func(r *http.Request) string
GetRequestURI func(r *http.Request) string
}

// NewLogEntry new log entry.
func (l *StructuredLogger) NewLogEntry(r *http.Request) middleware.LogEntry {
entry := &StructuredLoggerEntry{Logger: l.Logger}
logFields := map[string]interface{}{}

// Get request trace
trace := tracing.GetTraceFromRequest(r)
if trace != nil {
traceIDStr := trace.GetTraceID()
if traceIDStr != "" {
logFields["span_id"] = traceIDStr
}
// Get trace id
traceIDStr := l.GetTraceID(r)
if traceIDStr != "" {
logFields["span_id"] = traceIDStr
}

if reqID := middleware.GetReqID(r.Context()); reqID != "" {
Expand All @@ -53,9 +78,9 @@ func (l *StructuredLogger) NewLogEntry(r *http.Request) middleware.LogEntry {

logFields["remote_addr"] = r.RemoteAddr
logFields["user_agent"] = r.UserAgent()
logFields["client_ip"] = utils.ClientIP(r)
logFields["client_ip"] = l.GetClientIP(r)

logFields["uri"] = utils.GetRequestURI(r)
logFields["uri"] = l.GetRequestURI(r)

entry.Logger = entry.Logger.WithFields(logFields)

Expand All @@ -66,7 +91,7 @@ func (l *StructuredLogger) NewLogEntry(r *http.Request) middleware.LogEntry {

// StructuredLoggerEntry Structured logger entry.
type StructuredLoggerEntry struct {
Logger log.Logger
Logger Logger
}

// Write Write.
Expand Down Expand Up @@ -105,22 +130,8 @@ func (l *StructuredLoggerEntry) Panic(v interface{}, stack []byte) {
// with a call to .Print(), .Info(), etc.

// GetLogEntry get log entry.
func GetLogEntry(r *http.Request) log.Logger {
func GetLogEntry(r *http.Request) Logger {
entry := middleware.GetLogEntry(r).(*StructuredLoggerEntry)

return entry.Logger
}

// LogEntrySetField Log entry set field.
func LogEntrySetField(r *http.Request, key string, value interface{}) {
if entry, ok := r.Context().Value(middleware.LogEntryCtxKey).(*StructuredLoggerEntry); ok {
entry.Logger = entry.Logger.WithField(key, value)
}
}

// LogEntrySetFields Log entry set fields.
func LogEntrySetFields(r *http.Request, fields map[string]interface{}) {
if entry, ok := r.Context().Value(middleware.LogEntryCtxKey).(*StructuredLoggerEntry); ok {
entry.Logger = entry.Logger.WithFields(fields)
}
}
10 changes: 10 additions & 0 deletions pkg/s3-proxy/log/log.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package log

import (
"context"

"github.com/sirupsen/logrus"
)

var loggerContextKey = &contextKey{name: "LOGGER_CONTEXT_KEY"}

type Logger interface {
Configure(level string, format string, filePath string) error

Expand Down Expand Up @@ -57,3 +61,9 @@ func NewLogger() Logger {
FieldLogger: logrus.New(),
}
}

func GetLoggerFromContext(ctx context.Context) Logger {
res, _ := ctx.Value(loggerContextKey).(Logger)

return res
}
18 changes: 17 additions & 1 deletion pkg/s3-proxy/server/internal-server.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/log"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/metrics"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/middlewares"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/utils"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/tracing"
)

type InternalServer struct {
Expand Down Expand Up @@ -75,7 +77,21 @@ func (svr *InternalServer) generateInternalRouter() http.Handler {

r.Use(middleware.RequestID)
r.Use(middleware.RealIP)
r.Use(middlewares.NewStructuredLogger(svr.logger))
r.Use(log.NewStructuredLogger(
svr.logger,
func(r *http.Request) string {
// Get request trace
trace := tracing.GetTraceFromRequest(r)
if trace != nil {
return trace.GetTraceID()
}

return ""
},
utils.ClientIP,
utils.GetRequestURI,
))
r.Use(log.HTTPAddLoggerToContextMiddleware())
r.Use(svr.metricsCl.Instrument("internal"))
r.Use(middleware.Recoverer)

Expand Down
3 changes: 2 additions & 1 deletion pkg/s3-proxy/server/middlewares/bucket-request-context.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/bucket"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/config"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/log"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/metrics"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/server/utils"
"github.com/oxyno-zeta/s3-proxy/pkg/s3-proxy/tracing"
Expand All @@ -27,7 +28,7 @@ func BucketRequestContext(
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
// Get logger
logEntry := GetLogEntry(req)
logEntry := log.GetLoggerFromContext(req.Context())
// Get request URI
requestURI := req.URL.RequestURI()
errorhandlers := &bucket.ErrorHandlers{
Expand Down
24 changes: 19 additions & 5 deletions pkg/s3-proxy/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,21 @@ func (svr *Server) generateRouter() (http.Handler, error) {
// Put tracing middlewares
r.Use(httptracer.Tracer(svr.tracingSvc.GetTracer(), httptraCfg))
r.Use(middlewares.ImproveTracing())
r.Use(middlewares.NewStructuredLogger(svr.logger))
r.Use(log.NewStructuredLogger(
svr.logger,
func(r *http.Request) string {
// Get request trace
trace := tracing.GetTraceFromRequest(r)
if trace != nil {
return trace.GetTraceID()
}

return ""
},
utils.ClientIP,
utils.GetRequestURI,
))
r.Use(log.HTTPAddLoggerToContextMiddleware())
r.Use(svr.metricsCl.Instrument("business"))

// Check if cors is enabled
Expand All @@ -145,7 +159,7 @@ func (svr *Server) generateRouter() (http.Handler, error) {

notFoundHandler := func(w http.ResponseWriter, r *http.Request) {
// Get logger
logger := middlewares.GetLogEntry(r)
logger := log.GetLoggerFromContext(r.Context())
// Get request URI
requestURI := r.URL.RequestURI()
utils.HandleNotFound(logger, w, cfg.Templates, requestURI)
Expand All @@ -154,7 +168,7 @@ func (svr *Server) generateRouter() (http.Handler, error) {
internalServerHandlerGen := func(err error) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
// Get logger
logger := middlewares.GetLogEntry(r)
logger := log.GetLoggerFromContext(r.Context())
// Get request URI
requestURI := r.URL.RequestURI()
utils.HandleInternalServerError(logger, w, cfg.Templates, requestURI, err)
Expand Down Expand Up @@ -184,7 +198,7 @@ func (svr *Server) generateRouter() (http.Handler, error) {
rt2 = rt2.With(authorization.Middleware(cfg, svr.metricsCl))

rt2.Get("/", func(rw http.ResponseWriter, req *http.Request) {
logEntry := middlewares.GetLogEntry(req)
logEntry := log.GetLoggerFromContext(req.Context())
generateTargetList(rw, req.RequestURI, logEntry, cfg)
})
})
Expand Down Expand Up @@ -301,7 +315,7 @@ func (svr *Server) generateRouter() (http.Handler, error) {
// Get request path
requestPath := chi.URLParam(req, "*")
// Get logger
logEntry := middlewares.GetLogEntry(req)
logEntry := log.GetLoggerFromContext(req.Context())
if err := req.ParseForm(); err != nil {
logEntry.Error(err)
brctx.HandleInternalServerError(err, path)
Expand Down

0 comments on commit 1e4b610

Please sign in to comment.