Skip to content
Closed
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
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,16 @@ stopped by `Manager.Close()`.
- `Manager` — entry point; created with `New(cfg, opts...)`.
- `Registry` — interface for the Consul backend; can be replaced by an in-memory stub in tests.
- `Service` / `HealthCheck` / `Event` — domain types.
- `Logger` — minimal structured-logging interface the library needs. It mirrors
the context-aware methods of the stdlib `*slog.Logger`
(`InfoContext`/`WarnContext`/`ErrorContext`/`DebugContext`), so any structured
logger can be injected without coupling the consumer to a specific
lib-observability version. Use `libsd.NewNopLogger()` to disable logging.

**Functional options:**

```go
libsd.WithLogger(logger) // inject a lib-commons log.Logger
libsd.WithLogger(logger) // inject any logger implementing libsd.Logger (e.g. *slog.Logger)
Comment on lines +70 to +79

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document the breaking logger migration.

The README explains the new interface but does not warn that existing lib-observability/log.Logger values no longer satisfy libsd.Logger. Add a short migration note directing callers to *slog.Logger or an adapter.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 70 - 79, Add a concise migration note near the README
logger interface and functional-options documentation stating that existing
lib-observability/log.Logger values no longer implement libsd.Logger. Direct
callers to provide a *slog.Logger or use an adapter, while preserving the
existing injection and NewNopLogger guidance.

```

## Usage
Expand Down
2 changes: 1 addition & 1 deletion close_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func TestManagerClose_RegistryWithoutCloser(t *testing.T) {
func TestManagerClose_DisabledManagerSafe(t *testing.T) {
t.Parallel()

m, err := New(Config{Enabled: false, Logger: log.NewNop()})
m, err := New(Config{Enabled: false, Logger: NewNopLogger()})
require.NoError(t, err)

assert.NotPanics(t, func() {
Expand Down
9 changes: 4 additions & 5 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
"strconv"
"strings"
"time"

"github.com/LerianStudio/lib-observability/log"
)

const (
Expand Down Expand Up @@ -171,8 +169,9 @@ type Config struct {
AllowStale *bool

// Logger receives structured log output from the Manager and registry.
// Defaults to log.NewNop() when nil.
Logger log.Logger
// Any structured logger satisfying the minimal Logger interface is accepted
// (for example *slog.Logger). Defaults to a no-op logger when nil.
Logger Logger
}

// ConfigFromEnv returns a Config populated from environment variables.
Expand Down Expand Up @@ -251,7 +250,7 @@ func (c Config) Validate() error {

func (c Config) withDefaults() Config {
if c.Logger == nil {
c.Logger = log.NewNop()
c.Logger = NewNopLogger()
}

if c.ConsulAddr == "" {
Expand Down
5 changes: 2 additions & 3 deletions consul_dual_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"testing"
"time"

"github.com/LerianStudio/lib-observability/log"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -42,7 +41,7 @@ func integrationManagerDual(t *testing.T, internalPort int, preferView EndpointV
AdvertiseInternalAddr: advertiseAddr, // internal view — real/reachable (host.docker.internal)
AdvertiseInternalPort: internalPort,
PreferView: preferView,
Logger: log.NewNop(),
Logger: NewNopLogger(),
})
require.NoError(t, err)

Expand Down Expand Up @@ -271,7 +270,7 @@ func integrationManagerInternalOnly(t *testing.T, internalPort int) *Manager {
AdvertiseInternalAddr: advertiseAddr, // internal view — real/reachable
AdvertiseInternalPort: internalPort,
// AdvertiseAddr intentionally empty: no external endpoint is advertised.
Logger: log.NewNop(),
Logger: NewNopLogger(),
})
require.NoError(t, err)

Expand Down
5 changes: 2 additions & 3 deletions consul_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"testing"
"time"

"github.com/LerianStudio/lib-observability/log"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand All @@ -30,7 +29,7 @@ func integrationManager(t *testing.T) *Manager {
Enabled: true,
ConsulAddr: consulAddr,
AdvertiseAddr: advertiseAddr,
Logger: log.NewNop(),
Logger: NewNopLogger(),
})
require.NoError(t, err)

Expand Down Expand Up @@ -269,7 +268,7 @@ func TestIntegration_ResolveHonorsDialTimeout(t *testing.T) {
ConsulAddr: "10.255.255.1:8500", // blackhole: SYNs are dropped, dial hangs until deadline
AdvertiseAddr: advertiseAddr,
DialTimeout: dialTimeout,
Logger: log.NewNop(),
Logger: NewNopLogger(),
})
require.NoError(t, err)
t.Cleanup(func() { _ = m.Close() })
Expand Down
107 changes: 107 additions & 0 deletions logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package libsd

import (
"context"

"github.com/LerianStudio/lib-observability/log"
)

// Logger is the minimal structured-logging interface required by
// lib-service-discovery.
//
// It intentionally mirrors the context-aware methods of the standard library's
// *slog.Logger, so a consumer can supply any structured logger — including
// *slog.Logger — without being forced to import and version-match a specific
// logging library. This keeps the lib-observability logging types out of the
// public API and avoids the type incompatibilities that arise when a consumer
// pins a different lib-observability version than this library does.
//
// Any value implementing these four methods satisfies the interface via Go's
// structural typing, so no concrete adapter type is required on the caller side.
type Logger interface {
InfoContext(ctx context.Context, msg string, args ...any)
WarnContext(ctx context.Context, msg string, args ...any)
ErrorContext(ctx context.Context, msg string, args ...any)
DebugContext(ctx context.Context, msg string, args ...any)
}

// nopLogger is a Logger that discards everything.
type nopLogger struct{}

func (nopLogger) InfoContext(context.Context, string, ...any) {}
func (nopLogger) WarnContext(context.Context, string, ...any) {}
func (nopLogger) ErrorContext(context.Context, string, ...any) {}
func (nopLogger) DebugContext(context.Context, string, ...any) {}

// NewNopLogger returns a Logger that discards all output. It is the default used
// when no logger is supplied via Config.Logger or WithLogger.
func NewNopLogger() Logger { return nopLogger{} }

// observabilityAdapter adapts a public Logger to the internal lib-observability
// log.Logger consumed by the registry and resolvers. It lets the internal
// logging call sites keep using the log.Log(ctx, level, msg, fields...) style
// while the public API only ever exposes the version-agnostic Logger interface.
type observabilityAdapter struct {
inner Logger
fields []log.Field
}

// toInternalLogger wraps a public Logger in the internal log.Logger contract.
// A nil Logger yields a lib-observability no-op logger.
func toInternalLogger(l Logger) log.Logger {
if l == nil {
return log.NewNop()
}

return &observabilityAdapter{inner: l}
Comment on lines +51 to +56

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Files ==\n'
git ls-files | rg '(^|/)(logger\.go|config\.go)$'

printf '\n== logger.go outline ==\n'
ast-grep outline logger.go --view expanded || true

printf '\n== config.go outline ==\n'
ast-grep outline config.go --view expanded || true

printf '\n== Relevant lines: logger.go ==\n'
nl -ba logger.go | sed -n '1,140p'

printf '\n== Relevant lines: config.go ==\n'
nl -ba config.go | sed -n '220,290p'

printf '\n== Search for Logger type and related helpers ==\n'
rg -n "type Logger|NewNopLogger|toInternalLogger|withDefaults|observabilityAdapter|isNil" .

Repository: LerianStudio/lib-service-discovery

Length of output: 3092


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Fallback if the previous request didn't return results yet: inspect the files directly.
printf '\n== logger.go ==\n'
nl -ba logger.go | sed -n '1,140p'

printf '\n== config.go ==\n'
nl -ba config.go | sed -n '220,290p'

printf '\n== Logger-related definitions ==\n'
rg -n "type Logger|func .*NewNopLogger|func .*toInternalLogger|func .*withDefaults|observabilityAdapter|isNil" logger.go config.go

Repository: LerianStudio/lib-service-discovery

Length of output: 229


🏁 Script executed:

#!/bin/bash
set -euo pipefail

pwd
git rev-parse --show-toplevel
git ls-files | head -n 50

Repository: LerianStudio/lib-service-discovery

Length of output: 1146


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== logger.go ==\n'
sed -n '1,140p' logger.go

printf '\n== config.go (selected) ==\n'
sed -n '230,280p' config.go

printf '\n== Logger usages and constructors ==\n'
grep -RIn --exclude-dir=.git -E 'Logger|NewNopLogger|toInternalLogger|withDefaults|observabilityAdapter' . | head -n 200

Repository: LerianStudio/lib-service-discovery

Length of output: 16857


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== logger.go ==\n'
sed -n '1,140p' logger.go

printf '\n== config.go (selected) ==\n'
sed -n '230,280p' config.go

printf '\n== Logger-related call sites ==\n'
grep -RIn --exclude-dir=.git -E 'NewNopLogger\(|toInternalLogger\(|withDefaults\(|Logger\s*[:=]' . | head -n 200

Repository: LerianStudio/lib-service-discovery

Length of output: 8852


Use a typed-nil-safe logger fallback.
l == nil and c.Logger == nil miss interface values that wrap nil pointers, so a logger can slip through withDefaults() and later panic when observabilityAdapter.Log calls through it. Add one shared nil-like check and use it in both logger.go#L51-L56 and config.go#L251-L254.

📍 Affects 2 files
  • logger.go#L51-L56 (this comment)
  • config.go#L251-L254
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@logger.go` around lines 51 - 56, Use one shared typed-nil-safe logger check
in logger.go at toInternalLogger and config.go at withDefaults: detect nil
interfaces and interfaces containing nil pointers, and apply the check in both
locations so nil-like loggers consistently fall back to the no-op logger.

}

// Log routes an internal log event to the appropriate context-aware method of
// the wrapped Logger, flattening the typed fields into slog-style key/value
// arguments.
func (a *observabilityAdapter) Log(ctx context.Context, level log.Level, msg string, fields ...log.Field) {
all := fields
if len(a.fields) > 0 {
all = make([]log.Field, 0, len(a.fields)+len(fields))
all = append(all, a.fields...)
all = append(all, fields...)
}

args := make([]any, 0, len(all)*2)
for _, f := range all {
args = append(args, f.Key, f.Value)
}

switch level {
case log.LevelError:
a.inner.ErrorContext(ctx, msg, args...)
case log.LevelWarn:
a.inner.WarnContext(ctx, msg, args...)
case log.LevelDebug:
a.inner.DebugContext(ctx, msg, args...)
default: // LevelInfo and any unknown level map to Info.
a.inner.InfoContext(ctx, msg, args...)
}
}

// With returns a child adapter carrying additional persistent fields.
//
//nolint:ireturn // must satisfy the lib-observability log.Logger contract.
func (a *observabilityAdapter) With(fields ...log.Field) log.Logger {
merged := make([]log.Field, 0, len(a.fields)+len(fields))
merged = append(merged, a.fields...)
merged = append(merged, fields...)

return &observabilityAdapter{inner: a.inner, fields: merged}
}

// WithGroup is a no-op: the public Logger interface has no grouping concept.
//
//nolint:ireturn // must satisfy the lib-observability log.Logger contract.
func (a *observabilityAdapter) WithGroup(_ string) log.Logger { return a }

// Enabled always reports true and defers level filtering to the wrapped Logger.
func (a *observabilityAdapter) Enabled(_ log.Level) bool { return true }

// Sync is a no-op: flushing is the responsibility of the wrapped Logger.
func (a *observabilityAdapter) Sync(_ context.Context) error { return nil }
7 changes: 3 additions & 4 deletions managed_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"testing"
"time"

"github.com/LerianStudio/lib-observability/log"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -160,7 +159,7 @@ func TestManagedResolver_CloseDuringInFlightSeedDoesNotLeak(t *testing.T) {
Enabled: true,
ConsulAddr: "localhost:8500",
AdvertiseAddr: "127.0.0.1",
Logger: log.NewNop(),
Logger: NewNopLogger(),
}, WithRegistry(reg))
require.NoError(t, err)

Expand Down Expand Up @@ -239,7 +238,7 @@ func TestManagedResolver_ResolveAfterCloseDoesNotLeakWatcher(t *testing.T) {
Enabled: true,
ConsulAddr: "localhost:8500",
AdvertiseAddr: "127.0.0.1",
Logger: log.NewNop(),
Logger: NewNopLogger(),
}, WithRegistry(reg))
require.NoError(t, err)

Expand Down Expand Up @@ -558,7 +557,7 @@ func TestManagedResolver_CloseStopsWatchers(t *testing.T) {
Enabled: true,
ConsulAddr: "localhost:8500",
AdvertiseAddr: "127.0.0.1",
Logger: log.NewNop(),
Logger: NewNopLogger(),
}, WithRegistry(reg))
require.NoError(t, err)

Expand Down
8 changes: 4 additions & 4 deletions manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ type Manager struct {
type Option func(*Manager)

// WithLogger sets the structured logger used by the Manager and its registry.
// A nil logger is silently ignored; the Config.Logger (or log.NewNop()) is used instead.
func WithLogger(l log.Logger) Option {
// A nil logger is silently ignored; the Config.Logger (or a no-op logger) is used instead.
func WithLogger(l Logger) Option {
return func(m *Manager) {
if m == nil || l == nil {
return
}

m.logger = l
m.logger = toInternalLogger(l)
}
}

Expand Down Expand Up @@ -125,7 +125,7 @@ func New(cfg Config, opts ...Option) (*Manager, error) {

m := &Manager{
config: cfg,
logger: cfg.Logger,
logger: toInternalLogger(cfg.Logger),
workload: cfg.Workload,
preferView: cfg.PreferView,
seedTimeout: cfg.SeedTimeout,
Expand Down
Loading
Loading