Skip to content

feat(go): abstract logging behind an interface to avoid forcing slog on callers #494

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.

Sign up for GitHub

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

Open
lbcjbb wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(go): abstract logging behind an interface to avoid forcing slog on callers #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 3 additions & 25 deletions openfeature-provider/go/confidence/flag_logger_e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ package confidence

import (
"context"
"log/slog"
"os"
"strings"
"sync"
"testing"
"time"

Expand All @@ -24,10 +22,8 @@ const flagLogsTargetingKey = "test-a"
func TestFlagLogs_ShouldSuccessfullySendToRealBackend(t *testing.T) {
ctx := context.Background()

// Create a custom logger that captures log messages (Debug level to capture all logs)
var logBuffer logCaptureBuffer
captureHandler := slog.NewTextHandler(&logBuffer, &slog.HandlerOptions{Level: slog.LevelDebug})
logger := slog.New(captureHandler)
// Create a custom logger that captures log messages
logger := newRecorderLogger()

// Create a real provider with real gRPC connection
provider, err := NewProvider(ctx, ProviderConfig{
Expand Down Expand Up @@ -64,7 +60,7 @@ func TestFlagLogs_ShouldSuccessfullySendToRealBackend(t *testing.T) {
time.Sleep(200 * time.Millisecond)

// Verify the logs contain success message and no errors
logs := logBuffer.String()
logs := logger.String()
if strings.Contains(logs, "Failed to write flag logs") {
t.Errorf("Backend returned error - found 'Failed to write flag logs' in logs:\n%s", logs)
}
Expand All @@ -74,21 +70,3 @@ func TestFlagLogs_ShouldSuccessfullySendToRealBackend(t *testing.T) {

t.Log("Successfully sent WriteFlagLogs to real Confidence backend")
}

// logCaptureBuffer is a thread-safe buffer for capturing log output
type logCaptureBuffer struct {
mu sync.Mutex
buf strings.Builder
}

func (b *logCaptureBuffer) Write(p []byte) (n int, err error) {
b.mu.Lock()
defer b.mu.Unlock()
return b.buf.Write(p)
}

func (b *logCaptureBuffer) String() string {
b.mu.Lock()
defer b.mu.Unlock()
return b.buf.String()
}
9 changes: 4 additions & 5 deletions openfeature-provider/go/confidence/flag_logs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@ package confidence

import (
"context"
"log/slog"
"os"
"strings"
"testing"
"time"

"github.com/open-feature/go-sdk/openfeature"

fl "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/internal/flag_logger"
lr "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/internal/local_resolver"
resolverv1 "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/internal/proto/resolverinternal"

"github.com/open-feature/go-sdk/openfeature"
)

// Unit tests that verify WriteFlagLogs contains correct flag assignment data.
Expand All @@ -37,7 +36,7 @@ func setupFlagLogsUnitTest(t *testing.T) (*fl.CapturingFlagLogger, openfeature.I
capturingLogger := fl.NewCapturingFlagLogger()

// Create state provider that fetches from real Confidence service
logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelInfo}))
logger := newLoggerForTest(t)
stateProvider := NewFlagsAdminStateFetcher(unitTestClientSecret, logger)

// Fetch initial state
Expand All @@ -49,7 +48,7 @@ func setupFlagLogsUnitTest(t *testing.T) (*fl.CapturingFlagLogger, openfeature.I
unsupportedMatStore := newUnsupportedMaterializationStore()

resolverSupplier := wrapResolverSupplierWithMaterializations(func(ctx context.Context, logSink lr.LogSink) lr.LocalResolver {
return lr.NewLocalResolverWithPoolSize(ctx, logSink, 2)
return lr.NewLocalResolverWithPoolSize(ctx, logSink, logger, 2)
}, unsupportedMatStore)
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, capturingLogger, unitTestClientSecret, logger)

Expand Down
9 changes: 4 additions & 5 deletions openfeature-provider/go/confidence/inmemory_materialization_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package confidence

import (
"context"
"log/slog"
"sync"
)

Expand Down Expand Up @@ -30,7 +29,7 @@ type inMemoryMaterializationStore struct {
// storage: unit -> materialization -> data
storage map[string]map[string]*materializationData
mu sync.RWMutex
logger *slog.Logger
logger Logger
// call tracking for tests
readCalls [][]ReadOp
writeCalls [][]WriteOp
Expand All @@ -42,9 +41,9 @@ type materializationData struct {
}

// newInMemoryMaterializationStore creates a new in-memory materialization store.
func newInMemoryMaterializationStore(logger *slog.Logger) *inMemoryMaterializationStore {
func newInMemoryMaterializationStore(logger Logger) *inMemoryMaterializationStore {
if logger == nil {
logger = slog.Default()
logger = &noopLogger{}
}
return &inMemoryMaterializationStore{
storage: make(map[string]map[string]*materializationData),
Expand All @@ -55,7 +54,7 @@ func newInMemoryMaterializationStore(logger *slog.Logger) *inMemoryMaterializati
// newInMemoryMaterializationStoreWithInclusions creates a store pre-populated with inclusion data.
// This is useful for testing materialized segment criterion evaluation.
// The initialInclusions map structure is: unit -> materialization -> included
func newInMemoryMaterializationStoreWithInclusions(logger *slog.Logger, initialInclusions map[string]map[string]bool) *inMemoryMaterializationStore {
func newInMemoryMaterializationStoreWithInclusions(logger Logger, initialInclusions map[string]map[string]bool) *inMemoryMaterializationStore {
store := newInMemoryMaterializationStore(logger)

store.mu.Lock()
Expand Down
53 changes: 26 additions & 27 deletions openfeature-provider/go/confidence/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package confidence

import (
"context"
"log/slog"
"os"
"strings"
"sync"
"sync/atomic"
Expand All @@ -13,11 +11,12 @@ import (
"github.com/open-feature/go-sdk/openfeature"
"github.com/prometheus/common/expfmt"
"github.com/prometheus/common/model"
"google.golang.org/grpc"

fl "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/internal/flag_logger"
lr "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/internal/local_resolver"
resolverv1 "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/internal/proto/resolverinternal"
tu "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/internal/testutil"
"google.golang.org/grpc"
)

// mockStateProvider provides test state for integration testing
Expand Down Expand Up @@ -100,7 +99,6 @@ func (m *mockGrpcStubForIntegration) GetCallsReceived() int32 {
func TestIntegration_OpenFeatureShutdownFlushesLogs(t *testing.T) {
// Load test state
testState := tu.LoadTestResolverState(t)
accountID := tu.LoadTestAccountID(t)

ctx := context.Background()

Expand All @@ -113,15 +111,15 @@ func TestIntegration_OpenFeatureShutdownFlushesLogs(t *testing.T) {
mockStub := &mockGrpcStubForIntegration{
onCallReceived: make(chan struct{}, 100), // Buffer to prevent blocking
}
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, "test-client-secret", slog.New(slog.NewTextHandler(os.Stderr, nil)))
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, "test-client-secret", newLoggerForTest(t))

trackingLogger := &trackingFlagLogger{
actualLogger: actualGrpcLogger,
lastWriteCompleted: make(chan struct{}, 1),
}

// Create provider with test state
provider, err := createProviderWithTestState(ctx, stateProvider, accountID, trackingLogger, newUnsupportedMaterializationStore())
provider, err := createProviderWithTestState(t, stateProvider, trackingLogger, newUnsupportedMaterializationStore())
if err != nil {
t.Fatalf("Failed to create provider: %v", err)
}
Expand Down Expand Up @@ -197,19 +195,19 @@ func TestIntegration_OpenFeatureResolveStickyFlagMatStoreReadAndWrite(t *testing
mockStub := &mockGrpcStubForIntegration{
onCallReceived: make(chan struct{}, 100), // Buffer to prevent blocking
}
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, "test-secret", slog.New(slog.NewTextHandler(os.Stderr, nil)))
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, "test-secret", newLoggerForTest(t))

trackingLogger := &trackingFlagLogger{
actualLogger: actualGrpcLogger,
lastWriteCompleted: make(chan struct{}, 1),
}
matStore := newInMemoryMaterializationStore(nil)
resolverSupplier := wrapResolverSupplierWithMaterializations(func(ctx context.Context, logSink lr.LogSink) lr.LocalResolver {
return lr.NewLocalResolverWithPoolSize(ctx, logSink, 2)
return lr.NewLocalResolverWithPoolSize(ctx, logSink, newLoggerForTest(t), 2)
}, matStore)

// Create provider with test state
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, trackingLogger, "test-secret", slog.New(slog.NewTextHandler(os.Stderr, nil)))
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, trackingLogger, "test-secret", newLoggerForTest(t))

client := openfeature.NewClient("integration-test")

Expand Down Expand Up @@ -320,7 +318,7 @@ func TestIntegration_OpenFeatureMaterializedSegmentCriterion(t *testing.T) {
mockStub := &mockGrpcStubForIntegration{
onCallReceived: make(chan struct{}, 100),
}
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, SECRET, slog.New(slog.NewTextHandler(os.Stderr, nil)))
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, SECRET, newLoggerForTest(t))

trackingLogger := &trackingFlagLogger{
actualLogger: actualGrpcLogger,
Expand All @@ -336,11 +334,11 @@ func TestIntegration_OpenFeatureMaterializedSegmentCriterion(t *testing.T) {
}
matStore := newInMemoryMaterializationStoreWithInclusions(nil, initialInclusions)
resolverSupplier := wrapResolverSupplierWithMaterializations(func(ctx context.Context, logSink lr.LogSink) lr.LocalResolver {
return lr.NewLocalResolverWithPoolSize(ctx, logSink, 2)
return lr.NewLocalResolverWithPoolSize(ctx, logSink, newLoggerForTest(t), 2)
}, matStore)

// Create provider with test state
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, trackingLogger, SECRET, slog.New(slog.NewTextHandler(os.Stderr, nil)))
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, trackingLogger, SECRET, newLoggerForTest(t))

client := openfeature.NewClient("integration-test-mat-seg")

Expand Down Expand Up @@ -398,11 +396,11 @@ func TestIntegration_OpenFeatureMaterializedSegmentCriterion(t *testing.T) {
}
matStore := newInMemoryMaterializationStoreWithInclusions(nil, initialInclusions)
resolverSupplier := wrapResolverSupplierWithMaterializations(func(ctx context.Context, logSink lr.LogSink) lr.LocalResolver {
return lr.NewLocalResolverWithPoolSize(ctx, logSink, 2)
return lr.NewLocalResolverWithPoolSize(ctx, logSink, newLoggerForTest(t), 2)
}, matStore)

// Create provider with test state
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, trackingLogger, SECRET, slog.New(slog.NewTextHandler(os.Stderr, nil)))
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, trackingLogger, SECRET, newLoggerForTest(t))

client := openfeature.NewClient("integration-test-mat-seg-not-in")

Expand Down Expand Up @@ -439,11 +437,11 @@ func TestIntegration_OpenFeatureMaterializedSegmentCriterion(t *testing.T) {
// Create empty materialization store (no context for tutorial_visitor)
matStore := newInMemoryMaterializationStore(nil)
resolverSupplier := wrapResolverSupplierWithMaterializations(func(ctx context.Context, logSink lr.LogSink) lr.LocalResolver {
return lr.NewLocalResolverWithPoolSize(ctx, logSink, 2)
return lr.NewLocalResolverWithPoolSize(ctx, logSink, newLoggerForTest(t), 2)
}, matStore)

// Create provider with test state
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, trackingLogger, SECRET, slog.New(slog.NewTextHandler(os.Stderr, nil)))
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, trackingLogger, SECRET, newLoggerForTest(t))

client := openfeature.NewClient("integration-test-mat-seg-no-ctx")

Expand Down Expand Up @@ -483,7 +481,6 @@ func TestIntegration_OpenFeatureMaterializedSegmentCriterion(t *testing.T) {
func TestIntegration_OpenFeatureObjectWithStructDefault(t *testing.T) {
// Load test state
testState := tu.LoadTestResolverState(t)
accountID := tu.LoadTestAccountID(t)

ctx := context.Background()

Expand All @@ -496,15 +493,15 @@ func TestIntegration_OpenFeatureObjectWithStructDefault(t *testing.T) {
mockStub := &mockGrpcStubForIntegration{
onCallReceived: make(chan struct{}, 100),
}
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, "mkjJruAATQWjeY7foFIWfVAcBWnci2YF", slog.New(slog.NewTextHandler(os.Stderr, nil)))
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, "mkjJruAATQWjeY7foFIWfVAcBWnci2YF", newLoggerForTest(t))

trackingLogger := &trackingFlagLogger{
actualLogger: actualGrpcLogger,
lastWriteCompleted: make(chan struct{}, 1),
}

// Create provider with test state
provider, err := createProviderWithTestState(ctx, stateProvider, accountID, trackingLogger, newUnsupportedMaterializationStore())
provider, err := createProviderWithTestState(t, stateProvider, trackingLogger, newUnsupportedMaterializationStore())
if err != nil {
t.Fatalf("Failed to create provider: %v", err)
}
Expand Down Expand Up @@ -691,7 +688,6 @@ func TestIntegration_OpenFeatureObjectWithStructDefault(t *testing.T) {
func TestIntegration_GetPrometheusMetrics(t *testing.T) {
// Load test state
testState := tu.LoadTestResolverState(t)
accountID := tu.LoadTestAccountID(t)

ctx := context.Background()

Expand All @@ -704,15 +700,15 @@ func TestIntegration_GetPrometheusMetrics(t *testing.T) {
mockStub := &mockGrpcStubForIntegration{
onCallReceived: make(chan struct{}, 100),
}
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, "mkjJruAATQWjeY7foFIWfVAcBWnci2YF", slog.New(slog.NewTextHandler(os.Stderr, nil)))
actualGrpcLogger := fl.NewGrpcWasmFlagLogger(mockStub, "mkjJruAATQWjeY7foFIWfVAcBWnci2YF", newLoggerForTest(t))

trackingLogger := &trackingFlagLogger{
actualLogger: actualGrpcLogger,
lastWriteCompleted: make(chan struct{}, 1),
}

// Create provider with test state
provider, err := createProviderWithTestState(ctx, stateProvider, accountID, trackingLogger, newUnsupportedMaterializationStore())
provider, err := createProviderWithTestState(t, stateProvider, trackingLogger, newUnsupportedMaterializationStore())
if err != nil {
t.Fatalf("Failed to create provider: %v", err)
}
Expand Down Expand Up @@ -808,17 +804,20 @@ func TestIntegration_GetPrometheusMetrics(t *testing.T) {

// createProviderWithTestState creates a provider with mock state provider and tracking logger
func createProviderWithTestState(
ctx context.Context,
tb testing.TB,
stateProvider StateProvider,
accountID string,
logger FlagLogger,
fl FlagLogger,
matStore MaterializationStore,
) (*LocalResolverProvider, error) {
logger := newLoggerForTest(tb)

resolverSupplier := wrapResolverSupplierWithMaterializations(func(ctx context.Context, logSink lr.LogSink) lr.LocalResolver {
return lr.NewLocalResolverWithPoolSize(ctx, logSink, 2)
return lr.NewLocalResolverWithPoolSize(ctx, logSink, logger, 2)
}, matStore)

// Create provider with the client secret from test state
// The test state includes client secret: mkjJruAATQWjeY7foFIWfVAcBWnci2YF
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, logger, "mkjJruAATQWjeY7foFIWfVAcBWnci2YF", slog.New(slog.NewTextHandler(os.Stderr, nil)))
provider := NewLocalResolverProvider(resolverSupplier, stateProvider, fl, "mkjJruAATQWjeY7foFIWfVAcBWnci2YF", logger)

return provider, nil
}
8 changes: 4 additions & 4 deletions openfeature-provider/go/confidence/internal/flag_logger/grpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,25 @@ package flag_logger
import (
"context"
"fmt"
"log/slog"
"sync"
"sync/atomic"
"time"

resolverv1 "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/internal/proto/resolverinternal"
"google.golang.org/grpc/metadata"

resolverv1 "github.com/spotify/confidence-resolver/openfeature-provider/go/confidence/internal/proto/resolverinternal"
)

type GrpcFlagLogger struct {
stub resolverv1.InternalFlagLoggerServiceClient
clientSecret string
logger *slog.Logger
logger Logger
wg sync.WaitGroup
attempts atomic.Int64
failures atomic.Int64
}

func NewGrpcWasmFlagLogger(stub resolverv1.InternalFlagLoggerServiceClient, clientSecret string, logger *slog.Logger) *GrpcFlagLogger {
func NewGrpcWasmFlagLogger(stub resolverv1.InternalFlagLoggerServiceClient, clientSecret string, logger Logger) *GrpcFlagLogger {
return &GrpcFlagLogger{
stub: stub,
clientSecret: clientSecret,
Expand Down
Loading