Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

overwrite multislogger.Logger memory rather than reassigning pointer #1578

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions cmd/launcher/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,6 @@ func runLauncher(ctx context.Context, cancel func(), multiSlogger, systemMultiSl
fcOpts := []flags.Option{flags.WithCmdLineOpts(opts)}
flagController := flags.NewFlagController(logger, stores[storage.AgentFlagsStore], fcOpts...)
k := knapsack.New(stores, flagController, db, multiSlogger, systemMultiSlogger)
// reassign slogger to knapsack slogger to get launcher run id added to slogger
slogger = k.Slogger()

go runOsqueryVersionCheck(ctx, slogger, k.LatestOsquerydPath(ctx))
go timemachine.AddExclusions(ctx, k)
Expand Down Expand Up @@ -221,6 +219,7 @@ func runLauncher(ctx context.Context, cancel func(), multiSlogger, systemMultiSl
logger = teelogger.New(logger, logShipper)
logger = log.With(logger, "caller", log.Caller(5))
k.AddSlogHandler(logShipper.SlogHandler())

ctx = ctxlog.NewContext(ctx, logger) // Set the logger back in the ctx

k.SetTraceSamplingRateOverride(1.0, initialDebugDuration)
Expand Down
10 changes: 0 additions & 10 deletions ee/agent/knapsack/knapsack.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"log/slog"

"github.com/go-kit/kit/log"
"github.com/kolide/kit/ulid"
"github.com/kolide/launcher/ee/agent/storage"
"github.com/kolide/launcher/ee/agent/types"
"github.com/kolide/launcher/ee/tuf"
Expand All @@ -33,7 +32,6 @@ type knapsack struct {
db *bbolt.DB

slogger, systemSlogger *multislogger.MultiSlogger
launcherRunId string

// This struct is a work in progress, and will be iteratively added to as needs arise.
// Some potential future additions include:
Expand All @@ -47,11 +45,6 @@ func New(stores map[storage.Store]types.KVStore, flags types.Flags, db *bbolt.DB
stores: stores,
slogger: slogger,
systemSlogger: systemSlogger,
launcherRunId: ulid.New(),
}

if k.slogger != nil {
k.slogger.Logger = k.slogger.Logger.With("launcher_run_id", k.launcherRunId)
}

return k
Expand All @@ -68,9 +61,6 @@ func (k *knapsack) SystemSlogger() *slog.Logger {

func (k *knapsack) AddSlogHandler(handler ...slog.Handler) {
k.slogger.AddHandler(handler...)
k.slogger.Logger = k.slogger.Logger.With("launcher_run_id", k.launcherRunId)

// also send system logs to the same handlers
k.systemSlogger.AddHandler(handler...)
}

Expand Down
25 changes: 11 additions & 14 deletions pkg/log/multislogger/multislogger.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package multislogger

import (
"context"
"io"
"log/slog"

"github.com/kolide/kit/ulid"
slogmulti "github.com/samber/slog-multi"
)

Expand Down Expand Up @@ -33,40 +33,37 @@ var ctxValueKeysToAdd = []contextKey{

type MultiSlogger struct {
*slog.Logger
handlers []slog.Handler
handlers []slog.Handler
launcherRunId string
}

// New creates a new multislogger if no handlers are passed in, it will
// create a logger that discards all logs
func New(h ...slog.Handler) *MultiSlogger {
ms := new(MultiSlogger)

if len(h) == 0 {
// if we don't have any handlers passed in, we'll just discard the logs
// do not add the discard handler to the handlers so it will not be
// included when a handler is added
ms.Logger = slog.New(slog.NewTextHandler(io.Discard, nil))
return ms
ms := &MultiSlogger{
// setting to fanout with no handlers is noop
Logger: slog.New(slogmulti.Fanout()),
launcherRunId: ulid.New(),
}

ms.AddHandler(h...)
return ms
}

// AddHandler adds a handler to the multislogger, this creates a branch new
// slog.Logger under the the hood, mean any attributes added with
// Logger.With will be lost
// slog.Logger under the the hood, and overwrites old Logger memory address,
// this means any attributes added with Logger.With will be lost
func (m *MultiSlogger) AddHandler(handler ...slog.Handler) {
m.handlers = append(m.handlers, handler...)

// we have to rebuild the handler everytime because the slogmulti package we're
// using doesn't support adding handlers after the Fanout handler has been created
m.Logger = slog.New(
*m.Logger = *slog.New(
slogmulti.
Pipe(slogmulti.NewHandleInlineMiddleware(utcTimeMiddleware)).
Pipe(slogmulti.NewHandleInlineMiddleware(ctxValuesMiddleWare)).
Handler(slogmulti.Fanout(m.handlers...)),
)
).With("launcher_run_id", m.launcherRunId)
}

func utcTimeMiddleware(ctx context.Context, record slog.Record, next func(context.Context, slog.Record) error) error {
Expand Down
Loading