Changes needed before gokit/log to slog transition. (#39527)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #38889 PLEASE READ BELOW before looking at file changes Before converting individual files/packages to slog, we generally need to make these 2 changes to make the conversion easier: - Replace uses of `kitlog.With` since they are not fully compatible with our kitlog adapter - Directly use the kitlog adapter logger type instead of the kitlog interface, which will let us have direct access to the underlying slog logger: `*logging.Logger` Note: that I did not replace absolutely all uses of `kitlog.Logger`, but I did remove all uses of `kitlog.With` except for these due to complexity: - server/logging/filesystem.go and the other log writers (webhook, firehose, kinesis, lambda, pubsub, nats) - server/datastore/mysql/nanomdm_storage.go (adapter pattern) - server/vulnerabilities/nvd/* (cascades to CLI tools) - server/service/osquery_utils/queries.go (callback type signatures cascade broadly) - cmd/maintained-apps/ (standalone, so can be transitioned later all at once) Most of the changes in this PR follow these patterns: - `kitlog.Logger` type → `*logging.Logger` - `kitlog.With(logger, ...)` → `logger.With(...)` - `kitlog.NewNopLogger() → logging.NewNopLogger()`, including similar variations such as `logging.NewLogfmtLogger(w)` and `logging.NewJSONLogger(w)` - removed many now-unused kitlog imports Unique changes that the PR review should focus on: - server/platform/logging/kitlog_adapter.go: Core adapter changes - server/platform/logging/logging.go: New convenience functions - server/service/integration_logger_test.go: Test changes for slog # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. - Was added in previous PR ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Migrated the codebase to a unified internal structured logging system for more consistent, reliable logs and observability. * No user-facing functionality changed; runtime behavior and APIs remain compatible. * **Tests** * Updated tests to use the new logging helpers to ensure consistent test logging and validation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -3,50 +3,45 @@ package logging
|
||||
import (
|
||||
"context"
|
||||
"log/slog"
|
||||
"slices"
|
||||
|
||||
kitlog "github.com/go-kit/log"
|
||||
)
|
||||
|
||||
// KitlogAdapter wraps a slog.Logger to implement the kitlog.Logger interface.
|
||||
// Logger wraps a slog.Logger to implement the kitlog.Logger interface.
|
||||
// This allows gradual migration from kitlog to slog by providing a drop-in
|
||||
// replacement that uses slog under the hood.
|
||||
type KitlogAdapter struct {
|
||||
type Logger struct {
|
||||
logger *slog.Logger
|
||||
// attrs holds any attributes added via With()
|
||||
attrs []any
|
||||
}
|
||||
|
||||
// NewKitlogAdapter creates a new adapter that implements kitlog.Logger
|
||||
// using the provided slog.Logger.
|
||||
func NewKitlogAdapter(logger *slog.Logger) kitlog.Logger {
|
||||
return &KitlogAdapter{
|
||||
// NewLogger creates a new adapter that implements kitlog.Logger
|
||||
// using the provided slog.Logger. It returns *Logger to preserve
|
||||
// type information, allowing callers to access SlogLogger() directly.
|
||||
func NewLogger(logger *slog.Logger) *Logger {
|
||||
return &Logger{
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
// Log implements kitlog.Logger. It converts key-value pairs to slog attributes
|
||||
// and logs at the appropriate level based on the "level" key if present.
|
||||
func (a *KitlogAdapter) Log(keyvals ...any) error {
|
||||
if len(keyvals) == 0 && len(a.attrs) == 0 {
|
||||
func (a *Logger) Log(keyvals ...any) error {
|
||||
if len(keyvals) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Combine pre-set attrs with new keyvals
|
||||
allKeyvals := slices.Concat(a.attrs, keyvals)
|
||||
|
||||
// Extract level and message from keyvals
|
||||
level := slog.LevelInfo
|
||||
msg := ""
|
||||
attrs := make([]slog.Attr, 0, len(allKeyvals)/2)
|
||||
attrs := make([]slog.Attr, 0, len(keyvals)/2)
|
||||
|
||||
for i := 0; i < len(allKeyvals)-1; i += 2 {
|
||||
key, ok := allKeyvals[i].(string)
|
||||
for i := 0; i < len(keyvals)-1; i += 2 {
|
||||
key, ok := keyvals[i].(string)
|
||||
if !ok {
|
||||
// If key isn't a string, skip this pair
|
||||
continue
|
||||
}
|
||||
val := allKeyvals[i+1]
|
||||
val := keyvals[i+1]
|
||||
|
||||
switch key {
|
||||
case "level":
|
||||
@@ -68,10 +63,11 @@ func (a *KitlogAdapter) Log(keyvals ...any) error {
|
||||
}
|
||||
|
||||
// With returns a new logger with the given key-value pairs added to every log.
|
||||
func (a *KitlogAdapter) With(keyvals ...any) kitlog.Logger {
|
||||
return &KitlogAdapter{
|
||||
logger: a.logger,
|
||||
attrs: slices.Concat(a.attrs, keyvals),
|
||||
// It returns *Logger (not kitlog.Logger) to preserve type information,
|
||||
// allowing callers to access SlogLogger() without type assertions.
|
||||
func (a *Logger) With(keyvals ...any) *Logger {
|
||||
return &Logger{
|
||||
logger: a.logger.With(keyvals...),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -104,5 +100,11 @@ func kitlogLevelToSlog(val any) slog.Level {
|
||||
}
|
||||
}
|
||||
|
||||
// Ensure KitlogAdapter implements kitlog.Logger at compile time.
|
||||
var _ kitlog.Logger = (*KitlogAdapter)(nil)
|
||||
// SlogLogger returns the underlying slog.Logger.
|
||||
// This is useful when migrating code from kitlog to slog.
|
||||
func (a *Logger) SlogLogger() *slog.Logger {
|
||||
return a.logger
|
||||
}
|
||||
|
||||
// Ensure Logger implements kitlog.Logger at compile time.
|
||||
var _ kitlog.Logger = (*Logger)(nil)
|
||||
|
||||
@@ -17,7 +17,7 @@ func newTestAdapter(t *testing.T) (*testutils.TestHandler, kitlog.Logger) {
|
||||
t.Helper()
|
||||
handler := testutils.NewTestHandler()
|
||||
slogLogger := slog.New(handler)
|
||||
return handler, NewKitlogAdapter(slogLogger)
|
||||
return handler, NewLogger(slogLogger)
|
||||
}
|
||||
|
||||
func TestKitlogAdapter(t *testing.T) {
|
||||
@@ -42,8 +42,8 @@ func TestKitlogAdapter(t *testing.T) {
|
||||
t.Parallel()
|
||||
handler, adapter := newTestAdapter(t)
|
||||
|
||||
kitlogAdapter, ok := adapter.(*KitlogAdapter)
|
||||
require.True(t, ok, "adapter should be *KitlogAdapter")
|
||||
kitlogAdapter, ok := adapter.(*Logger)
|
||||
require.True(t, ok, "adapter should be *Logger")
|
||||
|
||||
contextLogger := kitlogAdapter.With("component", "test-component")
|
||||
err := contextLogger.Log("msg", "message with context")
|
||||
|
||||
@@ -148,3 +148,32 @@ func (h *OtelTracingHandler) WithGroup(name string) slog.Handler {
|
||||
|
||||
// Ensure OtelTracingHandler implements slog.Handler at compile time.
|
||||
var _ slog.Handler = (*OtelTracingHandler)(nil)
|
||||
|
||||
// DiscardHandler is a slog.Handler that discards all log records.
|
||||
type DiscardHandler struct{}
|
||||
|
||||
func (DiscardHandler) Enabled(context.Context, slog.Level) bool { return false }
|
||||
func (DiscardHandler) Handle(context.Context, slog.Record) error { return nil }
|
||||
func (d DiscardHandler) WithAttrs([]slog.Attr) slog.Handler { return d }
|
||||
func (d DiscardHandler) WithGroup(string) slog.Handler { return d }
|
||||
|
||||
// Ensure DiscardHandler implements slog.Handler at compile time.
|
||||
var _ slog.Handler = DiscardHandler{}
|
||||
|
||||
// NewNopLogger returns a no-op *Logger that discards all log output.
|
||||
// Use this in tests instead of kitlog.NewNopLogger() to maintain type safety.
|
||||
func NewNopLogger() *Logger {
|
||||
return NewLogger(slog.New(DiscardHandler{}))
|
||||
}
|
||||
|
||||
// NewLogfmtLogger creates a *Logger that outputs text-formatted logs to the given writer.
|
||||
// This is a drop-in replacement for kitlog.NewLogfmtLogger().
|
||||
func NewLogfmtLogger(output io.Writer) *Logger {
|
||||
return NewLogger(NewSlogLogger(Options{Output: output, Debug: true}))
|
||||
}
|
||||
|
||||
// NewJSONLogger creates a *Logger that outputs JSON-formatted logs to the given writer.
|
||||
// This is a drop-in replacement for kitlog.NewJSONLogger().
|
||||
func NewJSONLogger(output io.Writer) *Logger {
|
||||
return NewLogger(NewSlogLogger(Options{Output: output, JSON: true, Debug: true}))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user