diff --git a/pkg/registry/apis/provisioning/auth/identity.go b/pkg/registry/apis/provisioning/auth/identity.go index bc3ce417830..2f16f916257 100644 --- a/pkg/registry/apis/provisioning/auth/identity.go +++ b/pkg/registry/apis/provisioning/auth/identity.go @@ -3,7 +3,6 @@ package auth import ( "errors" "fmt" - "log/slog" "sync" "time" @@ -18,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/slogctx" ) type BackgroundIdentityService interface { @@ -43,7 +43,6 @@ func ProvideProvisioningIdentityService( role: org.RoleAdmin, clientConfigProvider: clientConfigProvider, - log: slog.Default().With("logger", "background-identities", "prefix", prefix), accounts: make(map[int64]string), serviceAccounts: serviceAccounts, authn: authn, @@ -53,8 +52,6 @@ func ProvideProvisioningIdentityService( } type backgroundIdentities struct { - log *slog.Logger - serviceAccountNamePrefix string role org.RoleType clientConfigProvider apiserver.DirectRestConfigProvider @@ -158,6 +155,7 @@ func (o *backgroundIdentities) makeAdminUser(ctx context.Context, orgId int64) ( func (o *backgroundIdentities) verifyServiceAccount(ctx context.Context, orgId int64) (string, error) { serviceAccountName := fmt.Sprintf("%s-org-%d", o.serviceAccountNamePrefix, orgId) + logger := slogctx.From(ctx).With("account_name", serviceAccountName) saForm := serviceaccounts.CreateServiceAccountForm{ Name: serviceAccountName, Role: &o.role, @@ -169,7 +167,7 @@ func (o *backgroundIdentities) verifyServiceAccount(ctx context.Context, orgId i if accountAlreadyExists { accountId, err := o.serviceAccounts.RetrieveServiceAccountIdByName(ctx, orgId, serviceAccountName) if err != nil { - o.log.Error("Failed to retrieve service account", "err", err, "accountName", serviceAccountName) + logger.ErrorContext(ctx, "Failed to retrieve service account", "err", err) return "", err } // update org_role to make sure everything works properly if someone has changed the role since SA's original creation @@ -178,7 +176,7 @@ func (o *backgroundIdentities) verifyServiceAccount(ctx context.Context, orgId i Role: &o.role, }) if err != nil { - o.log.Error("Failed to update service account", "err", err, "accountName", serviceAccountName) + logger.ErrorContext(ctx, "Failed to update service account", "err", err) return "", err } @@ -189,7 +187,7 @@ func (o *backgroundIdentities) verifyServiceAccount(ctx context.Context, orgId i } } if serviceAccount == nil { - o.log.Error("Failed to retrieve service account", "err", err, "accountName", serviceAccountName) + logger.ErrorContext(ctx, "Failed to retrieve service account", "err", err) return "", err } diff --git a/pkg/registry/apis/provisioning/export.go b/pkg/registry/apis/provisioning/export.go index e2e8aa6c72c..3076a287149 100644 --- a/pkg/registry/apis/provisioning/export.go +++ b/pkg/registry/apis/provisioning/export.go @@ -3,7 +3,6 @@ package provisioning import ( "context" "fmt" - "log/slog" "net/http" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -13,11 +12,11 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" + "github.com/grafana/grafana/pkg/slogctx" ) type exportConnector struct { repoGetter RepoGetter - logger *slog.Logger queue jobs.JobQueue } @@ -63,12 +62,14 @@ func (c *exportConnector) Connect( return nil, err } ns := repo.Config().GetNamespace() - logger := c.logger.With("repository", name, "namespace", ns) + logger := slogctx.From(ctx).With("logger", "export-connector", "repository", name, "namespace", ns) // TODO: We need some way to filter what we export. return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - job, err := c.queue.Add(r.Context(), &provisioning.Job{ + ctx := slogctx.To(r.Context(), logger) + + job, err := c.queue.Add(ctx, &provisioning.Job{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, Labels: map[string]string{ diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index cf9b49ba0fd..d5dd4eb1fec 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "log/slog" "net/http" "path" "strings" @@ -18,12 +17,12 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" + "github.com/grafana/grafana/pkg/slogctx" ) type filesConnector struct { getter RepoGetter parsers *resources.ParserFactory - logger *slog.Logger } func (*filesConnector) New() runtime.Object { @@ -58,7 +57,8 @@ func (*filesConnector) NewConnectOptions() (runtime.Object, bool, string) { } func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { - logger := s.logger.With("repository_name", name) + logger := slogctx.From(ctx).With("logger", "files-connector", "repository_name", name) + ctx = slogctx.To(ctx, logger) repo, err := s.getter.GetRepository(ctx, name) if err != nil { logger.DebugContext(ctx, "failed to find repository", "error", err) @@ -69,7 +69,8 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. query := r.URL.Query() ref := query.Get("ref") message := query.Get("message") - logger = logger.With("url", r.URL.Path, "ref", ref, "message", message) + logger := logger.With("url", r.URL.Path, "ref", ref, "message", message) + ctx := slogctx.To(r.Context(), logger) prefix := fmt.Sprintf("/%s/files", name) idx := strings.Index(r.URL.Path, prefix) @@ -86,7 +87,7 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. return } - rsp, err := repo.ReadTree(r.Context(), logger, ref) + rsp, err := repo.ReadTree(ctx, ref) if err != nil { responder.Error(err) return @@ -111,7 +112,7 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. case ".json", ".yaml", ".yml": // ok default: - logger.DebugContext(r.Context(), "got a file extension that was not JSON or YAML", "extension", path.Ext(filePath)) + logger.DebugContext(ctx, "got a file extension that was not JSON or YAML", "extension", path.Ext(filePath)) responder.Error(apierrors.NewBadRequest("only yaml and json files supported")) return } @@ -120,13 +121,13 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. code := http.StatusOK switch r.Method { case http.MethodGet: - code, obj, err = s.doRead(r.Context(), logger, repo, filePath, ref) + code, obj, err = s.doRead(ctx, repo, filePath, ref) case http.MethodPost: - obj, err = s.doWrite(r.Context(), logger, false, repo, filePath, ref, message, r) + obj, err = s.doWrite(ctx, false, repo, filePath, ref, message, r) case http.MethodPut: - obj, err = s.doWrite(r.Context(), logger, true, repo, filePath, ref, message, r) + obj, err = s.doWrite(ctx, true, repo, filePath, ref, message, r) case http.MethodDelete: - obj, err = s.doDelete(r.Context(), logger, repo, filePath, ref, message) + obj, err = s.doDelete(ctx, repo, filePath, ref, message) default: err = apierrors.NewMethodNotSupported(provisioning.RepositoryResourceInfo.GroupResource(), r.Method) } @@ -141,18 +142,18 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. }), nil } -func (s *filesConnector) doRead(ctx context.Context, logger *slog.Logger, repo repository.Repository, path string, ref string) (int, *provisioning.ResourceWrapper, error) { - info, err := repo.Read(ctx, logger, path, ref) +func (s *filesConnector) doRead(ctx context.Context, repo repository.Repository, path string, ref string) (int, *provisioning.ResourceWrapper, error) { + info, err := repo.Read(ctx, path, ref) if err != nil { return 0, nil, err } - parser, err := s.parsers.GetParser(repo) + parser, err := s.parsers.GetParser(ctx, repo) if err != nil { return 0, nil, err } - parsed, err := parser.Parse(ctx, logger, info, true) + parsed, err := parser.Parse(ctx, info, true) if err != nil { return 0, nil, err } @@ -161,7 +162,7 @@ func (s *filesConnector) doRead(ctx context.Context, logger *slog.Logger, repo r if parsed.GVR == nil { if parsed.GVK != nil { //nolint:govet - parsed.Errors = append(parsed.Errors, fmt.Errorf("unknown resource for Kind: "+parsed.GVK.Kind)) + parsed.Errors = append(parsed.Errors, fmt.Errorf("unknown resource for Kind: %s", parsed.GVK.Kind)) } else { parsed.Errors = append(parsed.Errors, fmt.Errorf("unknown resource")) } @@ -174,7 +175,7 @@ func (s *filesConnector) doRead(ctx context.Context, logger *slog.Logger, repo r return code, parsed.AsResourceWrapper(), nil } -func (s *filesConnector) doWrite(ctx context.Context, logger *slog.Logger, update bool, repo repository.Repository, path string, ref string, message string, req *http.Request) (*provisioning.ResourceWrapper, error) { +func (s *filesConnector) doWrite(ctx context.Context, update bool, repo repository.Repository, path string, ref string, message string, req *http.Request) (*provisioning.ResourceWrapper, error) { settings := repo.Config().Spec.Editing if update && !settings.Update { return nil, apierrors.NewForbidden(provisioning.RepositoryResourceInfo.GroupResource(), "updating files not enabled", nil) @@ -194,12 +195,12 @@ func (s *filesConnector) doWrite(ctx context.Context, logger *slog.Logger, updat Ref: ref, } - parser, err := s.parsers.GetParser(repo) + parser, err := s.parsers.GetParser(ctx, repo) if err != nil { return nil, err } - parsed, err := parser.Parse(ctx, logger, info, true) + parsed, err := parser.Parse(ctx, info, true) if err != nil { if errors.Is(err, resources.ErrUnableToReadResourceBytes) { return nil, apierrors.NewBadRequest("unable to read the request as a resource") @@ -218,9 +219,9 @@ func (s *filesConnector) doWrite(ctx context.Context, logger *slog.Logger, updat } if update { - err = repo.Update(ctx, logger, path, ref, data, message) + err = repo.Update(ctx, path, ref, data, message) } else { - err = repo.Create(ctx, logger, path, ref, data, message) + err = repo.Create(ctx, path, ref, data, message) } if err != nil { return nil, err @@ -247,18 +248,19 @@ func (s *filesConnector) doWrite(ctx context.Context, logger *slog.Logger, updat return parsed.AsResourceWrapper(), err } -func (s *filesConnector) doDelete(ctx context.Context, logger *slog.Logger, repo repository.Repository, path string, ref string, message string) (*provisioning.ResourceWrapper, error) { +func (s *filesConnector) doDelete(ctx context.Context, repo repository.Repository, path string, ref string, message string) (*provisioning.ResourceWrapper, error) { settings := repo.Config().Spec.Editing if !settings.Delete { return nil, apierrors.NewForbidden(provisioning.RepositoryResourceInfo.GroupResource(), "deleting is not supported", nil) } - err := repo.Delete(ctx, logger, path, ref, message) + err := repo.Delete(ctx, path, ref, message) if err != nil { return nil, err } - fmt.Printf("TODO! trigger sync for this file we just deleted... %s\n", path) + logger := slogctx.From(ctx).With("path", path) + logger.InfoContext(ctx, "TODO! trigger sync for this file we just deleted...") return &provisioning.ResourceWrapper{ Path: path, diff --git a/pkg/registry/apis/provisioning/history.go b/pkg/registry/apis/provisioning/history.go index 96aabd60a21..94bd5cb5f34 100644 --- a/pkg/registry/apis/provisioning/history.go +++ b/pkg/registry/apis/provisioning/history.go @@ -3,7 +3,6 @@ package provisioning import ( "context" "fmt" - "log/slog" "net/http" "strings" @@ -11,11 +10,11 @@ import ( "k8s.io/apiserver/pkg/registry/rest" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/slogctx" ) type historySubresource struct { repoGetter RepoGetter - logger *slog.Logger } func (h *historySubresource) New() runtime.Object { @@ -50,7 +49,8 @@ func (h *historySubresource) NewConnectOptions() (runtime.Object, bool, string) } func (h *historySubresource) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { - logger := h.logger.With("repository_name", name) + logger := slogctx.From(ctx).With("logger", "history-subresource") + ctx = slogctx.To(ctx, logger) repo, err := h.repoGetter.GetRepository(ctx, name) if err != nil { logger.DebugContext(ctx, "failed to find repository", "error", err) @@ -60,8 +60,6 @@ func (h *historySubresource) Connect(ctx context.Context, name string, opts runt return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { query := r.URL.Query() ref := query.Get("ref") - logger = logger.With("ref", ref) - ctx := r.Context() var filePath string prefix := fmt.Sprintf("/%s/history/", name) @@ -70,9 +68,10 @@ func (h *historySubresource) Connect(ctx context.Context, name string, opts runt filePath = r.URL.Path[idx+len(prefix):] } - logger = logger.With("path", filePath) + logger := logger.With("ref", ref, "path", filePath) + ctx := slogctx.To(r.Context(), logger) - commits, err := repo.History(ctx, logger, filePath, ref) + commits, err := repo.History(ctx, filePath, ref) if err != nil { logger.DebugContext(ctx, "failed to get history", "error", err) responder.Error(err) diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest.go b/pkg/registry/apis/provisioning/jobs/pullrequest.go index 66ec7587299..ac87f0d2633 100644 --- a/pkg/registry/apis/provisioning/jobs/pullrequest.go +++ b/pkg/registry/apis/provisioning/jobs/pullrequest.go @@ -6,13 +6,13 @@ import ( "errors" "fmt" "html/template" - "log/slog" "net/url" "path" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" + "github.com/grafana/grafana/pkg/slogctx" ) // resourcePreview represents a resource that has changed in a pull request. @@ -52,11 +52,11 @@ Click the preview links above to view how your changes will look and compare the type PullRequestRepo interface { Config() *provisioning.Repository - Read(ctx context.Context, logger *slog.Logger, path, ref string) (*repository.FileInfo, error) - CompareFiles(ctx context.Context, logger *slog.Logger, base, ref string) ([]repository.FileChange, error) - ClearAllPullRequestFileComments(ctx context.Context, logger *slog.Logger, pr int) error - CommentPullRequestFile(ctx context.Context, logger *slog.Logger, pr int, path string, ref string, comment string) error - CommentPullRequest(ctx context.Context, logger *slog.Logger, pr int, comment string) error + Read(ctx context.Context, path, ref string) (*repository.FileInfo, error) + CompareFiles(ctx context.Context, base, ref string) ([]repository.FileChange, error) + ClearAllPullRequestFileComments(ctx context.Context, pr int) error + CommentPullRequestFile(ctx context.Context, pr int, path string, ref string, comment string) error + CommentPullRequest(ctx context.Context, pr int, comment string) error } // PreviewRenderer is an interface for rendering a preview of a file @@ -68,7 +68,6 @@ type PreviewRenderer interface { type PullRequestCommenter struct { repo PullRequestRepo parser *resources.Parser - logger *slog.Logger lintTemplate *template.Template prevTemplate *template.Template baseURL *url.URL @@ -78,7 +77,6 @@ type PullRequestCommenter struct { func NewPullRequestCommenter( repo PullRequestRepo, parser *resources.Parser, - logger *slog.Logger, renderer PreviewRenderer, baseURL *url.URL, ) (*PullRequestCommenter, error) { @@ -94,7 +92,6 @@ func NewPullRequestCommenter( return &PullRequestCommenter{ repo: repo, parser: parser, - logger: logger, lintTemplate: lintTemplate, prevTemplate: prevTemplate, renderer: renderer, @@ -114,7 +111,7 @@ func (c *PullRequestCommenter) Process(ctx context.Context, job provisioning.Job return nil } - logger := c.logger.With("pr", job.Spec.PR) + logger := slogctx.From(ctx).With("pr", job.Spec.PR) logger.InfoContext(ctx, "process pull request") defer logger.InfoContext(ctx, "pull request processed") @@ -123,13 +120,13 @@ func (c *PullRequestCommenter) Process(ctx context.Context, job provisioning.Job ref := spec.Hash // list pull requests changes files - files, err := c.repo.CompareFiles(ctx, logger, base, ref) + files, err := c.repo.CompareFiles(ctx, base, ref) if err != nil { return fmt.Errorf("failed to list pull request files: %w", err) } // clear all previous comments - if err := c.repo.ClearAllPullRequestFileComments(ctx, logger, spec.PR); err != nil { + if err := c.repo.ClearAllPullRequestFileComments(ctx, spec.PR); err != nil { return fmt.Errorf("failed to clear pull request comments: %w", err) } @@ -141,17 +138,17 @@ func (c *PullRequestCommenter) Process(ctx context.Context, job provisioning.Job previews := make([]resourcePreview, 0, len(files)) for _, f := range files { - if c.parser.ShouldIgnore(ctx, logger, f.Path) { + if c.parser.ShouldIgnore(f.Path) { continue } logger := logger.With("file", f.Path) - fileInfo, err := c.repo.Read(ctx, logger, f.Path, ref) + fileInfo, err := c.repo.Read(ctx, f.Path, ref) if err != nil { return fmt.Errorf("failed to read file %s: %w", f.Path, err) } - parsed, err := c.parser.Parse(ctx, logger, fileInfo, true) + parsed, err := c.parser.Parse(ctx, fileInfo, true) if err != nil { if errors.Is(err, resources.ErrUnableToReadResourceBytes) { logger.DebugContext(ctx, "file is not a resource", "path", f.Path) @@ -167,7 +164,7 @@ func (c *PullRequestCommenter) Process(ctx context.Context, job provisioning.Job return fmt.Errorf("execute lint comment template: %w", err) } - if err := c.repo.CommentPullRequestFile(ctx, logger, spec.PR, f.Path, ref, buf.String()); err != nil { + if err := c.repo.CommentPullRequestFile(ctx, spec.PR, f.Path, ref, buf.String()); err != nil { return fmt.Errorf("comment pull request file %s: %w", f.Path, err) } @@ -215,7 +212,7 @@ func (c *PullRequestCommenter) Process(ctx context.Context, job provisioning.Job return fmt.Errorf("execute previews comment template: %w", err) } - if err := c.repo.CommentPullRequest(ctx, logger, spec.PR, buf.String()); err != nil { + if err := c.repo.CommentPullRequest(ctx, spec.PR, buf.String()); err != nil { return fmt.Errorf("comment pull request: %w", err) } diff --git a/pkg/registry/apis/provisioning/jobs/queue.go b/pkg/registry/apis/provisioning/jobs/queue.go index 6f70b18ad02..70599f68997 100644 --- a/pkg/registry/apis/provisioning/jobs/queue.go +++ b/pkg/registry/apis/provisioning/jobs/queue.go @@ -3,7 +3,6 @@ package jobs import ( "context" "fmt" - "log/slog" "strconv" "sync" "time" @@ -14,6 +13,7 @@ import ( "k8s.io/apiserver/pkg/storage" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/slogctx" "github.com/grafana/grafana/pkg/util" ) @@ -24,12 +24,10 @@ func NewJobQueue(capacity int) JobQueue { jobs: []provisioning.Job{}, watchSet: NewWatchSet(), versioner: &storage.APIObjectVersioner{}, - logger: slog.Default().With("logger", "job-queue"), } } type jobStore struct { - logger *slog.Logger capacity int worker Worker @@ -108,16 +106,19 @@ func (s *jobStore) Add(ctx context.Context, job *provisioning.Job) (*provisionin // Reads the queue until no jobs remain func (s *jobStore) drainPending() { + logger := slogctx.From(context.Background()).With("logger", "job-store") + ctx := slogctx.To(context.Background(), logger) + var err error for { time.Sleep(time.Microsecond * 200) - ctx := context.Background() job := s.Checkout(ctx, nil) if job == nil { return // done } - logger := s.logger.With("job", job.GetName(), "namespace", job.GetNamespace()) + logger := logger.With("job", job.GetName(), "namespace", job.GetNamespace()) + ctx := slogctx.To(ctx, logger) started := time.Now() var status *provisioning.JobStatus diff --git a/pkg/registry/apis/provisioning/jobs/worker.go b/pkg/registry/apis/provisioning/jobs/worker.go index 90be7205059..b0e70ad05a8 100644 --- a/pkg/registry/apis/provisioning/jobs/worker.go +++ b/pkg/registry/apis/provisioning/jobs/worker.go @@ -3,7 +3,6 @@ package jobs import ( "context" "fmt" - "log/slog" "net/url" "time" @@ -17,6 +16,7 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" "github.com/grafana/grafana/pkg/services/rendering" + "github.com/grafana/grafana/pkg/slogctx" "github.com/grafana/grafana/pkg/storage/unified/blob" ) @@ -31,7 +31,6 @@ type JobWorker struct { getter RepoGetter parsers *resources.ParserFactory identities auth.BackgroundIdentityService - logger *slog.Logger render rendering.Service blobstore blob.PublicBlobStore urlProvider func(namespace string) string @@ -42,7 +41,6 @@ func NewJobWorker( parsers *resources.ParserFactory, client client.ProvisioningV0alpha1Interface, identities auth.BackgroundIdentityService, - logger *slog.Logger, render rendering.Service, blobstore blob.PublicBlobStore, urlProvider func(namespace string) string, @@ -52,7 +50,6 @@ func NewJobWorker( client: client, parsers: parsers, identities: identities, - logger: logger, render: render, blobstore: blobstore, urlProvider: urlProvider, @@ -60,7 +57,9 @@ func NewJobWorker( } func (g *JobWorker) Process(ctx context.Context, job provisioning.Job) (*provisioning.JobStatus, error) { - logger := g.logger.With("job", job.GetName(), "namespace", job.GetNamespace()) + logger := slogctx.From(ctx).With("job", job.GetName(), "namespace", job.GetNamespace()) + ctx = slogctx.To(ctx, logger) + id, err := g.identities.WorkerIdentity(ctx, job.Name) if err != nil { return nil, err @@ -72,6 +71,7 @@ func (g *JobWorker) Process(ctx context.Context, job provisioning.Job) (*provisi return nil, fmt.Errorf("missing repository name in label") } logger = logger.With("repository", repoName) + ctx = slogctx.To(ctx, logger) repo, err := g.getter.GetRepository(ctx, repoName) if err != nil { @@ -81,12 +81,12 @@ func (g *JobWorker) Process(ctx context.Context, job provisioning.Job) (*provisi return nil, fmt.Errorf("unknown repository") } - parser, err := g.parsers.GetParser(repo) + parser, err := g.parsers.GetParser(ctx, repo) if err != nil { return nil, fmt.Errorf("failed to get parser for %s: %w", repo.Config().Name, err) } - replicator, err := resources.NewReplicator(repo, parser, logger) + replicator, err := resources.NewReplicator(repo, parser) if err != nil { return nil, fmt.Errorf("error creating replicator") } @@ -151,7 +151,7 @@ func (g *JobWorker) Process(ctx context.Context, job provisioning.Job) (*provisi id: id, } - commenter, err := NewPullRequestCommenter(prRepo, parser, logger, renderer, baseURL) + commenter, err := NewPullRequestCommenter(prRepo, parser, renderer, baseURL) if err != nil { return nil, fmt.Errorf("error creating pull request commenter: %w", err) } diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 2414cf8e580..c5435cd6fab 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -3,7 +3,6 @@ package provisioning import ( "context" "fmt" - "log/slog" "net/http" "time" @@ -57,7 +56,6 @@ type ProvisioningAPIBuilder struct { features featuremgmt.FeatureToggles getter rest.Getter localFileResolver *repository.LocalFolderResolver - logger *slog.Logger render rendering.Service blobstore blob.PublicBlobStore client *resources.ClientFactory @@ -86,7 +84,6 @@ func NewProvisioningAPIBuilder( builder := &ProvisioningAPIBuilder{ urlProvider: urlProvider, localFileResolver: local, - logger: slog.Default().With("logger", "provisioning-api-builder"), webhookSecretKey: webhookSecretKey, features: features, ghFactory: ghFactory, @@ -94,7 +91,6 @@ func NewProvisioningAPIBuilder( client: clientFactory, parsers: &resources.ParserFactory{ Client: clientFactory, - Logger: slog.Default().With("logger", "provisioning-parser-factory"), }, render: render, blobstore: blobstore, @@ -189,29 +185,23 @@ func (b *ProvisioningAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserv getter: b, client: b.identities, jobs: b.jobs, - logger: b.logger.With("connector", "webhook"), } storage[provisioning.RepositoryResourceInfo.StoragePath("test")] = &testConnector{ getter: b, - logger: b.logger.With("connector", "test"), } storage[provisioning.RepositoryResourceInfo.StoragePath("files")] = &filesConnector{ getter: b, parsers: b.parsers, - logger: b.logger.With("connector", "files"), } storage[provisioning.RepositoryResourceInfo.StoragePath("history")] = &historySubresource{ repoGetter: b, - logger: b.logger.With("connector", "history"), } storage[provisioning.RepositoryResourceInfo.StoragePath("sync")] = &syncConnector{ repoGetter: b, jobs: b.jobs, - logger: b.logger.With("connector", "sync"), } storage[provisioning.RepositoryResourceInfo.StoragePath("export")] = &exportConnector{ repoGetter: b, - logger: b.logger.With("connector", "export"), queue: b.jobs, } apiGroupInfo.VersionedResourcesStorageMap[provisioning.VERSION] = storage @@ -390,7 +380,6 @@ func (b *ProvisioningAPIBuilder) GetPostStartHooks() (map[string]genericapiserve b.tester = &RepositoryTester{ clientFactory: b.client, client: c.ProvisioningV0alpha1(), - logger: slog.Default().With("logger", "provisioning-repository-tester"), } b.jobs.Register(jobs.NewJobWorker( @@ -398,7 +387,6 @@ func (b *ProvisioningAPIBuilder) GetPostStartHooks() (map[string]genericapiserve b.parsers, c.ProvisioningV0alpha1(), b.identities, - b.logger.With("worker", "github"), b.render, b.blobstore, b.urlProvider, diff --git a/pkg/registry/apis/provisioning/repository/github.go b/pkg/registry/apis/provisioning/repository/github.go index 7b1aeb65827..98b5ef29316 100644 --- a/pkg/registry/apis/provisioning/repository/github.go +++ b/pkg/registry/apis/provisioning/repository/github.go @@ -17,6 +17,7 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" pgh "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" + "github.com/grafana/grafana/pkg/slogctx" ) var subscribedEvents = []string{"push", "pull_request"} @@ -25,8 +26,8 @@ type SecretsService interface { Encrypt(ctx context.Context, data string) (string, error) } +// Make sure all public functions of this struct call the (*githubRepository).logger function, to ensure the GH repo details are included. type githubRepository struct { - logger *slog.Logger config *provisioning.Repository gh pgh.Client secrets SecretsService @@ -44,7 +45,6 @@ func NewGitHub( ) *githubRepository { return &githubRepository{ config: config, - logger: slog.Default().With("logger", "github-repository"), gh: factory.New(ctx, config.Spec.GitHub.Token), secrets: secrets, webhookURL: webhookURL, @@ -85,7 +85,7 @@ func (r *githubRepository) Validate() (list field.ErrorList) { } // Test implements provisioning.Repository. -func (r *githubRepository) Test(ctx context.Context, logger *slog.Logger) (*provisioning.TestResults, error) { +func (r *githubRepository) Test(ctx context.Context) (*provisioning.TestResults, error) { if err := r.gh.IsAuthenticated(ctx); err != nil { // TODO: should we return a more specific error or error code? return &provisioning.TestResults{ @@ -138,7 +138,7 @@ func (r *githubRepository) Test(ctx context.Context, logger *slog.Logger) (*prov } // ReadResource implements provisioning.Repository. -func (r *githubRepository) Read(ctx context.Context, logger *slog.Logger, filePath, ref string) (*FileInfo, error) { +func (r *githubRepository) Read(ctx context.Context, filePath, ref string) (*FileInfo, error) { if ref == "" { ref = r.config.Spec.GitHub.Branch } @@ -174,13 +174,13 @@ func (r *githubRepository) Read(ctx context.Context, logger *slog.Logger, filePa }, nil } -func (r *githubRepository) ReadTree(ctx context.Context, logger *slog.Logger, ref string) ([]FileTreeEntry, error) { +func (r *githubRepository) ReadTree(ctx context.Context, ref string) ([]FileTreeEntry, error) { if ref == "" { ref = r.config.Spec.GitHub.Branch } owner := r.config.Spec.GitHub.Owner repo := r.config.Spec.GitHub.Repository - logger = logger.With("owner", owner, "repo", repo, "ref", ref) + ctx, logger := r.logger(ctx, ref) tree, truncated, err := r.gh.GetTree(ctx, owner, repo, ref, true) if err != nil { @@ -210,10 +210,11 @@ func (r *githubRepository) ReadTree(ctx context.Context, logger *slog.Logger, re return entries, nil } -func (r *githubRepository) Create(ctx context.Context, logger *slog.Logger, path, ref string, data []byte, comment string) error { +func (r *githubRepository) Create(ctx context.Context, path, ref string, data []byte, comment string) error { if ref == "" { ref = r.config.Spec.GitHub.Branch } + ctx, _ = r.logger(ctx, ref) if err := r.ensureBranchExists(ctx, ref); err != nil { return fmt.Errorf("create branch on create: %w", err) @@ -234,10 +235,11 @@ func (r *githubRepository) Create(ctx context.Context, logger *slog.Logger, path return err } -func (r *githubRepository) Update(ctx context.Context, logger *slog.Logger, path, ref string, data []byte, comment string) error { +func (r *githubRepository) Update(ctx context.Context, path, ref string, data []byte, comment string) error { if ref == "" { ref = r.config.Spec.GitHub.Branch } + ctx, _ = r.logger(ctx, ref) if err := r.ensureBranchExists(ctx, ref); err != nil { return fmt.Errorf("create branch on update: %w", err) @@ -266,10 +268,11 @@ func (r *githubRepository) Update(ctx context.Context, logger *slog.Logger, path return nil } -func (r *githubRepository) Delete(ctx context.Context, logger *slog.Logger, path, ref, comment string) error { +func (r *githubRepository) Delete(ctx context.Context, path, ref, comment string) error { if ref == "" { ref = r.config.Spec.GitHub.Branch } + ctx, _ = r.logger(ctx, ref) if err := r.ensureBranchExists(ctx, ref); err != nil { return fmt.Errorf("create branch on delete: %w", err) @@ -294,10 +297,11 @@ func (r *githubRepository) Delete(ctx context.Context, logger *slog.Logger, path return r.gh.DeleteFile(ctx, owner, repo, path, ref, comment, file.GetSHA()) } -func (r *githubRepository) History(ctx context.Context, logger *slog.Logger, path, ref string) ([]provisioning.HistoryItem, error) { +func (r *githubRepository) History(ctx context.Context, path, ref string) ([]provisioning.HistoryItem, error) { if ref == "" { ref = r.config.Spec.GitHub.Branch } + ctx, _ = r.logger(ctx, ref) commits, err := r.gh.Commits(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, path, ref) if err != nil { @@ -386,7 +390,7 @@ func (r *githubRepository) ensureBranchExists(ctx context.Context, branchName st } if ok { - r.logger.InfoContext(ctx, "branch already exists", "branch", branchName) + slogctx.From(ctx).InfoContext(ctx, "branch already exists", "branch", branchName) return nil } @@ -412,7 +416,7 @@ func (r *githubRepository) ensureBranchExists(ctx context.Context, branchName st } // Webhook implements Repository. -func (r *githubRepository) Webhook(ctx context.Context, logger *slog.Logger, req *http.Request) (*provisioning.WebhookResponse, error) { +func (r *githubRepository) Webhook(ctx context.Context, req *http.Request) (*provisioning.WebhookResponse, error) { if r.config.Status.Webhook == nil { return nil, fmt.Errorf("unexpected webhook request") } @@ -524,7 +528,8 @@ func (r *githubRepository) parsePullRequestEvent(event *github.PullRequestEvent) }, nil } -func (r *githubRepository) LatestRef(ctx context.Context, logger *slog.Logger) (string, error) { +func (r *githubRepository) LatestRef(ctx context.Context) (string, error) { + ctx, _ = r.logger(ctx, "") branch, err := r.gh.GetBranch(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, r.Config().Spec.GitHub.Branch) if err != nil { return "", fmt.Errorf("get branch: %w", err) @@ -533,14 +538,15 @@ func (r *githubRepository) LatestRef(ctx context.Context, logger *slog.Logger) ( return branch.Sha, nil } -func (r *githubRepository) CompareFiles(ctx context.Context, logger *slog.Logger, base, ref string) ([]FileChange, error) { +func (r *githubRepository) CompareFiles(ctx context.Context, base, ref string) ([]FileChange, error) { if ref == "" { var err error - ref, err = r.LatestRef(ctx, logger) + ref, err = r.LatestRef(ctx) if err != nil { return nil, fmt.Errorf("get latest ref: %w", err) } } + ctx, logger := r.logger(ctx, ref) owner := r.config.Spec.GitHub.Owner repo := r.config.Spec.GitHub.Repository @@ -594,17 +600,20 @@ func (r *githubRepository) shouldLintPullRequest() bool { } // ClearAllPullRequestFileComments clears all comments on a pull request -func (r *githubRepository) ClearAllPullRequestFileComments(ctx context.Context, logger *slog.Logger, prNumber int) error { +func (r *githubRepository) ClearAllPullRequestFileComments(ctx context.Context, prNumber int) error { + ctx, _ = r.logger(ctx, "") return r.gh.ClearAllPullRequestFileComments(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, prNumber) } // CommentPullRequest adds a comment to a pull request. -func (r *githubRepository) CommentPullRequest(ctx context.Context, logger *slog.Logger, prNumber int, comment string) error { +func (r *githubRepository) CommentPullRequest(ctx context.Context, prNumber int, comment string) error { + ctx, _ = r.logger(ctx, "") return r.gh.CreatePullRequestComment(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, prNumber, comment) } // CommentPullRequestFile lints a file and comments the issues found. -func (r *githubRepository) CommentPullRequestFile(ctx context.Context, logger *slog.Logger, prNumber int, path, ref, comment string) error { +func (r *githubRepository) CommentPullRequestFile(ctx context.Context, prNumber int, path, ref, comment string) error { + ctx, _ = r.logger(ctx, ref) fileComment := pgh.FileComment{ Content: comment, Path: path, @@ -617,7 +626,7 @@ func (r *githubRepository) CommentPullRequestFile(ctx context.Context, logger *s return r.gh.CreatePullRequestFileComment(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, prNumber, fileComment) } -func (r *githubRepository) createWebhook(ctx context.Context, logger *slog.Logger) (pgh.WebhookConfig, error) { +func (r *githubRepository) createWebhook(ctx context.Context) (pgh.WebhookConfig, error) { secret, err := r.secrets.Encrypt(ctx, r.config.Spec.GitHub.Token) if err != nil { return pgh.WebhookConfig{}, fmt.Errorf("encrypt webhook secret: %w", err) @@ -636,15 +645,15 @@ func (r *githubRepository) createWebhook(ctx context.Context, logger *slog.Logge return pgh.WebhookConfig{}, err } - logger.InfoContext(ctx, "webhook created", "url", cfg.URL, "id", hook.ID) + slogctx.From(ctx).InfoContext(ctx, "webhook created", "url", cfg.URL, "id", hook.ID) return hook, nil } // updateWebhook checks if the webhook needs to be updated and updates it if necessary. // if the webhook does not exist, it will create it. -func (r *githubRepository) updateWebhook(ctx context.Context, logger *slog.Logger) (pgh.WebhookConfig, bool, error) { +func (r *githubRepository) updateWebhook(ctx context.Context) (pgh.WebhookConfig, bool, error) { if r.config.Status.Webhook == nil { - hook, err := r.createWebhook(ctx, logger) + hook, err := r.createWebhook(ctx) if err != nil { return pgh.WebhookConfig{}, false, err } @@ -657,7 +666,7 @@ func (r *githubRepository) updateWebhook(ctx context.Context, logger *slog.Logge hook, err := r.gh.GetWebhook(ctx, owner, repoName, r.config.Status.Webhook.ID) switch { case errors.Is(err, pgh.ErrResourceNotFound): - hook, err := r.createWebhook(ctx, logger) + hook, err := r.createWebhook(ctx) if err != nil { return pgh.WebhookConfig{}, false, err } @@ -703,7 +712,7 @@ func (r *githubRepository) updateWebhook(ctx context.Context, logger *slog.Logge return hook, true, nil } -func (r *githubRepository) deleteWebhook(ctx context.Context, logger *slog.Logger) error { +func (r *githubRepository) deleteWebhook(ctx context.Context) error { if r.config.Status.Webhook == nil { return fmt.Errorf("webhook not found") } @@ -716,12 +725,13 @@ func (r *githubRepository) deleteWebhook(ctx context.Context, logger *slog.Logge return fmt.Errorf("delete webhook: %w", err) } - logger.InfoContext(ctx, "webhook deleted", "url", r.config.Status.Webhook.URL, "id", id) + slogctx.From(ctx).InfoContext(ctx, "webhook deleted", "url", r.config.Status.Webhook.URL, "id", id) return nil } -func (r *githubRepository) OnCreate(ctx context.Context, logger *slog.Logger) (*provisioning.RepositoryStatus, error) { - hook, err := r.createWebhook(ctx, logger) +func (r *githubRepository) OnCreate(ctx context.Context) (*provisioning.RepositoryStatus, error) { + ctx, _ = r.logger(ctx, "") + hook, err := r.createWebhook(ctx) if err != nil { return nil, err } @@ -737,8 +747,9 @@ func (r *githubRepository) OnCreate(ctx context.Context, logger *slog.Logger) (* return status, nil } -func (r *githubRepository) OnUpdate(ctx context.Context, logger *slog.Logger) (*provisioning.RepositoryStatus, error) { - hook, updated, err := r.updateWebhook(ctx, logger) +func (r *githubRepository) OnUpdate(ctx context.Context) (*provisioning.RepositoryStatus, error) { + ctx, _ = r.logger(ctx, "") + hook, updated, err := r.updateWebhook(ctx) if err != nil { return nil, err } @@ -758,6 +769,28 @@ func (r *githubRepository) OnUpdate(ctx context.Context, logger *slog.Logger) (* return status, nil } -func (r *githubRepository) OnDelete(ctx context.Context, logger *slog.Logger) error { - return r.deleteWebhook(ctx, logger) +func (r *githubRepository) OnDelete(ctx context.Context) error { + ctx, _ = r.logger(ctx, "") + return r.deleteWebhook(ctx) +} + +func (r *githubRepository) logger(ctx context.Context, ref string) (context.Context, *slog.Logger) { + logger := slogctx.From(ctx) + + type containsGh int + var containsGhKey containsGh + if ctx.Value(containsGhKey) != nil { + return ctx, slogctx.From(ctx) + } + + if ref == "" { + ref = r.config.Spec.GitHub.Branch + } + owner := r.config.Spec.GitHub.Owner + repo := r.config.Spec.GitHub.Repository + logger = logger.With(slog.Group("github_repository", "owner", owner, "name", repo, "ref", ref)) + ctx = slogctx.To(ctx, logger) + // We want to ensure we don't add multiple github_repository keys. With doesn't deduplicate the keys... + ctx = context.WithValue(ctx, containsGhKey, true) + return ctx, logger } diff --git a/pkg/registry/apis/provisioning/repository/github_test.go b/pkg/registry/apis/provisioning/repository/github_test.go index 130bd23f4ee..bbebb862d4b 100644 --- a/pkg/registry/apis/provisioning/repository/github_test.go +++ b/pkg/registry/apis/provisioning/repository/github_test.go @@ -2,7 +2,6 @@ package repository import ( "fmt" - "log/slog" "net/http" "os" "path" @@ -100,7 +99,6 @@ func TestParseWebhooks(t *testing.T) { }, }, }, - logger: slog.Default().With("logger", "github-repository"), } for _, tt := range tests { diff --git a/pkg/registry/apis/provisioning/repository/local.go b/pkg/registry/apis/provisioning/repository/local.go index d2aa9e67cc9..6508154c855 100644 --- a/pkg/registry/apis/provisioning/repository/local.go +++ b/pkg/registry/apis/provisioning/repository/local.go @@ -12,7 +12,6 @@ import ( "fmt" "io" "io/fs" - "log/slog" "net/http" "os" "path/filepath" @@ -24,6 +23,7 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath" + "github.com/grafana/grafana/pkg/slogctx" ) type LocalFolderResolver struct { @@ -125,7 +125,7 @@ func (r *localRepository) Validate() (fields field.ErrorList) { // Test implements provisioning.Repository. // NOTE: Validate has been called (and passed) before this function should be called -func (r *localRepository) Test(ctx context.Context, logger *slog.Logger) (*provisioning.TestResults, error) { +func (r *localRepository) Test(ctx context.Context) (*provisioning.TestResults, error) { if r.config.Spec.Local.Path == "" { return &provisioning.TestResults{ Code: http.StatusBadRequest, @@ -185,7 +185,7 @@ func (r *localRepository) validateRequest(ref string) error { } // ReadResource implements provisioning.Repository. -func (r *localRepository) Read(ctx context.Context, logger *slog.Logger, filePath string, ref string) (*FileInfo, error) { +func (r *localRepository) Read(ctx context.Context, filePath string, ref string) (*FileInfo, error) { if err := r.validateRequest(ref); err != nil { return nil, err } @@ -224,7 +224,7 @@ func (r *localRepository) Read(ctx context.Context, logger *slog.Logger, filePat } // ReadResource implements provisioning.Repository. -func (r *localRepository) ReadTree(ctx context.Context, logger *slog.Logger, ref string) ([]FileTreeEntry, error) { +func (r *localRepository) ReadTree(ctx context.Context, ref string) ([]FileTreeEntry, error) { if err := r.validateRequest(ref); err != nil { return nil, err } @@ -277,7 +277,7 @@ func (r *localRepository) calculateFileHash(path string) (string, int64, error) return hex.EncodeToString(hasher.Sum(nil)), size, nil } -func (r *localRepository) Create(ctx context.Context, logger *slog.Logger, sanitisedPath string, ref string, data []byte, comment string) error { +func (r *localRepository) Create(ctx context.Context, sanitisedPath string, ref string, data []byte, comment string) error { if err := r.validateRequest(ref); err != nil { return err } @@ -286,7 +286,6 @@ func (r *localRepository) Create(ctx context.Context, logger *slog.Logger, sanit sanitisedPath, err := safepath.Join(r.path, sanitisedPath) if err != nil { - logger.WarnContext(ctx, "got an invalid path from caller", "path", inputUnsafePath, "err", err) return err } @@ -305,7 +304,7 @@ func (r *localRepository) Create(ctx context.Context, logger *slog.Logger, sanit return os.WriteFile(sanitisedPath, data, 0600) } -func (r *localRepository) Update(ctx context.Context, logger *slog.Logger, path string, ref string, data []byte, comment string) error { +func (r *localRepository) Update(ctx context.Context, path string, ref string, data []byte, comment string) error { if err := r.validateRequest(ref); err != nil { return err } @@ -321,7 +320,7 @@ func (r *localRepository) Update(ctx context.Context, logger *slog.Logger, path return os.WriteFile(path, data, 0600) } -func (r *localRepository) Delete(ctx context.Context, logger *slog.Logger, path string, ref string, comment string) error { +func (r *localRepository) Delete(ctx context.Context, path string, ref string, comment string) error { if err := r.validateRequest(ref); err != nil { return err } @@ -334,7 +333,7 @@ func (r *localRepository) Delete(ctx context.Context, logger *slog.Logger, path return os.Remove(path) } -func (r *localRepository) History(ctx context.Context, logger *slog.Logger, path string, ref string) ([]provisioning.HistoryItem, error) { +func (r *localRepository) History(ctx context.Context, path string, ref string) ([]provisioning.HistoryItem, error) { return nil, &apierrors.StatusError{ ErrStatus: metav1.Status{ Message: "history is not yet implemented", @@ -344,7 +343,7 @@ func (r *localRepository) History(ctx context.Context, logger *slog.Logger, path } // Webhook implements Repository. -func (r *localRepository) Webhook(ctx context.Context, logger *slog.Logger, req *http.Request) (*provisioning.WebhookResponse, error) { +func (r *localRepository) Webhook(ctx context.Context, req *http.Request) (*provisioning.WebhookResponse, error) { return &provisioning.WebhookResponse{ Code: http.StatusAccepted, Job: &provisioning.JobSpec{ diff --git a/pkg/registry/apis/provisioning/repository/local_test.go b/pkg/registry/apis/provisioning/repository/local_test.go index e3efe758aef..2ce395c929d 100644 --- a/pkg/registry/apis/provisioning/repository/local_test.go +++ b/pkg/registry/apis/provisioning/repository/local_test.go @@ -2,7 +2,6 @@ package repository import ( "context" - "log/slog" "testing" "github.com/stretchr/testify/require" @@ -43,13 +42,13 @@ func TestLocal(t *testing.T) { expected := "the path given ('invalid/path') is invalid for a local repository (no permitted prefixes were configured)" - rsp, err := r.Test(context.Background(), slog.Default()) + rsp, err := r.Test(context.Background()) require.NoError(t, err) require.Equal(t, false, rsp.Success) require.Equal(t, []string{expected}, rsp.Errors) // We get the same error when trying to read a file - _, err = r.Read(context.Background(), slog.Default(), "path/to/file", "") + _, err = r.Read(context.Background(), "path/to/file", "") require.Equal(t, expected, err.Error()) }) } diff --git a/pkg/registry/apis/provisioning/repository/repository.go b/pkg/registry/apis/provisioning/repository/repository.go index aa315be04d8..309b1c8413a 100644 --- a/pkg/registry/apis/provisioning/repository/repository.go +++ b/pkg/registry/apis/provisioning/repository/repository.go @@ -3,7 +3,6 @@ package repository import ( "context" "io/fs" - "log/slog" "net/http" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -72,48 +71,48 @@ type Repository interface { Validate() field.ErrorList // Called to check if all connection information actually works - Test(ctx context.Context, logger *slog.Logger) (*provisioning.TestResults, error) + Test(ctx context.Context) (*provisioning.TestResults, error) // Read a file from the resource // This data will be parsed and validated before it is shown to end users - Read(ctx context.Context, logger *slog.Logger, path, ref string) (*FileInfo, error) + Read(ctx context.Context, path, ref string) (*FileInfo, error) // Read all file names from the tree. // This data will be parsed and validated before it is shown. // // TODO: Make some API contract that lets us ignore files that aren't relevant to us (e.g. CI/CD, CODEOWNERS, other configs or source code). // TODO: Test scale: do we want to stream entries instead somehow? - ReadTree(ctx context.Context, logger *slog.Logger, ref string) ([]FileTreeEntry, error) + ReadTree(ctx context.Context, ref string) ([]FileTreeEntry, error) // Write a file to the repository. // The data has already been validated and is ready for save - Create(ctx context.Context, logger *slog.Logger, path, ref string, data []byte, message string) error + Create(ctx context.Context, path, ref string, data []byte, message string) error // Update a file in the remote repository // The data has already been validated and is ready for save - Update(ctx context.Context, logger *slog.Logger, path, ref string, data []byte, message string) error + Update(ctx context.Context, path, ref string, data []byte, message string) error // Delete a file in the remote repository - Delete(ctx context.Context, logger *slog.Logger, path, ref, message string) error + Delete(ctx context.Context, path, ref, message string) error // History of changes for a path - History(ctx context.Context, logger *slog.Logger, path, ref string) ([]provisioning.HistoryItem, error) + History(ctx context.Context, path, ref string) ([]provisioning.HistoryItem, error) // For repositories that support webhooks - Webhook(ctx context.Context, logger *slog.Logger, req *http.Request) (*provisioning.WebhookResponse, error) + Webhook(ctx context.Context, req *http.Request) (*provisioning.WebhookResponse, error) } // Hooks called after the repository has been created, updated or deleted type RepositoryHooks interface { - OnCreate(ctx context.Context, logger *slog.Logger) (*provisioning.RepositoryStatus, error) - OnUpdate(ctx context.Context, logger *slog.Logger) (*provisioning.RepositoryStatus, error) - OnDelete(ctx context.Context, logger *slog.Logger) error + OnCreate(ctx context.Context) (*provisioning.RepositoryStatus, error) + OnUpdate(ctx context.Context) (*provisioning.RepositoryStatus, error) + OnDelete(ctx context.Context) error } // VersionedRepository is a repository that supports versioning // this inferface may be extended to make the the original Repository interface // more agnostic to the underlying storage system type VersionedRepository interface { - LatestRef(ctx context.Context, logger *slog.Logger) (string, error) - CompareFiles(ctx context.Context, logger *slog.Logger, base, ref string) ([]FileChange, error) + LatestRef(ctx context.Context) (string, error) + CompareFiles(ctx context.Context, base, ref string) ([]FileChange, error) } diff --git a/pkg/registry/apis/provisioning/repository/s3.go b/pkg/registry/apis/provisioning/repository/s3.go index fe0f2762f6c..1edce3f8404 100644 --- a/pkg/registry/apis/provisioning/repository/s3.go +++ b/pkg/registry/apis/provisioning/repository/s3.go @@ -2,7 +2,6 @@ package repository import ( "context" - "log/slog" "net/http" "k8s.io/apimachinery/pkg/api/errors" @@ -43,7 +42,7 @@ func (r *s3Repository) Validate() (list field.ErrorList) { } // Test implements provisioning.Repository. -func (r *s3Repository) Test(ctx context.Context, logger *slog.Logger) (*provisioning.TestResults, error) { +func (r *s3Repository) Test(ctx context.Context) (*provisioning.TestResults, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Message: "test is not yet implemented", @@ -53,7 +52,7 @@ func (r *s3Repository) Test(ctx context.Context, logger *slog.Logger) (*provisio } // ReadResource implements provisioning.Repository. -func (r *s3Repository) Read(ctx context.Context, logger *slog.Logger, path string, ref string) (*FileInfo, error) { +func (r *s3Repository) Read(ctx context.Context, path string, ref string) (*FileInfo, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Message: "read resource is not yet implemented", @@ -62,7 +61,7 @@ func (r *s3Repository) Read(ctx context.Context, logger *slog.Logger, path strin } } -func (r *s3Repository) ReadTree(ctx context.Context, logger *slog.Logger, ref string) ([]FileTreeEntry, error) { +func (r *s3Repository) ReadTree(ctx context.Context, ref string) ([]FileTreeEntry, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Message: "read file tree resource is not yet implemented", @@ -71,7 +70,7 @@ func (r *s3Repository) ReadTree(ctx context.Context, logger *slog.Logger, ref st } } -func (r *s3Repository) Create(ctx context.Context, logger *slog.Logger, path string, ref string, data []byte, comment string) error { +func (r *s3Repository) Create(ctx context.Context, path string, ref string, data []byte, comment string) error { return &errors.StatusError{ ErrStatus: metav1.Status{ Message: "write file is not yet implemented", @@ -80,7 +79,7 @@ func (r *s3Repository) Create(ctx context.Context, logger *slog.Logger, path str } } -func (r *s3Repository) Update(ctx context.Context, logger *slog.Logger, path string, ref string, data []byte, comment string) error { +func (r *s3Repository) Update(ctx context.Context, path string, ref string, data []byte, comment string) error { return &errors.StatusError{ ErrStatus: metav1.Status{ Message: "write file is not yet implemented", @@ -89,7 +88,7 @@ func (r *s3Repository) Update(ctx context.Context, logger *slog.Logger, path str } } -func (r *s3Repository) Delete(ctx context.Context, logger *slog.Logger, path string, ref string, comment string) error { +func (r *s3Repository) Delete(ctx context.Context, path string, ref string, comment string) error { return &errors.StatusError{ ErrStatus: metav1.Status{ Message: "delete file not yet implemented", @@ -98,7 +97,7 @@ func (r *s3Repository) Delete(ctx context.Context, logger *slog.Logger, path str } } -func (r *s3Repository) History(ctx context.Context, logger *slog.Logger, path string, ref string) ([]provisioning.HistoryItem, error) { +func (r *s3Repository) History(ctx context.Context, path string, ref string) ([]provisioning.HistoryItem, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Message: "history is not yet implemented", @@ -108,7 +107,7 @@ func (r *s3Repository) History(ctx context.Context, logger *slog.Logger, path st } // Webhook implements Repository. -func (r *s3Repository) Webhook(ctx context.Context, logger *slog.Logger, req *http.Request) (*provisioning.WebhookResponse, error) { +func (r *s3Repository) Webhook(ctx context.Context, req *http.Request) (*provisioning.WebhookResponse, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Code: http.StatusNotImplemented, diff --git a/pkg/registry/apis/provisioning/repository/unknown.go b/pkg/registry/apis/provisioning/repository/unknown.go index d2d00d022c4..26419c6c0c1 100644 --- a/pkg/registry/apis/provisioning/repository/unknown.go +++ b/pkg/registry/apis/provisioning/repository/unknown.go @@ -2,7 +2,6 @@ package repository import ( "context" - "log/slog" "net/http" "k8s.io/apimachinery/pkg/api/errors" @@ -39,7 +38,7 @@ func (r *unknownRepository) Validate() (fields field.ErrorList) { } // Test implements provisioning.Repository. -func (r *unknownRepository) Test(ctx context.Context, logger *slog.Logger) (*provisioning.TestResults, error) { +func (r *unknownRepository) Test(ctx context.Context) (*provisioning.TestResults, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Message: "test is not yet implemented", @@ -49,7 +48,7 @@ func (r *unknownRepository) Test(ctx context.Context, logger *slog.Logger) (*pro } // ReadResource implements provisioning.Repository. -func (r *unknownRepository) Read(ctx context.Context, logger *slog.Logger, path, ref string) (*FileInfo, error) { +func (r *unknownRepository) Read(ctx context.Context, path, ref string) (*FileInfo, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Message: "read resource is not yet implemented", @@ -58,7 +57,7 @@ func (r *unknownRepository) Read(ctx context.Context, logger *slog.Logger, path, } } -func (r *unknownRepository) ReadTree(ctx context.Context, logger *slog.Logger, ref string) ([]FileTreeEntry, error) { +func (r *unknownRepository) ReadTree(ctx context.Context, ref string) ([]FileTreeEntry, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Message: "read file tree resource is not yet implemented", @@ -67,7 +66,7 @@ func (r *unknownRepository) ReadTree(ctx context.Context, logger *slog.Logger, r } } -func (r *unknownRepository) Create(ctx context.Context, logger *slog.Logger, path, ref string, data []byte, comment string) error { +func (r *unknownRepository) Create(ctx context.Context, path, ref string, data []byte, comment string) error { return &errors.StatusError{ ErrStatus: metav1.Status{ Message: "write file is not yet implemented", @@ -76,7 +75,7 @@ func (r *unknownRepository) Create(ctx context.Context, logger *slog.Logger, pat } } -func (r *unknownRepository) Update(ctx context.Context, logger *slog.Logger, path, ref string, data []byte, comment string) error { +func (r *unknownRepository) Update(ctx context.Context, path, ref string, data []byte, comment string) error { return &errors.StatusError{ ErrStatus: metav1.Status{ Message: "write file is not yet implemented", @@ -85,7 +84,7 @@ func (r *unknownRepository) Update(ctx context.Context, logger *slog.Logger, pat } } -func (r *unknownRepository) Delete(ctx context.Context, logger *slog.Logger, path, ref, comment string) error { +func (r *unknownRepository) Delete(ctx context.Context, path, ref, comment string) error { return &errors.StatusError{ ErrStatus: metav1.Status{ Message: "delete file not yet implemented", @@ -94,7 +93,7 @@ func (r *unknownRepository) Delete(ctx context.Context, logger *slog.Logger, pat } } -func (r *unknownRepository) History(ctx context.Context, logger *slog.Logger, path, ref string) ([]provisioning.HistoryItem, error) { +func (r *unknownRepository) History(ctx context.Context, path, ref string) ([]provisioning.HistoryItem, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Message: "history is not yet implemented", @@ -104,7 +103,7 @@ func (r *unknownRepository) History(ctx context.Context, logger *slog.Logger, pa } // Webhook implements Repository. -func (r *unknownRepository) Webhook(ctx context.Context, logger *slog.Logger, req *http.Request) (*provisioning.WebhookResponse, error) { +func (r *unknownRepository) Webhook(ctx context.Context, req *http.Request) (*provisioning.WebhookResponse, error) { return nil, &errors.StatusError{ ErrStatus: metav1.Status{ Code: http.StatusNotImplemented, diff --git a/pkg/registry/apis/provisioning/repository_controller.go b/pkg/registry/apis/provisioning/repository_controller.go index 0f7daaf4981..206e46cb9b3 100644 --- a/pkg/registry/apis/provisioning/repository_controller.go +++ b/pkg/registry/apis/provisioning/repository_controller.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "log/slog" "time" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -22,8 +21,11 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/provisioning/auth" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/slogctx" ) +const loggerName = "provisioning-repository-controller" + type operation int const ( @@ -59,8 +61,7 @@ type RepositoryController struct { enqueueRepository func(op operation, obj any) keyFunc func(obj any) (string, error) - queue workqueue.TypedRateLimitingInterface[*queueItem] - logger *slog.Logger + queue workqueue.TypedRateLimitingInterface[*queueItem] } // NewRepositoryController creates new RepositoryController. @@ -85,7 +86,6 @@ func NewRepositoryController( repoGetter: repoGetter, identities: identities, tester: tester, - logger: slog.Default().With("logger", "provisioning-repository-controller"), jobs: jobs, } @@ -118,21 +118,23 @@ func (rc *RepositoryController) Run(ctx context.Context, workerCount int) { defer utilruntime.HandleCrash() defer rc.queue.ShutDown() - rc.logger.Info("Starting RepositoryController") - defer rc.logger.Info("Shutting down RepositoryController") + logger := slogctx.From(ctx).With("logger", loggerName) + ctx = slogctx.To(ctx, logger) + logger.InfoContext(ctx, "Starting RepositoryController") + defer logger.InfoContext(ctx, "Shutting down RepositoryController") if !cache.WaitForCacheSync(ctx.Done(), rc.repoSynced) { return } - rc.logger.Info("Starting workers", "count", workerCount) + logger.InfoContext(ctx, "Starting workers", "count", workerCount) for i := 0; i < workerCount; i++ { go wait.UntilWithContext(ctx, rc.runWorker, time.Second) } - rc.logger.Info("Started workers") + logger.InfoContext(ctx, "Started workers") <-ctx.Done() - rc.logger.Info("Shutting down workers") + logger.InfoContext(ctx, "Shutting down workers") } func (rc *RepositoryController) runWorker(ctx context.Context) { @@ -172,7 +174,9 @@ func (rc *RepositoryController) processNextWorkItem(ctx context.Context) bool { } defer rc.queue.Done(item) - logger := rc.logger.With("key", item.key) + // TODO: should we move tracking work to trace ids instead? + logger := slogctx.From(ctx).With("work_key", item.key) + ctx = slogctx.To(ctx, logger) // lets us track the work in child tasks, similar to a trace id logger.InfoContext(ctx, "RepositoryController processing key") err := rc.processFn(item) @@ -205,13 +209,16 @@ func (rc *RepositoryController) processNextWorkItem(ctx context.Context) bool { // process is the business logic of the controller. func (rc *RepositoryController) process(item *queueItem) error { - logger := rc.logger.With("key", item.key) + ctx := context.Background() + + logger := slogctx.From(ctx).With("logger", loggerName, "key", item.key) + ctx = slogctx.To(ctx, logger) + namespace, name, err := cache.SplitMetaNamespaceKey(item.key) if err != nil { return err } - ctx := context.Background() if item.op == operationDelete { logger.InfoContext(ctx, "handle repository deletion") cfg, ok := item.obj.(*provisioning.Repository) @@ -226,7 +233,7 @@ func (rc *RepositoryController) process(item *queueItem) error { hooks, ok := repo.(repository.RepositoryHooks) if ok { - return hooks.OnDelete(ctx, logger) + return hooks.OnDelete(ctx) } return nil } @@ -245,6 +252,7 @@ func (rc *RepositoryController) process(item *queueItem) error { } ctx = identity.WithRequester(ctx, id) logger = logger.With("repository", cachedRepo.Name, "namespace", cachedRepo.Namespace) + ctx = slogctx.To(ctx, logger) repo, err := rc.repoGetter.AsRepository(ctx, cachedRepo) if err != nil { @@ -264,13 +272,13 @@ func (rc *RepositoryController) process(item *queueItem) error { if ok { if cachedRepo.Status.ObservedGeneration > 0 { logger.InfoContext(ctx, "handle repository update") - status, err = hooks.OnUpdate(ctx, logger) + status, err = hooks.OnUpdate(ctx) if err != nil { return fmt.Errorf("handle repository update: %w", err) } } else { logger.InfoContext(ctx, "handle repository init") - status, err = hooks.OnCreate(ctx, logger) + status, err = hooks.OnCreate(ctx) if err != nil { return fmt.Errorf("handle repository create: %w", err) } diff --git a/pkg/registry/apis/provisioning/resources/fileformat.go b/pkg/registry/apis/provisioning/resources/fileformat.go index faa172d99d0..2282feace54 100644 --- a/pkg/registry/apis/provisioning/resources/fileformat.go +++ b/pkg/registry/apis/provisioning/resources/fileformat.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "io" - "log/slog" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -17,6 +16,7 @@ import ( dashboard "github.com/grafana/grafana/pkg/apis/dashboard/v1alpha1" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/slogctx" ) var ErrUnableToReadResourceBytes = errors.New("unable to read bytes as a resource") @@ -26,7 +26,7 @@ var ErrClassicResourceIsAlreadyK8sForm = errors.New("classic resource is already // The file path may determine how the resource is parsed // // The context and logger are both only used for logging purposes. They do not control any logic. -func ReadClassicResource(ctx context.Context, logger *slog.Logger, info *repository.FileInfo) (*unstructured.Unstructured, *schema.GroupVersionKind, provisioning.ClassicFileType, error) { +func ReadClassicResource(ctx context.Context, info *repository.FileInfo) (*unstructured.Unstructured, *schema.GroupVersionKind, provisioning.ClassicFileType, error) { var value map[string]any // Try parsing as JSON @@ -46,7 +46,7 @@ func ReadClassicResource(ctx context.Context, logger *slog.Logger, info *reposit return nil, nil, "", ErrClassicResourceIsAlreadyK8sForm } - logger.DebugContext(ctx, "TODO... likely a provisioning", + slogctx.From(ctx).DebugContext(ctx, "TODO... likely a provisioning", "apiVersion", value["apiVersion"], "kind", value["Kind"]) gv, err := schema.ParseGroupVersion(value["apiVersion"].(string)) diff --git a/pkg/registry/apis/provisioning/resources/fileformat_test.go b/pkg/registry/apis/provisioning/resources/fileformat_test.go index 8030ee8dd7b..6de40148d94 100644 --- a/pkg/registry/apis/provisioning/resources/fileformat_test.go +++ b/pkg/registry/apis/provisioning/resources/fileformat_test.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "log/slog" "os" "path" "testing" @@ -73,7 +72,7 @@ spec: t.Run("load dashboard json", func(t *testing.T) { // Support dashboard conversion - obj, gvk, classic, err := ReadClassicResource(context.Background(), slog.Default(), &repository.FileInfo{ + obj, gvk, classic, err := ReadClassicResource(context.Background(), &repository.FileInfo{ Data: []byte(`{ "schemaVersion": 7, "panels": [], @@ -115,7 +114,7 @@ spec: parser.linter = lint.NewDashboardLinter() // Support dashboard conversion - parsed, err := parser.Parse(context.Background(), slog.Default(), info, validate) + parsed, err := parser.Parse(context.Background(), info, validate) require.NoError(t, err) require.Equal(t, provisioning.ClassicDashboard, parsed.Classic) diff --git a/pkg/registry/apis/provisioning/resources/parser.go b/pkg/registry/apis/provisioning/resources/parser.go index 10dbd492d46..97f504119ed 100644 --- a/pkg/registry/apis/provisioning/resources/parser.go +++ b/pkg/registry/apis/provisioning/resources/parser.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "log/slog" "path" "path/filepath" @@ -22,22 +21,21 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/lint" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/slogctx" ) var ErrNamespaceMismatch = errors.New("the file namespace does not match target namespace") type ParserFactory struct { Client *ClientFactory - Logger *slog.Logger } -func (f *ParserFactory) GetParser(repo repository.Repository) (*Parser, error) { +func (f *ParserFactory) GetParser(ctx context.Context, repo repository.Repository) (*Parser, error) { config := repo.Config() client, kinds, err := f.Client.New(config.Namespace) // As system user if err != nil { return nil, err } - logger := f.Logger.With("parser", config.Name) parser := &Parser{ repo: config, @@ -46,11 +44,10 @@ func (f *ParserFactory) GetParser(repo repository.Repository) (*Parser, error) { mapper: NamesFromHashedRepoPath, } if repo.Config().Spec.Linting { - ctx := context.Background() - linterFactory := lint.NewDashboardLinterFactory() - cfg, err := repo.Read(ctx, logger, linterFactory.ConfigPath(), "") + cfg, err := repo.Read(ctx, linterFactory.ConfigPath(), "") + logger := slogctx.From(ctx) var linter lint.Linter switch { case err == nil: @@ -123,7 +120,7 @@ func (r *Parser) Client() *DynamicClient { return r.client } -func (r *Parser) ShouldIgnore(ctx context.Context, logger *slog.Logger, p string) bool { +func (r *Parser) ShouldIgnore(p string) bool { ext := filepath.Ext(p) if ext == ".yaml" || ext == ".json" { return false @@ -132,19 +129,20 @@ func (r *Parser) ShouldIgnore(ctx context.Context, logger *slog.Logger, p string return true } -func (r *Parser) Parse(ctx context.Context, logger *slog.Logger, info *repository.FileInfo, validate bool) (parsed *ParsedResource, err error) { +func (r *Parser) Parse(ctx context.Context, info *repository.FileInfo, validate bool) (parsed *ParsedResource, err error) { + logger := slogctx.From(ctx).With("path", info.Path, "validate", validate) parsed = &ParsedResource{ Info: info, } - if r.ShouldIgnore(ctx, logger, info.Path) { + if r.ShouldIgnore(info.Path) { return parsed, ErrUnableToReadResourceBytes } parsed.Obj, parsed.GVK, err = LoadYAMLOrJSON(bytes.NewBuffer(info.Data)) if err != nil { logger.DebugContext(ctx, "failed to find GVK of the input data", "error", err) - parsed.Obj, parsed.GVK, parsed.Classic, err = ReadClassicResource(ctx, logger, info) + parsed.Obj, parsed.GVK, parsed.Classic, err = ReadClassicResource(ctx, info) if err != nil { logger.DebugContext(ctx, "also failed to get GVK from fallback loader?", "error", err) return parsed, err diff --git a/pkg/registry/apis/provisioning/resources/replicator.go b/pkg/registry/apis/provisioning/resources/replicator.go index 395e1f158b9..020df151639 100644 --- a/pkg/registry/apis/provisioning/resources/replicator.go +++ b/pkg/registry/apis/provisioning/resources/replicator.go @@ -21,10 +21,10 @@ import ( apiutils "github.com/grafana/grafana/pkg/apimachinery/utils" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/slogctx" ) type Replicator struct { - logger *slog.Logger client *DynamicClient parser *Parser folders dynamic.ResourceInterface @@ -34,7 +34,6 @@ type Replicator struct { func NewReplicator( repo repository.Repository, parser *Parser, - logger *slog.Logger, ) (*Replicator, error) { dynamicClient := parser.Client() folders := dynamicClient.Resource(schema.GroupVersionResource{ @@ -44,7 +43,6 @@ func NewReplicator( }) return &Replicator{ - logger: logger, parser: parser, client: dynamicClient, folders: folders, @@ -63,32 +61,33 @@ func (r *Replicator) Sync(ctx context.Context) (string, error) { return "", fmt.Errorf("ensure repository folder exists: %w", err) } + logger := slogctx.From(ctx) var latest string switch { case !isVersioned: - r.logger.InfoContext(ctx, "replicate tree unversioned repository") + logger.InfoContext(ctx, "replicate tree unversioned repository") if err := r.replicateTree(ctx, ""); err != nil { return "", fmt.Errorf("replicate tree: %w", err) } case lastCommit == "": var err error - latest, err = versionedRepo.LatestRef(ctx, r.logger) + latest, err = versionedRepo.LatestRef(ctx) if err != nil { return "", fmt.Errorf("latest ref: %w", err) } if err := r.replicateTree(ctx, latest); err != nil { return latest, fmt.Errorf("replicate tree: %w", err) } - r.logger.InfoContext(ctx, "initial replication for versioned repository", "latest", latest) + logger.InfoContext(ctx, "initial replication for versioned repository", "latest", latest) default: var err error - latest, err = versionedRepo.LatestRef(ctx, r.logger) + latest, err = versionedRepo.LatestRef(ctx) if err != nil { return "", fmt.Errorf("latest ref: %w", err) } - r.logger.InfoContext(ctx, "replicate changes for versioned repository", "last_commit", lastCommit, "latest", latest) - changes, err := versionedRepo.CompareFiles(ctx, r.logger, lastCommit, latest) + logger.InfoContext(ctx, "replicate changes for versioned repository", "last_commit", lastCommit, "latest", latest) + changes, err := versionedRepo.CompareFiles(ctx, lastCommit, latest) if err != nil { return latest, fmt.Errorf("compare files: %w", err) } @@ -103,25 +102,24 @@ func (r *Replicator) Sync(ctx context.Context) (string, error) { // replicateTree replicates all files in the repository. func (r *Replicator) replicateTree(ctx context.Context, ref string) error { - logger := r.logger - tree, err := r.repository.ReadTree(ctx, logger, ref) + tree, err := r.repository.ReadTree(ctx, ref) if err != nil { return fmt.Errorf("read tree: %w", err) } for _, entry := range tree { - logger := logger.With("file", entry.Path) + logger := slogctx.From(ctx).With("file", entry.Path) if !entry.Blob { logger.DebugContext(ctx, "ignoring non-blob entry") continue } - if r.parser.ShouldIgnore(ctx, logger, entry.Path) { + if r.parser.ShouldIgnore(entry.Path) { logger.DebugContext(ctx, "ignoring file") continue } - info, err := r.repository.Read(ctx, logger, entry.Path, ref) + info, err := r.repository.Read(ctx, entry.Path, ref) if err != nil { return fmt.Errorf("read file: %w", err) } @@ -145,7 +143,7 @@ func (r *Replicator) replicateTree(ctx context.Context, ref string) error { // replicateFile creates a new resource in the cluster. // If the resource already exists, it will be updated. func (r *Replicator) replicateFile(ctx context.Context, fileInfo *repository.FileInfo) error { - logger := r.logger.With("file", fileInfo.Path, "ref", fileInfo.Ref) + logger := slogctx.From(ctx).With("file", fileInfo.Path, "ref", fileInfo.Ref) file, err := r.parseResource(ctx, fileInfo) if err != nil { return err @@ -213,7 +211,7 @@ func (r *Replicator) createFolderPath(ctx context.Context, filePath string) (str return parent, nil } - logger := r.logger.With("file", filePath) + logger := slogctx.From(ctx).With("file", filePath) for _, folder := range strings.Split(dir, "/") { if folder == "" { // Trailing / leading slash? @@ -257,11 +255,11 @@ func (r *Replicator) createFolderPath(ctx context.Context, filePath string) (str func (r *Replicator) replicateChanges(ctx context.Context, changes []repository.FileChange) error { for _, change := range changes { - if r.parser.ShouldIgnore(ctx, r.logger, change.Path) { + if r.parser.ShouldIgnore(change.Path) { continue } - fileInfo, err := r.repository.Read(ctx, r.logger, change.Path, change.Ref) + fileInfo, err := r.repository.Read(ctx, change.Path, change.Ref) if err != nil { return fmt.Errorf("read file: %w", err) } @@ -273,7 +271,7 @@ func (r *Replicator) replicateChanges(ctx context.Context, changes []repository. } case repository.FileActionRenamed: // delete in old path - oldPath, err := r.repository.Read(ctx, r.logger, change.PreviousPath, change.Ref) + oldPath, err := r.repository.Read(ctx, change.PreviousPath, change.Ref) if err != nil { return fmt.Errorf("read previous path: %w", err) } @@ -316,13 +314,11 @@ func (r *Replicator) deleteFile(ctx context.Context, fileInfo *repository.FileIn // TODO: delete folders if empty recursively - r.logger.InfoContext(ctx, "Deleted file", "name", file.Obj.GetName(), "path", fileInfo.Path) - return nil } func (r *Replicator) parseResource(ctx context.Context, fileInfo *repository.FileInfo) (*ParsedResource, error) { - file, err := r.parser.Parse(ctx, r.logger, fileInfo, true) + file, err := r.parser.Parse(ctx, fileInfo, true) if err != nil { return nil, fmt.Errorf("failed to parse file %s: %w", fileInfo.Path, err) } @@ -339,7 +335,7 @@ func (r *Replicator) parseResource(ctx context.Context, fileInfo *repository.Fil } func (r *Replicator) Export(ctx context.Context) error { - logger := r.logger + logger := slogctx.From(ctx) dashboardIface := r.client.Resource(schema.GroupVersionResource{ Group: "dashboard.grafana.app", Version: "v2alpha1", @@ -385,7 +381,7 @@ func (r *Replicator) Export(ctx context.Context) error { return fmt.Errorf("failed to marshal dashboard %s: %w", name, err) } fileName := filepath.Join(folders.DirPath(folder), baseFileName) - logger = logger.With("file_name", fileName) + logger = logger.With("file", fileName) if logger.Enabled(ctx, slog.LevelDebug) { bodyStr := string(marshalledBody) logger.DebugContext(ctx, "got marshalled body for item", "body", bodyStr) @@ -397,14 +393,14 @@ func (r *Replicator) Export(ctx context.Context) error { } logger = logger.With("ref", ref) - _, err = r.repository.Read(ctx, r.logger, fileName, ref) + _, err = r.repository.Read(ctx, fileName, ref) if err != nil && !(errors.Is(err, repository.ErrFileNotFound) || apierrors.IsNotFound(err)) { logger.ErrorContext(ctx, "failed to check if file exists before writing", "error", err) return fmt.Errorf("failed to check if file exists before writing: %w", err) } else if err != nil { // ErrFileNotFound - err = r.repository.Create(ctx, r.logger, fileName, ref, marshalledBody, "export of dashboard "+name+" in namespace "+ns) + err = r.repository.Create(ctx, fileName, ref, marshalledBody, "export of dashboard "+name+" in namespace "+ns) } else { - err = r.repository.Update(ctx, r.logger, fileName, ref, marshalledBody, "export of dashboard "+name+" in namespace "+ns) + err = r.repository.Update(ctx, fileName, ref, marshalledBody, "export of dashboard "+name+" in namespace "+ns) } if err != nil { logger.ErrorContext(ctx, "failed to write a file in repository", "error", err) diff --git a/pkg/registry/apis/provisioning/sync.go b/pkg/registry/apis/provisioning/sync.go index dc110770ff9..3566cc083de 100644 --- a/pkg/registry/apis/provisioning/sync.go +++ b/pkg/registry/apis/provisioning/sync.go @@ -2,7 +2,6 @@ package provisioning import ( "context" - "log/slog" "net/http" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -15,7 +14,6 @@ import ( type syncConnector struct { repoGetter RepoGetter - logger *slog.Logger jobs jobs.JobQueue } diff --git a/pkg/registry/apis/provisioning/test.go b/pkg/registry/apis/provisioning/test.go index 3e8f7111943..230e39092fa 100644 --- a/pkg/registry/apis/provisioning/test.go +++ b/pkg/registry/apis/provisioning/test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "io" - "log/slog" "net/http" "slices" "time" @@ -24,7 +23,6 @@ import ( type testConnector struct { getter RepoGetter tester *RepositoryTester - logger *slog.Logger } func (*testConnector) New() runtime.Object { @@ -112,8 +110,6 @@ type RepositoryTester struct { // Repository+Jobs client client.ProvisioningV0alpha1Interface - - logger *slog.Logger } // This function will check if the repository is configured and functioning as expected @@ -131,7 +127,7 @@ func (t *RepositoryTester) TestRepository(ctx context.Context, repo repository.R return rsp, nil } - return repo.Test(ctx, t.logger) + return repo.Test(ctx) } // This function will check if the repository is configured and functioning as expected diff --git a/pkg/registry/apis/provisioning/webhook.go b/pkg/registry/apis/provisioning/webhook.go index d436943830d..b9a27064fb0 100644 --- a/pkg/registry/apis/provisioning/webhook.go +++ b/pkg/registry/apis/provisioning/webhook.go @@ -3,7 +3,6 @@ package provisioning import ( "context" "fmt" - "log/slog" "net/http" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -15,13 +14,13 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/auth" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" + "github.com/grafana/grafana/pkg/slogctx" ) // This only works for github right now type webhookConnector struct { client auth.BackgroundIdentityService getter RepoGetter - logger *slog.Logger jobs jobs.JobQueue } @@ -64,8 +63,10 @@ func (s *webhookConnector) Connect(ctx context.Context, name string, opts runtim } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - logger := s.logger.With("repo", name) - rsp, err := repo.Webhook(ctx, logger, r) + logger := slogctx.From(r.Context()).With("logger", "webhook-connector", "repo", name) + ctx := slogctx.To(r.Context(), logger) + + rsp, err := repo.Webhook(ctx, r) if err != nil { responder.Error(err) return diff --git a/pkg/slogctx/slogctx.go b/pkg/slogctx/slogctx.go new file mode 100644 index 00000000000..e2a72e72b1b --- /dev/null +++ b/pkg/slogctx/slogctx.go @@ -0,0 +1,30 @@ +// Slogctx defines contextual slog utilities, that allow for tying a slog logger with each context. +// The package only defines the context-related actions. To add more data to the logger itself, call the `log/slog` functions directly, e.g. With or WithGroup. +package slogctx + +import ( + "context" + "log/slog" +) + +// key is defined to be a separate type for Context. This is recommended practice, per Context.Value. +type key int + +// The loggerKey is the default key in the context. It is a separate type from any other, so no value is necessary. This is recommended practice, per Context.Value. +var loggerKey key + +// FromContext fetches the logger from the context, or constructs a new one from slog.Default. +// To include more values, you may want to call `With` or `WithGroup` directly afterwards. +func From(ctx context.Context) *slog.Logger { + logger, ok := ctx.Value(loggerKey).(*slog.Logger) + if !ok { + return slog.Default() + } + return logger +} + +// ToContext writes the logger to a new context wrapping the context given. +// The new context will include all other values, cancellations, etc. as the one given. +func To(ctx context.Context, logger *slog.Logger) context.Context { + return context.WithValue(ctx, loggerKey, logger) +}