From fea972cb11496a14534272baef9aa6bbc8c5f5a3 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 16 Dec 2025 14:11:46 +0300 Subject: [PATCH] Dashboards: Avoid infra/log in apps (#115396) --- .golangci.yml | 20 +++++++++---- apps/dashboard/go.mod | 2 +- .../pkg/migration/schemaversion/cache.go | 28 ++++++++----------- .../pkg/migration/schemaversion/cache_test.go | 24 ++++++++-------- .../schemaversion/datasource_utils.go | 6 ++-- 5 files changed, 42 insertions(+), 38 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3b7871662e2..b52a986d435 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,7 +3,7 @@ # Others can set up the YAML LSP manually, which supports schemas: https://github.com/redhat-developer/yaml-language-server # $schema: https://golangci-lint.run/jsonschema/golangci.jsonschema.json -version: "2" +version: '2' run: timeout: 15m concurrency: 10 @@ -83,6 +83,16 @@ linters: deny: - pkg: github.com/grafana/grafana/pkg desc: apps/playlist is not allowed to import grafana core + apps-dashboard: + list-mode: lax + files: + - ./apps/dashboard/* + - ./apps/dashboard/**/* + allow: + - github.com/grafana/grafana/pkg/apimachinery + deny: + - pkg: github.com/grafana/grafana/pkg + desc: apps/dashboard is not allowed to import grafana core apps-secret: list-mode: lax files: @@ -281,16 +291,16 @@ linters: text: G306 - linters: - gosec - text: "401" + text: '401' - linters: - gosec - text: "402" + text: '402' - linters: - gosec - text: "501" + text: '501' - linters: - gosec - text: "404" + text: '404' - linters: - errorlint text: non-wrapping format verb for fmt.Errorf diff --git a/apps/dashboard/go.mod b/apps/dashboard/go.mod index 2182e0af949..00fe99f0c9c 100644 --- a/apps/dashboard/go.mod +++ b/apps/dashboard/go.mod @@ -9,6 +9,7 @@ require ( github.com/grafana/grafana-app-sdk/logging v0.48.3 github.com/grafana/grafana-plugin-sdk-go v0.284.0 github.com/grafana/grafana/pkg/apimachinery v0.0.0-20250514132646-acbc7b54ed9e + github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/prometheus/client_golang v1.23.2 github.com/stretchr/testify v1.11.1 k8s.io/apimachinery v0.34.2 @@ -57,7 +58,6 @@ require ( github.com/hashicorp/go-hclog v1.6.3 // indirect github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/go-plugin v1.7.0 // indirect - github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect github.com/hashicorp/yamux v0.1.2 // indirect github.com/jaegertracing/jaeger-idl v0.5.0 // indirect github.com/josharian/intern v1.0.0 // indirect diff --git a/apps/dashboard/pkg/migration/schemaversion/cache.go b/apps/dashboard/pkg/migration/schemaversion/cache.go index f31eaa14e1a..2548c2cc5ba 100644 --- a/apps/dashboard/pkg/migration/schemaversion/cache.go +++ b/apps/dashboard/pkg/migration/schemaversion/cache.go @@ -5,12 +5,11 @@ import ( "sync" "time" - "github.com/grafana/authlib/types" - "github.com/grafana/grafana/pkg/infra/log" "github.com/hashicorp/golang-lru/v2/expirable" - k8srequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/endpoints/request" - "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" + "github.com/grafana/authlib/types" + "github.com/grafana/grafana-app-sdk/logging" ) const defaultCacheSize = 1000 @@ -32,17 +31,15 @@ type cachedProvider[T any] struct { fetch func(context.Context) T cache *expirable.LRU[string, T] // LRU cache: namespace to cache entry inFlight sync.Map // map[string]*sync.Mutex - per-namespace fetch locks - logger log.Logger } // newCachedProvider creates a new cachedProvider. // The fetch function should be able to handle context with different namespaces. // A non-positive size turns LRU mechanism off (cache of unlimited size). // A non-positive cacheTTL disables TTL expiration. -func newCachedProvider[T any](fetch func(context.Context) T, size int, cacheTTL time.Duration, logger log.Logger) *cachedProvider[T] { +func newCachedProvider[T any](fetch func(context.Context) T, size int, cacheTTL time.Duration) *cachedProvider[T] { cacheProvider := &cachedProvider[T]{ - fetch: fetch, - logger: logger, + fetch: fetch, } cacheProvider.cache = expirable.NewLRU(size, func(key string, value T) { cacheProvider.inFlight.Delete(key) @@ -53,14 +50,13 @@ func newCachedProvider[T any](fetch func(context.Context) T, size int, cacheTTL // Get returns the cached value if it's still valid, otherwise calls fetch and caches the result. func (p *cachedProvider[T]) Get(ctx context.Context) T { // Get namespace info from ctx - nsInfo, err := request.NamespaceInfoFrom(ctx, true) - if err != nil { + namespace, ok := request.NamespaceFrom(ctx) + if !ok { // No namespace, fall back to direct fetch call without caching - p.logger.Warn("Unable to get namespace info from context, skipping cache", "error", err) + logging.FromContext(ctx).Warn("Unable to get namespace info from context, skipping cache") return p.fetch(ctx) } - namespace := nsInfo.Value // Fast path: check if cache is still valid if entry, ok := p.cache.Get(namespace); ok { return entry @@ -81,7 +77,7 @@ func (p *cachedProvider[T]) Get(ctx context.Context) T { } // Fetch outside the main lock - only this namespace is blocked - p.logger.Debug("cache miss or expired, fetching new value", "namespace", namespace) + logging.FromContext(ctx).Debug("cache miss or expired, fetching new value", "namespace", namespace) value := p.fetch(ctx) // Update the cache for this namespace @@ -93,12 +89,12 @@ func (p *cachedProvider[T]) Get(ctx context.Context) T { // Preload loads data into the cache for the given namespaces. func (p *cachedProvider[T]) Preload(ctx context.Context, nsInfos []types.NamespaceInfo) { // Build the cache using a context with the namespace - p.logger.Info("preloading cache", "nsInfos", len(nsInfos)) + logging.FromContext(ctx).Info("preloading cache", "nsInfos", len(nsInfos)) startedAt := time.Now() defer func() { - p.logger.Info("finished preloading cache", "nsInfos", len(nsInfos), "elapsed", time.Since(startedAt)) + logging.FromContext(ctx).Info("finished preloading cache", "nsInfos", len(nsInfos), "elapsed", time.Since(startedAt)) }() for _, nsInfo := range nsInfos { - p.cache.Add(nsInfo.Value, p.fetch(k8srequest.WithNamespace(ctx, nsInfo.Value))) + p.cache.Add(nsInfo.Value, p.fetch(request.WithNamespace(ctx, nsInfo.Value))) } } diff --git a/apps/dashboard/pkg/migration/schemaversion/cache_test.go b/apps/dashboard/pkg/migration/schemaversion/cache_test.go index f044ab0f813..081455143e4 100644 --- a/apps/dashboard/pkg/migration/schemaversion/cache_test.go +++ b/apps/dashboard/pkg/migration/schemaversion/cache_test.go @@ -8,11 +8,11 @@ import ( "testing" "time" - authlib "github.com/grafana/authlib/types" - "github.com/grafana/grafana/pkg/infra/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apiserver/pkg/endpoints/request" + + authlib "github.com/grafana/authlib/types" ) // testProvider tracks how many times get() is called @@ -44,7 +44,7 @@ func TestCachedProvider_CacheHit(t *testing.T) { underlying := newTestProvider(datasources) // Test newCachedProvider directly instead of the wrapper - cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute, log.New("test")) + cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute) // Use "default" namespace (org 1) - this is the standard Grafana namespace format ctx := request.WithNamespace(context.Background(), "default") @@ -69,7 +69,7 @@ func TestCachedProvider_NamespaceIsolation(t *testing.T) { } underlying := newTestProvider(datasources) - cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute, log.New("test")) + cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute) // Use "default" (org 1) and "org-2" (org 2) - standard Grafana namespace formats ctx1 := request.WithNamespace(context.Background(), "default") @@ -102,7 +102,7 @@ func TestCachedProvider_NoNamespaceFallback(t *testing.T) { } underlying := newTestProvider(datasources) - cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute, log.New("test")) + cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute) // Context without namespace - should fall back to direct provider call ctx := context.Background() @@ -123,7 +123,7 @@ func TestCachedProvider_ConcurrentAccess(t *testing.T) { } underlying := newTestProvider(datasources) - cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute, log.New("test")) + cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute) // Use "default" namespace (org 1) ctx := request.WithNamespace(context.Background(), "default") @@ -155,7 +155,7 @@ func TestCachedProvider_ConcurrentNamespaces(t *testing.T) { } underlying := newTestProvider(datasources) - cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute, log.New("test")) + cached := newCachedProvider(underlying.get, defaultCacheSize, time.Minute) var wg sync.WaitGroup numOrgs := 10 @@ -198,7 +198,7 @@ func TestCachedProvider_CorrectDataPerNamespace(t *testing.T) { "org-2": {{UID: "org2-ds", Type: "loki", Name: "Org2 DS", Default: true}}, }, } - cached := newCachedProvider(underlying.Index, defaultCacheSize, time.Minute, log.New("test")) + cached := newCachedProvider(underlying.Index, defaultCacheSize, time.Minute) // Use valid namespace formats ctx1 := request.WithNamespace(context.Background(), "default") @@ -228,7 +228,7 @@ func TestCachedProvider_PreloadMultipleNamespaces(t *testing.T) { "org-3": {{UID: "org3-ds", Type: "tempo", Name: "Org3 DS", Default: true}}, }, } - cached := newCachedProvider(underlying.Index, defaultCacheSize, time.Minute, log.New("test")) + cached := newCachedProvider(underlying.Index, defaultCacheSize, time.Minute) // Preload multiple namespaces nsInfos := []authlib.NamespaceInfo{ @@ -346,7 +346,7 @@ func TestCachedProvider_TTLExpiration(t *testing.T) { underlying := newTestProvider(datasources) // Use a very short TTL for testing shortTTL := 50 * time.Millisecond - cached := newCachedProvider(underlying.get, defaultCacheSize, shortTTL, log.New("test")) + cached := newCachedProvider(underlying.get, defaultCacheSize, shortTTL) ctx := request.WithNamespace(context.Background(), "default") @@ -379,7 +379,7 @@ func TestCachedProvider_ParallelNamespacesFetch(t *testing.T) { {UID: "ds1", Type: "prometheus", Name: "Prometheus", Default: true}, }, } - cached := newCachedProvider(provider.get, defaultCacheSize, time.Minute, log.New("test")) + cached := newCachedProvider(provider.get, defaultCacheSize, time.Minute) numNamespaces := 5 var wg sync.WaitGroup @@ -421,7 +421,7 @@ func TestCachedProvider_SameNamespaceSerialFetch(t *testing.T) { {UID: "ds1", Type: "prometheus", Name: "Prometheus", Default: true}, }, } - cached := newCachedProvider(provider.get, defaultCacheSize, time.Minute, log.New("test")) + cached := newCachedProvider(provider.get, defaultCacheSize, time.Minute) numGoroutines := 10 var wg sync.WaitGroup diff --git a/apps/dashboard/pkg/migration/schemaversion/datasource_utils.go b/apps/dashboard/pkg/migration/schemaversion/datasource_utils.go index 9deee29b414..c7215495dc9 100644 --- a/apps/dashboard/pkg/migration/schemaversion/datasource_utils.go +++ b/apps/dashboard/pkg/migration/schemaversion/datasource_utils.go @@ -3,8 +3,6 @@ package schemaversion import ( "context" "time" - - "github.com/grafana/grafana/pkg/infra/log" ) // Shared utility functions for datasource migrations across different schema versions. @@ -36,7 +34,7 @@ func WrapIndexProviderWithCache(provider DataSourceIndexProvider, cacheTTL time. return provider } return &cachedIndexProvider{ - newCachedProvider[*DatasourceIndex](provider.Index, defaultCacheSize, cacheTTL, log.New("schemaversion.dsindexprovider")), + newCachedProvider[*DatasourceIndex](provider.Index, defaultCacheSize, cacheTTL), } } @@ -46,7 +44,7 @@ func WrapLibraryElementProviderWithCache(provider LibraryElementIndexProvider, c return provider } return &cachedLibraryElementProvider{ - newCachedProvider[[]LibraryElementInfo](provider.GetLibraryElementInfo, defaultCacheSize, cacheTTL, log.New("schemaversion.leindexprovider")), + newCachedProvider[[]LibraryElementInfo](provider.GetLibraryElementInfo, defaultCacheSize, cacheTTL), } }