From 1a216a8fffaa2e123dc3a5bcc8a46adb2958bfd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20Qu=C3=A9m=C3=A9ner?= Date: Tue, 10 Jun 2025 15:33:14 +0200 Subject: [PATCH] feat(unified-storage): use logger from context for dualwriter (#106473) --- pkg/storage/legacysql/dualwrite/dualwriter.go | 17 ++++++++++------- pkg/storage/legacysql/dualwrite/runtime.go | 10 +++------- pkg/storage/legacysql/dualwrite/static.go | 9 +++------ 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/pkg/storage/legacysql/dualwrite/dualwriter.go b/pkg/storage/legacysql/dualwrite/dualwriter.go index 0c23cd8e02e..8cda0f6191d 100644 --- a/pkg/storage/legacysql/dualwrite/dualwriter.go +++ b/pkg/storage/legacysql/dualwrite/dualwriter.go @@ -13,6 +13,7 @@ import ( "k8s.io/apiserver/pkg/registry/rest" "github.com/grafana/grafana-app-sdk/logging" + "github.com/grafana/grafana/pkg/apimachinery/utils" grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" ) @@ -33,7 +34,6 @@ type dualWriter struct { unified grafanarest.Storage readUnified bool errorIsOK bool // in "mode1" we try writing both -- but don't block on unified write errors - log logging.Logger } func (d *dualWriter) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { @@ -52,7 +52,8 @@ func (d *dualWriter) Get(ctx context.Context, name string, options *metav1.GetOp go func(ctxBg context.Context, cancel context.CancelFunc) { defer cancel() if _, err := d.unified.Get(ctxBg, name, options); err != nil { - d.log.Error("failed background GET to unified", "err", err) + log := logging.FromContext(ctxBg).With("method", "Get") + log.Error("failed background GET to unified", "err", err) } }(context.WithTimeout(context.WithoutCancel(ctx), backgroundReqTimeout)) return legacyGet, nil @@ -81,7 +82,8 @@ func (d *dualWriter) List(ctx context.Context, options *metainternalversion.List go func(ctxBg context.Context, cancel context.CancelFunc) { defer cancel() if _, err := d.unified.List(ctxBg, options); err != nil { - d.log.Error("failed background LIST to unified", "err", err) + log := logging.FromContext(ctxBg).With("method", "List") + log.Error("failed background LIST to unified", "err", err) } }(context.WithTimeout(context.WithoutCancel(ctx), backgroundReqTimeout)) return legacyList, nil @@ -95,7 +97,7 @@ func (d *dualWriter) List(ctx context.Context, options *metainternalversion.List // Create overrides the behavior of the generic DualWriter and writes to LegacyStorage and Storage. func (d *dualWriter) Create(ctx context.Context, in runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { - log := d.log.With("method", "Create").WithContext(ctx) + log := logging.FromContext(ctx).With("method", "Create") accIn, err := meta.Accessor(in) if err != nil { @@ -190,7 +192,8 @@ func (d *dualWriter) Delete(ctx context.Context, name string, deleteValidation r defer cancel() _, _, err := d.unified.Delete(ctxBg, name, deleteValidation, options) if err != nil && !apierrors.IsNotFound(err) && !d.errorIsOK { - d.log.Error("failed background DELETE in unified storage", "err", err) + log := logging.FromContext(ctxBg).With("method", "Delete") + log.Error("failed background DELETE in unified storage", "err", err) } }(context.WithTimeout(context.WithoutCancel(ctx), backgroundReqTimeout)) } @@ -204,7 +207,7 @@ func (d *dualWriter) Delete(ctx context.Context, name string, deleteValidation r // Update overrides the behavior of the generic DualWriter and writes first to Storage and then to LegacyStorage. func (d *dualWriter) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { - log := d.log.With("method", "Update").WithContext(ctx) + log := logging.FromContext(ctx).With("method", "Update") // update in legacy first, and then unistore. Will return a failure if either fails. // @@ -251,7 +254,7 @@ func (d *dualWriter) Update(ctx context.Context, name string, objInfo rest.Updat // DeleteCollection overrides the behavior of the generic DualWriter and deletes from both LegacyStorage and Storage. func (d *dualWriter) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { - log := d.log.With("method", "DeleteCollection", "resourceVersion", listOptions.ResourceVersion).WithContext(ctx) + log := logging.FromContext(ctx).With("method", "DeleteCollection", "resourceVersion", listOptions.ResourceVersion) // delete from legacy first, and anything that is successful can be deleted in unistore too. // diff --git a/pkg/storage/legacysql/dualwrite/runtime.go b/pkg/storage/legacysql/dualwrite/runtime.go index 8d9d5eb872d..180beeb255a 100644 --- a/pkg/storage/legacysql/dualwrite/runtime.go +++ b/pkg/storage/legacysql/dualwrite/runtime.go @@ -11,8 +11,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/registry/rest" - "github.com/grafana/grafana-app-sdk/logging" - grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" ) @@ -22,15 +20,13 @@ func (m *service) NewStorage(gr schema.GroupResource, legacy grafanarest.Storage return nil, err } - log := logging.DefaultLogger.With("gr", gr.String()) - if m.enabled && status.Runtime { // Dynamic storage behavior return &runtimeDualWriter{ service: m, legacy: legacy, unified: unified, - dualwrite: &dualWriter{legacy: legacy, unified: unified, log: log}, // not used for read + dualwrite: &dualWriter{legacy: legacy, unified: unified}, // not used for read gr: gr, }, nil } @@ -38,13 +34,13 @@ func (m *service) NewStorage(gr schema.GroupResource, legacy grafanarest.Storage if status.ReadUnified { if status.WriteLegacy { // Write both, read unified - return &dualWriter{legacy: legacy, unified: unified, log: log, readUnified: true}, nil + return &dualWriter{legacy: legacy, unified: unified, readUnified: true}, nil } return unified, nil } if status.WriteUnified { // Write both, read legacy - return &dualWriter{legacy: legacy, unified: unified, log: log}, nil + return &dualWriter{legacy: legacy, unified: unified}, nil } return legacy, nil } diff --git a/pkg/storage/legacysql/dualwrite/static.go b/pkg/storage/legacysql/dualwrite/static.go index db71d338377..3835cf80752 100644 --- a/pkg/storage/legacysql/dualwrite/static.go +++ b/pkg/storage/legacysql/dualwrite/static.go @@ -6,7 +6,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/grafana/grafana-app-sdk/logging" "github.com/grafana/grafana/pkg/apiserver/rest" "github.com/grafana/grafana/pkg/setting" ) @@ -41,16 +40,14 @@ func (m *staticService) SetMode(gr schema.GroupResource, mode rest.DualWriterMod } func (m *staticService) NewStorage(gr schema.GroupResource, legacy rest.Storage, unified rest.Storage) (rest.Storage, error) { - log := logging.DefaultLogger.With("dualwrite", gr.String()) - config := m.cfg.UnifiedStorage[gr.String()] switch config.DualWriterMode { case rest.Mode1: - return &dualWriter{log: log, legacy: legacy, unified: unified, errorIsOK: true}, nil + return &dualWriter{legacy: legacy, unified: unified, errorIsOK: true}, nil case rest.Mode2: - return &dualWriter{log: log, legacy: legacy, unified: unified}, nil + return &dualWriter{legacy: legacy, unified: unified}, nil case rest.Mode3: - return &dualWriter{log: log, legacy: legacy, unified: unified, readUnified: true}, nil + return &dualWriter{legacy: legacy, unified: unified, readUnified: true}, nil case rest.Mode4, rest.Mode5: return unified, nil // use unified directly case rest.Mode0: