From def4c644ec4d5c9ad281d6ac07f3355e1b24e463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=A0tibran=C3=BD?= Date: Fri, 21 Nov 2025 14:38:09 +0100 Subject: [PATCH] Use infra/log instead of slog in pkg/storage/unified (#114292) Use infra/log instead of slog. Use log.FromContext to add contextual info to log messages. --- pkg/storage/unified/resource/access.go | 11 +++++---- pkg/storage/unified/resource/client.go | 6 ++--- .../unified/resource/grpc/authenticator.go | 4 ++-- pkg/storage/unified/resource/search.go | 24 +++++++------------ pkg/storage/unified/resource/server.go | 13 +++++----- pkg/storage/unified/resource/server_test.go | 5 ++-- pkg/storage/unified/search/bleve.go | 21 ++++++++-------- pkg/storage/unified/search/bleve_test.go | 5 ++-- 8 files changed, 40 insertions(+), 49 deletions(-) diff --git a/pkg/storage/unified/resource/access.go b/pkg/storage/unified/resource/access.go index 94cd5b4a801..58a9d8b8b76 100644 --- a/pkg/storage/unified/resource/access.go +++ b/pkg/storage/unified/resource/access.go @@ -3,7 +3,6 @@ package resource import ( "context" "fmt" - "log/slog" "sync" "time" @@ -14,6 +13,8 @@ import ( "go.opentelemetry.io/otel/trace/noop" claims "github.com/grafana/authlib/types" + + "github.com/grafana/grafana/pkg/infra/log" ) type groupResource map[string]map[string]interface{} @@ -75,7 +76,7 @@ type authzLimitedClient struct { client claims.AccessClient // allowlist is a map of group to resources that are compatible with RBAC. allowlist groupResource - logger *slog.Logger + logger log.Logger tracer trace.Tracer metrics *accessMetrics } @@ -87,7 +88,7 @@ type AuthzOptions struct { // NewAuthzLimitedClient creates a new authzLimitedClient. func NewAuthzLimitedClient(client claims.AccessClient, opts AuthzOptions) claims.AccessClient { - logger := slog.Default().With("logger", "limited-authz-client") + logger := log.New("limited-authz-client") if opts.Tracer == nil { opts.Tracer = noop.NewTracerProvider().Tracer("limited-authz-client") } @@ -147,7 +148,7 @@ func (c authzLimitedClient) Check(ctx context.Context, id claims.AuthInfo, req c } resp, err := c.client.Check(ctx, id, req, folder) if err != nil { - c.logger.Error("Check", "group", req.Group, "resource", req.Resource, "error", err, "duration", time.Since(t), "traceid", trace.SpanContextFromContext(ctx).TraceID().String()) + c.logger.FromContext(ctx).Error("Check", "group", req.Group, "resource", req.Resource, "error", err, "duration", time.Since(t)) c.metrics.errorsTotal.WithLabelValues(req.Group, req.Resource, req.Verb).Inc() span.SetStatus(codes.Error, fmt.Sprintf("check failed: %v", err)) span.RecordError(err) @@ -197,7 +198,7 @@ func (c authzLimitedClient) Compile(ctx context.Context, id claims.AuthInfo, req } checker, zookie, err := c.client.Compile(ctx, id, req) if err != nil { - c.logger.Error("Compile", "group", req.Group, "resource", req.Resource, "error", err, "traceid", trace.SpanContextFromContext(ctx).TraceID().String()) + c.logger.FromContext(ctx).Error("Compile", "group", req.Group, "resource", req.Resource, "error", err) c.metrics.errorsTotal.WithLabelValues(req.Group, req.Resource, req.Verb).Inc() span.SetStatus(codes.Error, fmt.Sprintf("compile failed: %v", err)) span.RecordError(err) diff --git a/pkg/storage/unified/resource/client.go b/pkg/storage/unified/resource/client.go index e7d43a7bbcc..b2742f71dc9 100644 --- a/pkg/storage/unified/resource/client.go +++ b/pkg/storage/unified/resource/client.go @@ -7,7 +7,6 @@ import ( "crypto/rand" "crypto/tls" "fmt" - "log/slog" "net/http" "github.com/fullstorydev/grpchan" @@ -24,6 +23,7 @@ import ( "github.com/grafana/authlib/types" "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/infra/log" authnGrpcUtils "github.com/grafana/grafana/pkg/services/authn/grpcutils" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" @@ -158,7 +158,7 @@ func NewRemoteResourceClient(tracer trace.Tracer, conn grpc.ClientConnInterface, return newResourceClient(cc, cci), nil } -var authLogger = slog.Default().With("logger", "resource-client-auth-interceptor") +var authLogger = log.New("resource-client-auth-interceptor") func idTokenExtractor(ctx context.Context) (string, error) { if identity.IsServiceIdentity(ctx) { @@ -175,7 +175,7 @@ func idTokenExtractor(ctx context.Context) (string, error) { } if !types.IsIdentityType(info.GetIdentityType(), types.TypeAccessPolicy) { - authLogger.Warn( + authLogger.FromContext(ctx).Warn( "calling resource store as the service without id token or marking it as the service identity", "subject", info.GetSubject(), "uid", info.GetUID(), diff --git a/pkg/storage/unified/resource/grpc/authenticator.go b/pkg/storage/unified/resource/grpc/authenticator.go index 270835a85ef..6cc601565b7 100644 --- a/pkg/storage/unified/resource/grpc/authenticator.go +++ b/pkg/storage/unified/resource/grpc/authenticator.go @@ -2,7 +2,6 @@ package grpc import ( "context" - "log/slog" "strconv" "google.golang.org/grpc" @@ -14,6 +13,7 @@ import ( "go.opentelemetry.io/otel/trace" "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/infra/log" ) const ( @@ -25,7 +25,7 @@ const ( mdOrgRole = "grafana-org-role" ) -var logger = slog.Default().With("logger", "legacy.grpc.Authenticator") +var logger = log.New("legacy.grpc.Authenticator") // This is in a package we can no import // var _ interceptors.Authenticator = (*Authenticator)(nil) diff --git a/pkg/storage/unified/resource/search.go b/pkg/storage/unified/resource/search.go index 2752c59650d..dce40f884cf 100644 --- a/pkg/storage/unified/resource/search.go +++ b/pkg/storage/unified/resource/search.go @@ -4,7 +4,6 @@ import ( "cmp" "context" "fmt" - "log/slog" "slices" "strings" "sync" @@ -23,6 +22,7 @@ import ( dashboardv1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/storage/unified/resourcepb" "github.com/grafana/grafana/pkg/util/debouncer" @@ -129,7 +129,7 @@ type SearchBackend interface { // This supports indexing+search regardless of implementation type searchSupport struct { - log *slog.Logger + log log.Logger storage StorageBackend search SearchBackend indexMetrics *BleveIndexMetrics @@ -184,7 +184,7 @@ func newSearchSupport(opts SearchOptions, storage StorageBackend, access types.A access: access, storage: storage, search: opts.Backend, - log: slog.Default().With("logger", "resource-search"), + log: log.New("resource-search"), initWorkers: opts.InitWorkerThreads, rebuildWorkers: opts.IndexRebuildWorkers, initMinSize: opts.InitMinCount, @@ -295,14 +295,6 @@ func (s *searchSupport) ListManagedObjects(ctx context.Context, req *resourcepb. return rsp, nil } -func (s *searchSupport) logWithTraceID(ctx context.Context) *slog.Logger { - l := s.log - if traceID := tracing.TraceIDFromContext(ctx, false); traceID != "" { - l = l.With("traceID", traceID) - } - return l -} - func (s *searchSupport) logStats(ctx context.Context, stats *SearchStats, span trace.Span, params ...any) { elapsed := time.Since(stats.startTime) @@ -319,7 +311,7 @@ func (s *searchSupport) logStats(ctx context.Context, stats *SearchStats, span t } args = append(args, params...) - s.logWithTraceID(ctx).Debug("Search stats", args...) + s.log.FromContext(ctx).Debug("Search stats", args...) if span != nil { attrs := make([]attribute.KeyValue, 0, len(args)/2) @@ -696,7 +688,7 @@ func (s *searchSupport) rebuildIndex(ctx context.Context, req rebuildRequest) { ctx, span := tracer.Start(ctx, "resource.searchSupport.rebuildIndex") defer span.End() - l := s.log.With("namespace", req.Namespace, "group", req.Group, "resource", req.Resource) + l := s.log.New("namespace", req.Namespace, "group", req.Group, "resource", req.Resource) idx := s.search.GetIndex(req.NamespacedResource) if idx == nil { @@ -752,7 +744,7 @@ func (s *searchSupport) rebuildIndex(ctx context.Context, req rebuildRequest) { } } -func shouldRebuildIndex(buildInfo IndexBuildInfo, minBuildVersion *semver.Version, minBuildTime time.Time, lastImportTime time.Time, rebuildLogger *slog.Logger) bool { +func shouldRebuildIndex(buildInfo IndexBuildInfo, minBuildVersion *semver.Version, minBuildTime time.Time, lastImportTime time.Time, rebuildLogger log.Logger) bool { if !minBuildTime.IsZero() { if buildInfo.BuildTime.IsZero() || buildInfo.BuildTime.Before(minBuildTime) { if rebuildLogger != nil { @@ -873,7 +865,7 @@ func (s *searchSupport) getOrCreateIndex(ctx context.Context, stats *SearchStats if s.indexMetrics != nil { s.indexMetrics.SearchUpdateWaitTime.WithLabelValues(reason).Observe(elapsed.Seconds()) } - s.logWithTraceID(ctx).Debug("Index updated before search", "namespace", key.Namespace, "group", key.Group, "resource", key.Resource, "reason", reason, "duration", elapsed, "rv", rv) + s.log.FromContext(ctx).Debug("Index updated before search", "namespace", key.Namespace, "group", key.Group, "resource", key.Resource, "reason", reason, "duration", elapsed, "rv", rv) span.AddEvent("Index updated") return idx, nil @@ -890,7 +882,7 @@ func (s *searchSupport) build(ctx context.Context, nsr NamespacedResource, size attribute.Int64("size", size), ) - logger := s.logWithTraceID(ctx).With("namespace", nsr.Namespace, "group", nsr.Group, "resource", nsr.Resource) + logger := s.log.New("namespace", nsr.Namespace, "group", nsr.Group, "resource", nsr.Resource) builder, err := s.builders.get(ctx, nsr) if err != nil { diff --git a/pkg/storage/unified/resource/server.go b/pkg/storage/unified/resource/server.go index bf805cc0a6a..ca9f87896e1 100644 --- a/pkg/storage/unified/resource/server.go +++ b/pkg/storage/unified/resource/server.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "iter" - "log/slog" "net/http" "sync" "sync/atomic" @@ -24,6 +23,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/apimachinery/validation" + "github.com/grafana/grafana/pkg/infra/log" secrets "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/storage/unified/resourcepb" "github.com/grafana/grafana/pkg/util/scheduler" @@ -320,7 +320,7 @@ func NewResourceServer(opts ResourceServerOptions) (*server, error) { } } - logger := slog.Default().With("logger", "resource-server") + logger := log.New("resource-server") // Make this cancelable ctx, cancel := context.WithCancel(context.Background()) @@ -366,7 +366,7 @@ func NewResourceServer(opts ResourceServerOptions) (*server, error) { var _ ResourceServer = &server{} type server struct { - log *slog.Logger + log log.Logger backend StorageBackend blob BlobSupport secure secrets.InlineSecureValueSupport @@ -470,14 +470,15 @@ func (s *server) newEvent(ctx context.Context, user claims.AuthInfo, key *resour return nil, AsErrorResult(err) } + l := s.log.FromContext(ctx) if obj.GetUID() == "" { // TODO! once https://github.com/grafana/grafana/pull/96086 is deployed everywhere // return nil, NewBadRequestError("object is missing UID") - s.log.Error("object is missing UID", "key", key) + l.Error("object is missing UID", "key", key) } if obj.GetResourceVersion() != "" { - s.log.Error("object must not include a resource version", "key", key) + l.Error("object must not include a resource version", "key", key) } // Make sure the command labels are not saved @@ -694,7 +695,7 @@ func (s *server) create(ctx context.Context, user claims.AuthInfo, req *resource if err != nil { rsp.Error = AsErrorResult(err) } - s.log.Debug("server.WriteEvent", "type", event.Type, "rv", rsp.ResourceVersion, "previousRV", event.PreviousRV, "group", event.Key.Group, "namespace", event.Key.Namespace, "name", event.Key.Name, "resource", event.Key.Resource) + s.log.FromContext(ctx).Debug("server.WriteEvent", "type", event.Type, "rv", rsp.ResourceVersion, "previousRV", event.PreviousRV, "group", event.Key.Group, "namespace", event.Key.Namespace, "name", event.Key.Name, "resource", event.Key.Resource) return rsp, nil } diff --git a/pkg/storage/unified/resource/server_test.go b/pkg/storage/unified/resource/server_test.go index 38df20e8151..504c49616f9 100644 --- a/pkg/storage/unified/resource/server_test.go +++ b/pkg/storage/unified/resource/server_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "log/slog" "net/http" "strings" "sync" @@ -581,7 +580,7 @@ func newTestServerWithQueue(t *testing.T, maxSizePerTenant int, numWorkers int) MaxRetries: 2, MinBackoff: 10 * time.Millisecond, }, - log: slog.Default(), + log: log.NewNopLogger(), } return s, q } @@ -589,7 +588,7 @@ func newTestServerWithQueue(t *testing.T, maxSizePerTenant int, numWorkers int) func TestArtificialDelayAfterSuccessfulOperation(t *testing.T) { s := &server{ artificialSuccessfulWriteDelay: 1 * time.Millisecond, - log: slog.Default(), + log: log.NewNopLogger(), } check := func(t *testing.T, expectedSleep bool, res responseWithErrorResult, err error) { diff --git a/pkg/storage/unified/search/bleve.go b/pkg/storage/unified/search/bleve.go index 836c491b3c7..36c68183203 100644 --- a/pkg/storage/unified/search/bleve.go +++ b/pkg/storage/unified/search/bleve.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "log/slog" "math" "os" "path/filepath" @@ -71,7 +70,7 @@ type BleveOptions struct { BuildVersion string - Logger *slog.Logger + Logger log.Logger // Minimum time between index updates. IndexMinUpdateInterval time.Duration @@ -84,7 +83,7 @@ type BleveOptions struct { type bleveBackend struct { tracer trace.Tracer - log *slog.Logger + log log.Logger opts BleveOptions // set from opts.OwnsIndex, always non-nil @@ -125,9 +124,9 @@ func NewBleveBackend(opts BleveOptions, tracer trace.Tracer, indexMetrics *resou } } - log := opts.Logger - if log == nil { - log = slog.Default().With("logger", "bleve-backend") + l := opts.Logger + if l == nil { + l = log.New("bleve-backend") } ownFn := opts.OwnsIndex @@ -137,7 +136,7 @@ func NewBleveBackend(opts BleveOptions, tracer trace.Tracer, indexMetrics *resou } be := &bleveBackend{ - log: log, + log: l, tracer: tracer, cache: map[resource.NamespacedResource]*bleveIndex{}, opts: opts, @@ -381,7 +380,7 @@ func (b *bleveBackend) BuildIndex( return nil, err } - logWithDetails := b.log.With("namespace", key.Namespace, "group", key.Group, "resource", key.Resource, "size", size, "reason", indexBuildReason) + logWithDetails := b.log.FromContext(ctx).New("namespace", key.Namespace, "group", key.Group, "resource", key.Resource, "size", size, "reason", indexBuildReason) // Close the newly created/opened index by default. closeIndex := true @@ -462,7 +461,7 @@ func (b *bleveBackend) BuildIndex( } // Batch all the changes - idx := b.newBleveIndex(key, index, newIndexType, fields, allFields, standardSearchFields, updater, b.log.With("namespace", key.Namespace, "group", key.Group, "resource", key.Resource)) + idx := b.newBleveIndex(key, index, newIndexType, fields, allFields, standardSearchFields, updater, b.log.New("namespace", key.Namespace, "group", key.Group, "resource", key.Resource)) if build { if b.indexMetrics != nil { @@ -714,7 +713,7 @@ type bleveIndex struct { // The values returned with all allFields []*resourcepb.ResourceTableColumnDefinition tracing trace.Tracer - logger *slog.Logger + logger log.Logger updaterFn resource.UpdateFn minUpdateInterval time.Duration @@ -741,7 +740,7 @@ func (b *bleveBackend) newBleveIndex( allFields []*resourcepb.ResourceTableColumnDefinition, standardSearchFields resource.SearchableDocumentFields, updaterFn resource.UpdateFn, - logger *slog.Logger, + logger log.Logger, ) *bleveIndex { bi := &bleveIndex{ key: key, diff --git a/pkg/storage/unified/search/bleve_test.go b/pkg/storage/unified/search/bleve_test.go index b303d11c6e8..4c66fc4c1b5 100644 --- a/pkg/storage/unified/search/bleve_test.go +++ b/pkg/storage/unified/search/bleve_test.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "log/slog" "math" "os" "path/filepath" @@ -26,7 +25,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" - "github.com/grafana/grafana/pkg/infra/log/logtest" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" "github.com/grafana/grafana/pkg/services/store/kind/dashboard" @@ -772,7 +771,7 @@ func setupBleveBackend(t *testing.T, options ...setupOption) (*bleveBackend, pro opts := BleveOptions{ FileThreshold: defaultFileThreshold, IndexCacheTTL: defaultIndexCacheTTL, - Logger: slog.New(logtest.NewNopHandler(t)), + Logger: log.NewNopLogger(), BuildVersion: buildVersion, } for _, opt := range options {