Skip to content

fix: initializing logger earlier#4466

Closed
casibbald wants to merge 2 commits intoweaveworks:mainfrom
casibbald:logger-initialization-issue
Closed

fix: initializing logger earlier#4466
casibbald wants to merge 2 commits intoweaveworks:mainfrom
casibbald:logger-initialization-issue

Conversation

@casibbald
Copy link
Copy Markdown
Collaborator

Closes

What changed?
initializing logger earlier on in start process

Why was this change made?
[controller-runtime] log.SetLogger(...) was never called; logs will not be displayed. Detected at: > goroutine 434 [running]: > runtime/debug.Stack() > /usr/local/go/src/runtime/debug/stack.go:26 +0x64 > sigs.k8s.io/controller-runtime/pkg/log.eventuallyFulfillRoot() > /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.4/pkg/log/log.go:60 +0xa0 > sigs.k8s.io/controller-runtime/pkg/log.(*delegatingLogSink).WithName(0x4000132580, {0x3291787, 0x14}) > /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.4/pkg/log/deleg.go:147 +0x34 > github.com/go-logr/logr.Logger.WithName({{0x3600dc8, 0x4000132580}, 0x0}, {0x3291787, 0x14}) > /go/pkg/mod/github.com/go-logr/logr@v1.4.2/logr.go:345 +0x5c > sigs.k8s.io/controller-runtime/pkg/client.newClient(0x400036d448, {0x0, 0x4000261650, {0x3602610, 0x40006ad270}, 0x0, 0x0}) > /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.4/pkg/client/client.go:118 +0xf8 > sigs.k8s.io/controller-runtime/pkg/client.New(0x400029fd48, {0x0, 0x4000261650, {0x3602610, 0x40006ad270}, 0x0, 0x0}) > /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.4/pkg/client/client.go:98 +0x68 > github.com/weaveworks/weave-gitops/core/clustersmngr/cluster.getClientFromConfig(0x400029fd48, 0x4000261650) > /app/core/clustersmngr/cluster/single.go:69 +0x2f0 > github.com/weaveworks/weave-gitops/core/clustersmngr/cluster.(*singleCluster).GetUserClient(0x40001320c0, 0x40007cfda0) > /app/core/clustersmngr/cluster/single.go:94 +0x108 > github.com/weaveworks/weave-gitops/core/clustersmngr.(*clustersManager).getOrCreateClient(0x40001acb40, 0x40007cfda0, {0x36023d0, 0x40001320c0}) > /app/core/clustersmngr/factory.go:623 +0x38c > github.com/weaveworks/weave-gitops/core/clustersmngr.(*clustersManager).getUserClientWithNamespaces.func1({0x36023d0, 0x40001320c0}, {0x35e7d98, 0x4000c9a890}, 0x4000974e00) > /app/core/clustersmngr/factory.go:426 +0xd8 > created by github.com/weaveworks/weave-gitops/core/clustersmngr.(*clustersManager).getUserClientWithNamespaces in goroutine 402 > /app/core/clustersmngr/factory.go:423 +0x364

How was this change implemented?

How did you validate the change?
Services starts, stacktrace gone

Release notes

Documentation Changes

Comment thread core/clustersmngr/cluster/single.go Outdated
"fmt"
"net"

//nolint:gci
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to run golangci-lint run --fix to avoid this suppression. I can add a new target to our makefile: make lint-fix.

@casibbald casibbald changed the title Initializing logger earlier WIP: Initializing logger earlier - do not merge Jan 8, 2025
Copy link
Copy Markdown
Contributor

@erikgb erikgb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! If you squash your commits I'll approve this! 😉

@casibbald casibbald changed the title WIP: Initializing logger earlier - do not merge fix: initializing logger earlier Jan 9, 2025
@casibbald casibbald enabled auto-merge (squash) January 9, 2025 07:54
@casibbald casibbald requested a review from erikgb January 9, 2025 07:54
@casibbald casibbald disabled auto-merge January 9, 2025 07:56
@casibbald casibbald force-pushed the logger-initialization-issue branch 2 times, most recently from 6dbd806 to 6a1ecea Compare January 9, 2025 09:06
@casibbald casibbald force-pushed the logger-initialization-issue branch from 6a1ecea to 91a72c8 Compare January 9, 2025 09:17
@casibbald casibbald closed this Jan 9, 2025
@casibbald casibbald deleted the logger-initialization-issue branch January 9, 2025 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants