From c5f5cccd21fd11ebb0c1005c5c36d404c1c30976 Mon Sep 17 00:00:00 2001 From: Dusan Borovcanin Date: Wed, 17 Jan 2024 17:52:20 +0100 Subject: [PATCH] Use slog for logging Signed-off-by: Dusan Borovcanin --- api/endpoint_test.go | 6 +++--- api/logging.go | 24 ++++++++++++------------ api/transport.go | 4 ++-- api/utils.go | 4 ++-- cmd/main.go | 31 +++++++++++++++++++++++-------- docker/nginx/nginx.conf | 2 +- internal/server/http/http.go | 4 ++-- internal/server/server.go | 7 +++---- pkg/client/client.go | 7 +++---- pkg/client/client_test.go | 5 ++--- timescale/init.go | 2 +- timescale/timescale.go | 10 +++++----- timescale/timescale_test.go | 2 +- 13 files changed, 60 insertions(+), 48 deletions(-) diff --git a/api/endpoint_test.go b/api/endpoint_test.go index 9db2929..f34c72e 100644 --- a/api/endpoint_test.go +++ b/api/endpoint_test.go @@ -2,6 +2,7 @@ package api import ( "fmt" + "log/slog" "net/http" "net/http/httptest" "strings" @@ -9,7 +10,6 @@ import ( "github.com/absmach/callhome" "github.com/absmach/callhome/mocks" - "github.com/absmach/magistrala/logger" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "go.opentelemetry.io/otel/trace" @@ -18,7 +18,7 @@ import ( func TestEndpointsRetrieve(t *testing.T) { svc := mocks.NewService(t) svc.On("Retrieve", mock.Anything, callhome.PageMetadata{Limit: 10}).Return(callhome.TelemetryPage{}, nil) - h := MakeHandler(svc, trace.NewNoopTracerProvider(), logger.NewMock()) + h := MakeHandler(svc, trace.NewNoopTracerProvider(), slog.Default()) server := httptest.NewServer(h) client := server.Client() testCases := []struct { @@ -52,7 +52,7 @@ func TestEndpointSave(t *testing.T) { }` svc := mocks.NewService(t) svc.On("Save", mock.Anything, mock.AnythingOfType("callhome.Telemetry")).Return(nil) - h := MakeHandler(svc, trace.NewNoopTracerProvider(), logger.NewMock()) + h := MakeHandler(svc, trace.NewNoopTracerProvider(), slog.Default()) server := httptest.NewServer(h) client := server.Client() testCases := []struct { diff --git a/api/logging.go b/api/logging.go index b68dadc..1a0a899 100644 --- a/api/logging.go +++ b/api/logging.go @@ -3,21 +3,21 @@ package api import ( "context" "fmt" + "log/slog" "time" "github.com/absmach/callhome" - "github.com/absmach/magistrala/logger" ) var _ callhome.Service = (*loggingMiddleware)(nil) type loggingMiddleware struct { - hommingLogger logger.Logger - svc callhome.Service + logger *slog.Logger + svc callhome.Service } // LoggingMiddleware is a middleware that adds logging facilities to the core homing service. -func LoggingMiddleware(svc callhome.Service, logger logger.Logger) callhome.Service { +func LoggingMiddleware(svc callhome.Service, logger *slog.Logger) callhome.Service { return &loggingMiddleware{logger, svc} } @@ -26,10 +26,10 @@ func (lm *loggingMiddleware) Retrieve(ctx context.Context, pm callhome.PageMetad defer func(begin time.Time) { message := fmt.Sprintf("Method retrieve with took %s to complete", time.Since(begin)) if err != nil { - lm.hommingLogger.Warn(fmt.Sprintf("%s with error: %s.", message, err)) + lm.logger.Warn(fmt.Sprintf("%s with error: %s.", message, err)) return } - lm.hommingLogger.Info(fmt.Sprintf("%s without errors.", message)) + lm.logger.Info(fmt.Sprintf("%s without errors.", message)) }(time.Now()) return lm.svc.Retrieve(ctx, pm, filters) @@ -40,10 +40,10 @@ func (lm *loggingMiddleware) Save(ctx context.Context, t callhome.Telemetry) (er defer func(begin time.Time) { message := fmt.Sprintf("Method save telemetry event took %s to complete", time.Since(begin)) if err != nil { - lm.hommingLogger.Warn(fmt.Sprintf("%s with error: %s.", message, err)) + lm.logger.Warn(fmt.Sprintf("%s with error: %s.", message, err)) return } - lm.hommingLogger.Info(fmt.Sprintf("%s without errors.", message)) + lm.logger.Info(fmt.Sprintf("%s without errors.", message)) }(time.Now()) return lm.svc.Save(ctx, t) @@ -53,10 +53,10 @@ func (lm *loggingMiddleware) RetrieveSummary(ctx context.Context, filters callho defer func(begin time.Time) { message := fmt.Sprintf("Method retrieve summary event took %s to complete", time.Since(begin)) if err != nil { - lm.hommingLogger.Warn(fmt.Sprintf("%s with error: %s.", message, err)) + lm.logger.Warn(fmt.Sprintf("%s with error: %s.", message, err)) return } - lm.hommingLogger.Info(fmt.Sprintf("%s without errors.", message)) + lm.logger.Info(fmt.Sprintf("%s without errors.", message)) }(time.Now()) return lm.svc.RetrieveSummary(ctx, filters) @@ -67,10 +67,10 @@ func (lm *loggingMiddleware) ServeUI(ctx context.Context, filters callhome.Telem defer func(begin time.Time) { message := fmt.Sprintf("Method serve ui event took %s to complete", time.Since(begin)) if err != nil { - lm.hommingLogger.Warn(fmt.Sprintf("%s with error: %s.", message, err)) + lm.logger.Warn(fmt.Sprintf("%s with error: %s.", message, err)) return } - lm.hommingLogger.Info(fmt.Sprintf("%s without errors.", message)) + lm.logger.Info(fmt.Sprintf("%s without errors.", message)) }(time.Now()) return lm.svc.ServeUI(ctx, filters) diff --git a/api/transport.go b/api/transport.go index ea448d2..1ce9f5a 100644 --- a/api/transport.go +++ b/api/transport.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "log/slog" "net/http" "strings" "time" @@ -10,7 +11,6 @@ import ( "github.com/absmach/callhome" "github.com/absmach/callhome/timescale" "github.com/absmach/magistrala" - "github.com/absmach/magistrala/logger" "github.com/absmach/magistrala/pkg/errors" "github.com/absmach/magistrala/pkg/uuid" kithttp "github.com/go-kit/kit/transport/http" @@ -37,7 +37,7 @@ const ( ) // MakeHandler returns a HTTP handler for API endpoints. -func MakeHandler(svc callhome.Service, tp trace.TracerProvider, logger logger.Logger) http.Handler { +func MakeHandler(svc callhome.Service, tp trace.TracerProvider, logger *slog.Logger) http.Handler { opts := []kithttp.ServerOption{ kithttp.ServerErrorEncoder(LoggingErrorEncoder(logger, encodeError)), } diff --git a/api/utils.go b/api/utils.go index f827d93..2e6c6e3 100644 --- a/api/utils.go +++ b/api/utils.go @@ -2,10 +2,10 @@ package api import ( "context" + "log/slog" "net/http" "strconv" - "github.com/absmach/magistrala/logger" "github.com/absmach/magistrala/pkg/errors" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-zoo/bone" @@ -19,7 +19,7 @@ type ErrorRes struct { var ErrInvalidQueryParams = errors.New("invalid query params") // LoggingErrorEncoder is a go-kit error encoder logging decorator. -func LoggingErrorEncoder(logger logger.Logger, enc kithttp.ErrorEncoder) kithttp.ErrorEncoder { +func LoggingErrorEncoder(logger *slog.Logger, enc kithttp.ErrorEncoder) kithttp.ErrorEncoder { return func(ctx context.Context, err error, w http.ResponseWriter) { switch err { case ErrLimitSize, ErrOffsetSize: diff --git a/cmd/main.go b/cmd/main.go index 55a3ab7..2fde69c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -3,8 +3,11 @@ package main import ( "context" "fmt" + "io" "log" + "log/slog" "os" + "time" "github.com/absmach/callhome" "github.com/absmach/callhome/api" @@ -17,7 +20,6 @@ import ( "github.com/absmach/callhome/timescale" "github.com/absmach/callhome/timescale/tracing" stracing "github.com/absmach/callhome/tracing" - mglog "github.com/absmach/magistrala/logger" "github.com/jmoiron/sqlx" "go.opentelemetry.io/otel/trace" "golang.org/x/sync/errgroup" @@ -45,30 +47,29 @@ func main() { log.Fatalf("failed to load %s configuration : %s", svcName, err) } - logger, err := mglog.New(os.Stdout, cfg.LogLevel) + logger, err := newLogger(os.Stdout, cfg.LogLevel) if err != nil { log.Fatalf(fmt.Sprintf("failed to init logger: %s", err.Error())) } - timescaleDB, err := postgres.Setup(envPrefix, timescale.Migration()) if err != nil { - logger.Fatal(fmt.Sprintf("failed to setup timescale db : %s", err)) + log.Fatalf("failed to setup timescale db : %s", err) } tp, err := jaegerClient.NewProvider(svcName, cfg.JaegerURL) if err != nil { - logger.Error(fmt.Sprintf("Failed to init Jaeger: %s", err)) + log.Fatalf("Failed to init Jaeger: %s", err) } defer func() { if err := tp.Shutdown(context.Background()); err != nil { - logger.Error(fmt.Sprintf("Error shutting down tracer provider: %v", err)) + log.Fatalf("Error shutting down tracer provider: %v", err) } }() tracer := tp.Tracer(svcName) svc, err := newService(ctx, logger, cfg.IPDatabaseFile, timescaleDB, tracer) if err != nil { - logger.Error(fmt.Sprintf("failed to initialize service: %s", err.Error())) + log.Fatalf("failed to initialize service: %s", err) return } @@ -92,7 +93,7 @@ func main() { } } -func newService(ctx context.Context, logger mglog.Logger, ipDB string, timescaleDB *sqlx.DB, tracer trace.Tracer) (callhome.Service, error) { +func newService(ctx context.Context, logger *slog.Logger, ipDB string, timescaleDB *sqlx.DB, tracer trace.Tracer) (callhome.Service, error) { timescaleRepo := timescale.New(timescaleDB) timescaleRepo = tracing.New(tracer, timescaleRepo) locSvc, err := callhome.NewLocationService(ipDB) @@ -107,3 +108,17 @@ func newService(ctx context.Context, logger mglog.Logger, ipDB string, timescale svc = api.LoggingMiddleware(svc, logger) return svc, nil } + +// New returns wrapped logger. +func newLogger(w io.Writer, levelText string) (*slog.Logger, error) { + var level slog.Level + if err := level.UnmarshalText([]byte(levelText)); err != nil { + return &slog.Logger{}, fmt.Errorf(`{"level":"error","message":"%s: %s","ts":"%s"}`, err, levelText, time.RFC3339Nano) + } + + logHandler := slog.NewJSONHandler(w, &slog.HandlerOptions{ + Level: level, + }) + + return slog.New(logHandler), nil +} diff --git a/docker/nginx/nginx.conf b/docker/nginx/nginx.conf index fb6814a..da40d9e 100644 --- a/docker/nginx/nginx.conf +++ b/docker/nginx/nginx.conf @@ -7,7 +7,7 @@ http{ server callhome-server:8855; } - map $http_apikey $mf_version{ + map $http_apikey $mg_version{ "77e04a7c-f207-40dd-8950-c344871fd516" "0.13"; "b9738244-ae83-48ec-8601-1be1a1e47788" "0.14"; } diff --git a/internal/server/http/http.go b/internal/server/http/http.go index 7635cc4..ab7f893 100644 --- a/internal/server/http/http.go +++ b/internal/server/http/http.go @@ -3,11 +3,11 @@ package http import ( "context" "fmt" + "log/slog" "net/http" "time" mfserver "github.com/absmach/callhome/internal/server" - "github.com/absmach/magistrala/logger" ) const ( @@ -23,7 +23,7 @@ type Server struct { var _ mfserver.Server = (*Server)(nil) -func New(ctx context.Context, cancel context.CancelFunc, name string, config mfserver.Config, handler http.Handler, logger logger.Logger) mfserver.Server { +func New(ctx context.Context, cancel context.CancelFunc, name string, config mfserver.Config, handler http.Handler, logger *slog.Logger) mfserver.Server { listenFullAddress := fmt.Sprintf("%s:%s", config.Host, config.Port) server := &http.Server{Addr: listenFullAddress, Handler: handler} return &Server{ diff --git a/internal/server/server.go b/internal/server/server.go index 1a23042..5706b34 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -3,11 +3,10 @@ package server import ( "context" "fmt" + "log/slog" "os" "os/signal" "syscall" - - "github.com/absmach/magistrala/logger" ) type Server interface { @@ -28,7 +27,7 @@ type BaseServer struct { Name string Address string Config Config - Logger logger.Logger + Logger *slog.Logger Protocol string } @@ -47,7 +46,7 @@ func stopAllServer(servers ...Server) error { return err } -func StopSignalHandler(ctx context.Context, cancel context.CancelFunc, logger logger.Logger, svcName string, servers ...Server) error { +func StopSignalHandler(ctx context.Context, cancel context.CancelFunc, logger *slog.Logger, svcName string, servers ...Server) error { var err error c := make(chan os.Signal, 2) signal.Notify(c, syscall.SIGINT, syscall.SIGABRT) diff --git a/pkg/client/client.go b/pkg/client/client.go index 058ce0c..defc365 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -6,12 +6,11 @@ import ( "encoding/json" "fmt" "io" + "log/slog" "net/http" "net/netip" "strings" "time" - - mflog "github.com/absmach/magistrala/logger" ) const ( @@ -31,12 +30,12 @@ var ipEndpoints = []string{ type homingService struct { serviceName string version string - logger mflog.Logger + logger *slog.Logger cancel context.CancelFunc httpClient http.Client } -func New(svc, version string, homingLogger mflog.Logger, cancel context.CancelFunc) *homingService { +func New(svc, version string, homingLogger *slog.Logger, cancel context.CancelFunc) *homingService { return &homingService{ serviceName: svc, version: version, diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index e4c82b4..6315827 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -4,15 +4,14 @@ import ( "bytes" "context" "io" + "log/slog" "net/http" "testing" - - "github.com/absmach/magistrala/logger" ) func TestGetIp(t *testing.T) { _, cancel := context.WithCancel(context.TODO()) - hs := New("test_svc", "test.1", logger.NewMock(), cancel) + hs := New("test_svc", "test.1", slog.Default(), cancel) for _, endpoint := range ipEndpoints { if _, err := hs.getIP(endpoint); err != nil { t.Errorf("endpoint %s ip request unsuccessful with err : %v", endpoint, err) diff --git a/timescale/init.go b/timescale/init.go index 22409cd..14e2336 100644 --- a/timescale/init.go +++ b/timescale/init.go @@ -18,7 +18,7 @@ func Migration() migrate.MemoryMigrationSource { ip_address TEXT NOT NULL, longitude FLOAT NOT NULL, latitude FLOAT NOT NULL, - mf_version TEXT, + mg_version TEXT, service TEXT, country TEXT, city TEXT, diff --git a/timescale/timescale.go b/timescale/timescale.go index 3bb7973..086309a 100644 --- a/timescale/timescale.go +++ b/timescale/timescale.go @@ -32,7 +32,7 @@ func (r repo) RetrieveAll(ctx context.Context, pm callhome.PageMetadata, filters %s GROUP BY ip_address ) - SELECT ad.ip_address, ad.services, t.time, t.service_time, t.longitude, t.latitude, t.mf_version, t.country, t.city + SELECT ad.ip_address, ad.services, t.time, t.service_time, t.longitude, t.latitude, t.mg_version, t.country, t.city FROM aggregated_data ad INNER JOIN ( SELECT DISTINCT ON (ip_address) * @@ -93,9 +93,9 @@ func (r repo) RetrieveAll(ctx context.Context, pm callhome.PageMetadata, filters // Save creates record in repo. func (r repo) Save(ctx context.Context, t callhome.Telemetry) error { q := `INSERT INTO telemetry (ip_address, longitude, latitude, - mf_version, service, time, country, city, service_time) + mg_version, service, time, country, city, service_time) VALUES (:ip_address, :longitude, :latitude, - :mf_version, :service, :time, :country, :city, :service_time);` + :mg_version, :service, :time, :country, :city, :service_time);` tx, err := r.db.BeginTxx(ctx, nil) if err != nil { @@ -174,7 +174,7 @@ func (r repo) RetrieveSummary(ctx context.Context, filters callhome.TelemetryFil summary.Services = append(summary.Services, val) } - q3 := fmt.Sprintf(`select distinct mf_version from telemetry %s;`, filterQuery) + q3 := fmt.Sprintf(`select distinct mg_version from telemetry %s;`, filterQuery) versionRows, err := r.db.NamedQuery(q3, params) if err != nil { return callhome.TelemetrySummary{}, err @@ -213,7 +213,7 @@ func generateQuery(filters callhome.TelemetryFilters) (string, map[string]interf } if filters.Version != "" { - queries = append(queries, "mf_version = :version") + queries = append(queries, "mg_version = :version") params["version"] = filters.Version } diff --git a/timescale/timescale_test.go b/timescale/timescale_test.go index 2aa23a8..3952bc7 100644 --- a/timescale/timescale_test.go +++ b/timescale/timescale_test.go @@ -135,7 +135,7 @@ func TestRetrieveAll(t *testing.T) { repo := New(sqlxDB) rows := sqlmock.NewRows( - []string{"ip_address", "longitude", "latitude", "mf_version", "service", "time", "country", "city", "service_time"}, + []string{"ip_address", "longitude", "latitude", "mg_version", "service", "time", "country", "city", "service_time"}, ).AddRow(mTel.IpAddress, mTel.Longitude, mTel.Latitude, mTel.Version, mTel.Service, mTel.LastSeen, mTel.Country, mTel.City, mTel.ServiceTime) rows2 := sqlmock.NewRows(