Skip to content

Commit

Permalink
Chore: Moving to slog (folders) (#87000)
Browse files Browse the repository at this point in the history 
* moving folders to slog

* trying to fix the tests

* fix per-logger filters in slog

* allow using slog.Default()

* bring cfg back to keep the pr small

* fix tests

* back to the roots
  • Loading branch information
@zserge
zserge committed May 2, 2024
1 parent a6b7fc5 commit 6ddb8ae
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 80 deletions.
30 changes: 22 additions & 8 deletions pkg/infra/log/log.go
View file
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"fmt"
"io"
"log/slog"
"os"
"path/filepath"
"sort"
Expand Down Expand Up @@ -111,15 +112,26 @@ func (lm *logManager) initialize(loggers []logWithFilters) {
}

func (lm *logManager) New(ctx ...any) *ConcreteLogger {
if len(ctx) == 0 {
// First key-value could be "logger" and a logger name, that would be handled differently
// to allow per-logger filtering. Otherwise a simple concrete logger is returned.
if len(ctx) < 2 {
return lm.ConcreteLogger
}
if ctx[0] != "logger" {
return newConcreteLogger(lm.ConcreteLogger, ctx...)
}

lm.mutex.Lock()
defer lm.mutex.Unlock()

loggerName, ok := ctx[1].(string)
if !ok {
// Logger name could be a string variable or an slog.Value()
loggerName := ""
switch v := ctx[1].(type) {
case string:
loggerName = v
case slog.Value:
loggerName = v.String()
default:
return lm.ConcreteLogger
}

Expand Down Expand Up @@ -200,25 +212,27 @@ func (cl *ConcreteLogger) log(msg string, logLevel level.Value, args ...any) err
return cl.Log(append([]any{level.Key(), logLevel, "msg", msg}, args...)...)
}

func (cl *ConcreteLogger) FromContext(ctx context.Context) Logger {
func FromContext(ctx context.Context) []any {
args := []any{}

for _, p := range ctxLogProviders {
if pArgs, exists := p(ctx); exists {
args = append(args, pArgs...)
}
}
return args
}

func (cl *ConcreteLogger) FromContext(ctx context.Context) Logger {
args := FromContext(ctx)
if len(args) > 0 {
return cl.New(args...)
}

return cl
}

func (cl *ConcreteLogger) New(ctx ...any) *ConcreteLogger {
if len(ctx) == 0 {
root.New()
if len(cl.ctx) == 0 {
return root.New(ctx...)
}

return newConcreteLogger(gokitlog.With(&cl.SwapLogger), ctx...)
Expand Down
41 changes: 41 additions & 0 deletions pkg/infra/log/logtest/slog.go
View file
@@ -0,0 +1,41 @@
package logtest

import (
"context"
"log/slog"
"strings"
"testing"
)

type NopHandler struct{}

func NewNopHandler(t *testing.T) *NopHandler {
return &NopHandler{}
}
func (h *NopHandler) Enabled(_ context.Context, _ slog.Level) bool { return false }
func (h *NopHandler) WithAttrs(_ []slog.Attr) slog.Handler { return h }
func (h *NopHandler) WithGroup(_ string) slog.Handler { return h }
func (h *NopHandler) Handle(_ context.Context, r slog.Record) error { return nil }

type TestHandler struct {
t *testing.T
}

func NewTestHandler(t *testing.T) *TestHandler {
return &TestHandler{t: t}
}

func (th *TestHandler) Enabled(_ context.Context, _ slog.Level) bool { return true }
func (th *TestHandler) WithAttrs(_ []slog.Attr) slog.Handler { return th }
func (th *TestHandler) WithGroup(_ string) slog.Handler { return th }
func (th *TestHandler) Handle(ctx context.Context, r slog.Record) error {
th.t.Helper()
buf := &strings.Builder{}
h := slog.NewTextHandler(buf, nil)
err := h.Handle(ctx, r)
if err != nil {
return err
}
th.t.Log(buf.String())
return nil
}
9 changes: 9 additions & 0 deletions pkg/infra/log/slogadapter/adapter.go
View file
Expand Up @@ -13,6 +13,14 @@ type slogHandler struct {
log.Logger
}

func init() {
// Lots of New's here: Default = slog.Logger <- slog.Handler <- infra/log.Logger
slog.SetDefault(slog.New(New(log.New())))
}

// Provide is a helper method to be used with Wire, however most services should use slog.Default()
func Provide() slog.Handler { return New(log.New()) }

// NewSLogHandler returns a new slog.Handler that logs to the given log.Logger.
func New(logger log.Logger) *slogHandler {
return &slogHandler{logger}
Expand All @@ -31,6 +39,7 @@ func (h *slogHandler) Handle(ctx context.Context, r slog.Record) error {
return true
}
r.Attrs(fn)
attrs = append(attrs, log.FromContext(ctx)...)

switch level := r.Level; {
case level < slog.LevelInfo:
Expand Down
2 changes: 2 additions & 0 deletions pkg/server/wire.go
View file
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/grafana/grafana/pkg/infra/httpclient/httpclientprovider"
"github.com/grafana/grafana/pkg/infra/kvstore"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log/slogadapter"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/infra/remotecache"
"github.com/grafana/grafana/pkg/infra/serverlock"
Expand Down Expand Up @@ -368,6 +369,7 @@ var wireBasicSet = wire.NewSet(
anonstore.ProvideAnonDBStore,
wire.Bind(new(anonstore.AnonStore), new(*anonstore.AnonDBStore)),
loggermw.Provide,
slogadapter.Provide,
signingkeysimpl.ProvideEmbeddedSigningKeysService,
wire.Bind(new(signingkeys.Service), new(*signingkeysimpl.Service)),
ssoSettingsImpl.ProvideService,
Expand Down
41 changes: 15 additions & 26 deletions pkg/services/folder/folderimpl/folder.go
View file
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"log/slog"
"runtime"
"strings"
"sync"
Expand All @@ -17,7 +18,6 @@ import (
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/events"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth/identity"
Expand All @@ -38,8 +38,7 @@ import (
type Service struct {
store store
db db.DB
log log.Logger
cfg *setting.Cfg
log *slog.Logger
dashboardStore dashboards.Store
dashboardFolderStore folder.FolderStore
features featuremgmt.FeatureToggles
Expand All @@ -55,18 +54,17 @@ type Service struct {
func ProvideService(
ac accesscontrol.AccessControl,
bus bus.Bus,
cfg *setting.Cfg,
_ *setting.Cfg,
dashboardStore dashboards.Store,
folderStore folder.FolderStore,
db db.DB, // DB for the (new) nested folder store
features featuremgmt.FeatureToggles,
supportBundles supportbundles.Service,
r prometheus.Registerer,
) folder.Service {
store := ProvideStore(db, cfg)
store := ProvideStore(db)
srv := &Service{
cfg: cfg,
log: log.New("folder-service"),
log: slog.Default().With("logger", "folder-service"),
dashboardStore: dashboardStore,
dashboardFolderStore: folderStore,
store: store,
Expand Down Expand Up @@ -546,8 +544,6 @@ func (s *Service) getFolderByTitle(ctx context.Context, orgID int64, title strin
}

func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) {
logger := s.log.FromContext(ctx)

if cmd.SignedInUser == nil || cmd.SignedInUser.IsNil() {
return nil, folder.ErrBadRequest.Errorf("missing signed in user")
}
Expand Down Expand Up @@ -630,7 +626,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
}

if nestedFolder, err = s.nestedFolderCreate(ctx, cmd); err != nil {
logger.Error("error saving folder to nested folder store", "error", err)
s.log.ErrorContext(ctx, "error saving folder to nested folder store", "error", err)
return err
}

Expand All @@ -648,8 +644,6 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
}

func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) {
logger := s.log.FromContext(ctx)

if cmd.SignedInUser == nil {
return nil, folder.ErrBadRequest.Errorf("missing signed in user")
}
Expand Down Expand Up @@ -683,7 +677,7 @@ func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (
UID: dashFolder.UID,
OrgID: cmd.OrgID,
}); err != nil {
logger.Error("failed to publish FolderTitleUpdated event", "folder", foldr.Title, "user", id, "namespace", namespace, "error", err)
s.log.ErrorContext(ctx, "failed to publish FolderTitleUpdated event", "folder", foldr.Title, "user", id, "namespace", namespace, "error", err)
return err
}
}
Expand All @@ -692,7 +686,7 @@ func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (
})

if err != nil {
logger.Error("folder update failed", "folderUID", cmd.UID, "error", err)
s.log.ErrorContext(ctx, "folder update failed", "folderUID", cmd.UID, "error", err)
return nil, err
}

Expand All @@ -710,8 +704,6 @@ func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (
}

func (s *Service) legacyUpdate(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) {
logger := s.log.FromContext(ctx)

query := dashboards.GetDashboardQuery{OrgID: cmd.OrgID, UID: cmd.UID}
queryResult, err := s.dashboardStore.GetDashboard(ctx, &query)
if err != nil {
Expand All @@ -736,7 +728,7 @@ func (s *Service) legacyUpdate(ctx context.Context, cmd *folder.UpdateFolderComm
if namespace == identity.NamespaceUser || namespace == identity.NamespaceServiceAccount {
userID, err = identity.IntIdentifier(namespace, id)
if err != nil {
logger.Error("failed to parse user ID", "namespace", namespace, "userID", id, "error", err)
s.log.ErrorContext(ctx, "failed to parse user ID", "namespace", namespace, "userID", id, "error", err)
}
}

Expand Down Expand Up @@ -791,7 +783,6 @@ func prepareForUpdate(dashFolder *dashboards.Dashboard, orgId int64, userId int6
}

func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) error {
logger := s.log.FromContext(ctx)
if cmd.SignedInUser == nil {
return folder.ErrBadRequest.Errorf("missing signed in user")
}
Expand Down Expand Up @@ -819,7 +810,7 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e
descendants, err := s.nestedFolderDelete(ctx, cmd)

if err != nil {
logger.Error("the delete folder on folder table failed with err: ", "error", err)
s.log.ErrorContext(ctx, "the delete folder on folder table failed with err: ", "error", err)
return err
}
folders = append(folders, descendants...)
Expand Down Expand Up @@ -962,7 +953,6 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
// its descendant folders (folders which are nested within it either directly or indirectly) from
// the folder store and returns the UIDs for all its descendants.
func (s *Service) nestedFolderDelete(ctx context.Context, cmd *folder.DeleteFolderCommand) ([]string, error) {
logger := s.log.FromContext(ctx)
descendantUIDs := []string{}
if cmd.SignedInUser == nil {
return descendantUIDs, folder.ErrBadRequest.Errorf("missing signed in user")
Expand All @@ -979,25 +969,24 @@ func (s *Service) nestedFolderDelete(ctx context.Context, cmd *folder.DeleteFold

descendants, err := s.store.GetDescendants(ctx, cmd.OrgID, cmd.UID)
if err != nil {
logger.Error("failed to get descendant folders", "error", err)
s.log.ErrorContext(ctx, "failed to get descendant folders", "error", err)
return descendantUIDs, err
}

for _, f := range descendants {
descendantUIDs = append(descendantUIDs, f.UID)
}
logger.Info("deleting folder and its descendants", "org_id", cmd.OrgID, "uid", cmd.UID)
s.log.InfoContext(ctx, "deleting folder and its descendants", "org_id", cmd.OrgID, "uid", cmd.UID)
toDelete := append(descendantUIDs, cmd.UID)
err = s.store.Delete(ctx, toDelete, cmd.OrgID)
if err != nil {
logger.Info("failed deleting folder", "org_id", cmd.OrgID, "uid", cmd.UID, "err", err)
s.log.InfoContext(ctx, "failed deleting folder", "org_id", cmd.OrgID, "uid", cmd.UID, "err", err)
return descendantUIDs, err
}
return descendantUIDs, nil
}

func (s *Service) GetDescendantCounts(ctx context.Context, q *folder.GetDescendantCountsQuery) (folder.DescendantCounts, error) {
logger := s.log.FromContext(ctx)
if q.SignedInUser == nil {
return nil, folder.ErrBadRequest.Errorf("missing signed-in user")
}
Expand All @@ -1013,7 +1002,7 @@ func (s *Service) GetDescendantCounts(ctx context.Context, q *folder.GetDescenda
if s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) {
descendantFolders, err := s.store.GetDescendants(ctx, q.OrgID, *q.UID)
if err != nil {
logger.Error("failed to get descendant folders", "error", err)
s.log.ErrorContext(ctx, "failed to get descendant folders", "error", err)
return nil, err
}
for _, f := range descendantFolders {
Expand All @@ -1025,7 +1014,7 @@ func (s *Service) GetDescendantCounts(ctx context.Context, q *folder.GetDescenda
for _, v := range s.registry {
c, err := v.CountInFolders(ctx, q.OrgID, folders, q.SignedInUser)
if err != nil {
logger.Error("failed to count folder descendants", "error", err)
s.log.ErrorContext(ctx, "failed to count folder descendants", "error", err)
return nil, err
}
countsMap[v.Kind()] = c
Expand Down

0 comments on commit 6ddb8ae

Please sign in to comment.