Skip to content

refactor: WIP: #1324 Slog refactor #1802

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
djosephsen wants to merge 6 commits into
base: main
Choose a base branch
from
Open

refactor: WIP: #1324 Slog refactor #1802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2cad4a6
refactor: pkg/logger uses slog or zap
djosephsen Sep 19, 2025
ea7bc64
Co-contributer
aharab Sep 19, 2025
f56e57a
fix: review-comment; fix logger.new invocations in pre-existing tests
djosephsen Feb 27, 2026
e850a87
fix: review-comment; badness in makefields()
djosephsen Feb 27, 2026
2a1d573
fix: add tests for zaplog implementation
djosephsen Feb 27, 2026
12f676b
Fixing the WithID problem, some cases where we'd potentially drop att…
aharab Feb 27, 2026
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
4 changes: 2 additions & 2 deletions core/pkg/evaluator/fractional.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
const FractionEvaluationName = "fractional"

type Fractional struct {
Logger *logger.Logger
Logger logger.Logger
}

type fractionalEvaluationDistribution struct {
Expand All @@ -33,7 +33,7 @@ func (v fractionalEvaluationVariant) getPercentage(totalWeight int) float64 {
return 100 * float64(v.weight) / float64(totalWeight)
}

func NewFractional(logger *logger.Logger) *Fractional {
func NewFractional(logger logger.Logger) *Fractional {
return &Fractional{Logger: logger}
}

Expand Down
10 changes: 5 additions & 5 deletions core/pkg/evaluator/fractional_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (

func TestFractionalEvaluation(t *testing.T) {
const source = "testSource"
var sources = []string{source}
sources := []string{source}
ctx := context.Background()

commonFlags := map[string]model.Flag{
Expand Down Expand Up @@ -377,7 +377,7 @@ func TestFractionalEvaluation(t *testing.T) {
"headerColor": {
State: "ENABLED",
DefaultVariant: "red",
Variants: colorVariants,
Variants: colorVariants,
Targeting: []byte(
`{
"fractional": [
Expand All @@ -400,7 +400,7 @@ func TestFractionalEvaluation(t *testing.T) {
const reqID = "default"
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
log := logger.NewLogger(nil, false)
log := logger.New("slog", false, "json")
s, err := store.NewStore(log, sources)
if err != nil {
t.Fatalf("NewStore failed: %v", err)
Expand Down Expand Up @@ -435,7 +435,7 @@ func TestFractionalEvaluation(t *testing.T) {

func BenchmarkFractionalEvaluation(b *testing.B) {
const source = "testSource"
var sources = []string{source}
sources := []string{source}
ctx := context.Background()

flags := map[string]model.Flag{
Expand Down Expand Up @@ -529,7 +529,7 @@ func BenchmarkFractionalEvaluation(b *testing.B) {
reqID := "test"
for name, tt := range tests {
b.Run(name, func(b *testing.B) {
log := logger.NewLogger(nil, false)
log := logger.New("slog", false, "json")
s, err := store.NewStore(log, sources)
if err != nil {
b.Fatalf("NewStore failed: %v", err)
Expand Down
13 changes: 6 additions & 7 deletions core/pkg/evaluator/json.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,14 @@ func WithEvaluator(name string, evalFunc func(interface{}, interface{}) interfac
// JSON evaluator
type JSON struct {
store store.IStore
Logger *logger.Logger
Logger logger.Logger
jsonEvalTracer trace.Tracer
jsonSchema *jsonschema.Schema
Resolver
}

func NewJSON(logger *logger.Logger, s store.IStore, opts ...JSONEvaluatorOption) *JSON {
logger = logger.WithFields(
func NewJSON(logger logger.Logger, s store.IStore, opts ...JSONEvaluatorOption) *JSON {
logger = logger.With(
zap.String("component", "evaluator"),
zap.String("evaluator", "json"),
)
Expand Down Expand Up @@ -141,11 +141,11 @@ func (je *JSON) SetState(payload sync.DataSync) error {
// Resolver implementation for flagd flags. This resolver should be kept reusable, hence must interact with interfaces.
type Resolver struct {
store store.IStore
Logger *logger.Logger
Logger logger.Logger
tracer trace.Tracer
}

func NewResolver(store store.IStore, logger *logger.Logger, jsonEvalTracer trace.Tracer) Resolver {
func NewResolver(store store.IStore, logger logger.Logger, jsonEvalTracer trace.Tracer) Resolver {
// register supported json logic custom operator implementations
jsonlogic.AddOperator(FractionEvaluationName, NewFractional(logger).Evaluate)
jsonlogic.AddOperator(StartsWithEvaluationName, NewStringComparisonEvaluator(logger).StartsWithEvaluation)
Expand Down Expand Up @@ -317,7 +317,6 @@ func resolve[T constraints](ctx context.Context, reqID string, key string, conte
func (je *Resolver) evaluateVariant(ctx context.Context, reqID string, flagKey string, evalCtx map[string]any) (
variant string, variants map[string]interface{}, reason string, metadata map[string]interface{}, err error,
) {

var selector store.Selector
s := ctx.Value(store.SelectorContextKey{})
if s != nil {
Expand Down Expand Up @@ -403,7 +402,7 @@ func (je *Resolver) evaluateVariant(ctx context.Context, reqID string, flagKey s
}

func setFlagdProperties(
log *logger.Logger,
log logger.Logger,
context map[string]any,
properties flagdProperties,
) map[string]any {
Expand Down
60 changes: 34 additions & 26 deletions core/pkg/evaluator/json_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/stretchr/testify/require"
"testing"
"time"

"github.com/stretchr/testify/require"

flagdEvaluator "github.com/open-feature/flagd/core/pkg/evaluator"
"github.com/open-feature/flagd/core/pkg/logger"
"github.com/open-feature/flagd/core/pkg/model"
Expand Down Expand Up @@ -380,8 +381,16 @@ var flagConfig = fmt.Sprintf(`{
MetadataFlag,
VersionOverride)

func newTestEvaluator(t testing.TB) *flagdEvaluator.JSON {
t.Helper()
log := logger.New("slog", false, "json")
s, err := store.NewStore(log, []string{"testSource"})
require.NoError(t, err)
return flagdEvaluator.NewJSON(log, s)
}

func TestGetState_Valid_ContainsFlag(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)
err := evaluator.SetState(sync.DataSync{FlagData: ValidFlags, Source: "testSource"})
if err != nil {
t.Fatalf("Expected no error")
Expand All @@ -395,7 +404,7 @@ func TestGetState_Valid_ContainsFlag(t *testing.T) {
}

func TestSetState_Invalid_Error(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)

// set state with an invalid flag definition
err := evaluator.SetState(sync.DataSync{FlagData: InvalidFlags, Source: "testSource"})
Expand All @@ -405,7 +414,7 @@ func TestSetState_Invalid_Error(t *testing.T) {
}

func TestSetState_Valid_NoError(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)

// set state with a valid flag definition
err := evaluator.SetState(sync.DataSync{FlagData: ValidFlags, Source: "testSource"})
Expand All @@ -415,7 +424,7 @@ func TestSetState_Valid_NoError(t *testing.T) {
}

func TestResolveAllValues(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
t.Fatalf("expected no error")
Expand Down Expand Up @@ -484,7 +493,7 @@ func TestResolveBooleanValue(t *testing.T) {
{DisabledFlag, nil, StaticBoolValue, model.ErrorReason, model.FlagDisabledErrorCode},
}
const reqID = "default"
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
t.Fatalf("expected no error")
Expand Down Expand Up @@ -519,7 +528,7 @@ func BenchmarkResolveBooleanValue(b *testing.B) {
{DisabledFlag, nil, StaticBoolValue, model.ErrorReason, model.FlagDisabledErrorCode},
}

evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(b)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
b.Fatalf("expected no error")
Expand Down Expand Up @@ -559,7 +568,7 @@ func TestResolveStringValue(t *testing.T) {
{DisabledFlag, nil, "", model.ErrorReason, model.FlagDisabledErrorCode},
}
const reqID = "default"
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
t.Fatalf("expected no error")
Expand Down Expand Up @@ -595,7 +604,7 @@ func BenchmarkResolveStringValue(b *testing.B) {
{DisabledFlag, nil, "", model.ErrorReason, model.FlagDisabledErrorCode},
}

evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(b)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
b.Fatalf("expected no error")
Expand Down Expand Up @@ -635,7 +644,7 @@ func TestResolveFloatValue(t *testing.T) {
{DisabledFlag, nil, 0, model.ErrorReason, model.FlagDisabledErrorCode},
}
const reqID = "default"
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
t.Fatalf("expected no error")
Expand Down Expand Up @@ -671,7 +680,7 @@ func BenchmarkResolveFloatValue(b *testing.B) {
{DisabledFlag, nil, 0, model.ErrorReason, model.FlagDisabledErrorCode},
}

evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(b)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
b.Fatalf("expected no error")
Expand Down Expand Up @@ -711,7 +720,7 @@ func TestResolveIntValue(t *testing.T) {
{DisabledFlag, nil, 0, model.ErrorReason, model.FlagDisabledErrorCode},
}
const reqID = "default"
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
t.Fatalf("expected no error")
Expand Down Expand Up @@ -747,7 +756,7 @@ func BenchmarkResolveIntValue(b *testing.B) {
{DisabledFlag, nil, 0, model.ErrorReason, model.FlagDisabledErrorCode},
}

evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(b)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
b.Fatalf("expected no error")
Expand Down Expand Up @@ -787,7 +796,7 @@ func TestResolveObjectValue(t *testing.T) {
{DisabledFlag, nil, "{}", model.ErrorReason, model.FlagDisabledErrorCode},
}
const reqID = "default"
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
t.Fatalf("expected no error")
Expand Down Expand Up @@ -826,7 +835,7 @@ func BenchmarkResolveObjectValue(b *testing.B) {
{DisabledFlag, nil, "{}", model.ErrorReason, model.FlagDisabledErrorCode},
}

evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(b)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
b.Fatalf("expected no error")
Expand Down Expand Up @@ -871,7 +880,7 @@ func TestResolveAsAnyValue(t *testing.T) {
{DisabledFlag, nil, "{}", model.ErrorReason, model.FlagDisabledErrorCode},
}

evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)
err := evaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
t.Fatalf("expected no error")
Expand Down Expand Up @@ -907,9 +916,8 @@ func TestResolve_DefaultVariant(t *testing.T) {

for _, test := range tests {
t.Run("", func(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)
err := evaluator.SetState(sync.DataSync{FlagData: test.flags, Source: "testSource"})

if err != nil {
t.Fatalf("expected no error")
}
Expand Down Expand Up @@ -973,7 +981,7 @@ func TestSetState_DefaultVariantValidation(t *testing.T) {

for name, tt := range tests {
t.Run(name, func(t *testing.T) {
jsonEvaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
jsonEvaluator := newTestEvaluator(t)

err := jsonEvaluator.SetState(sync.DataSync{FlagData: tt.jsonFlags, Source: "testSource"})

Expand Down Expand Up @@ -1304,7 +1312,7 @@ func TestState_Evaluator(t *testing.T) {
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
flagStore := store.NewFlags()
jsonEvaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), flagStore)
jsonEvaluator := newTestEvaluator(t)

err := jsonEvaluator.SetState(sync.DataSync{FlagData: tt.inputState, Source: "testSource"})
if err != nil {
Expand Down Expand Up @@ -1422,7 +1430,7 @@ func TestFlagStateSafeForConcurrentReadWrites(t *testing.T) {

for name, tt := range tests {
t.Run(name, func(t *testing.T) {
jsonEvaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
jsonEvaluator := newTestEvaluator(t)

err := jsonEvaluator.SetState(sync.DataSync{FlagData: flagConfig, Source: "testSource"})
if err != nil {
Expand Down Expand Up @@ -1487,7 +1495,7 @@ func TestFlagStateSafeForConcurrentReadWrites(t *testing.T) {

func TestFlagdAmbientProperties(t *testing.T) {
t.Run("flagKeyIsInTheContext", func(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)

err := evaluator.SetState(sync.DataSync{Source: "testSource", FlagData: `{
"flags": {
Expand Down Expand Up @@ -1527,7 +1535,7 @@ func TestFlagdAmbientProperties(t *testing.T) {
})

t.Run("timestampIsInTheContext", func(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)

err := evaluator.SetState(sync.DataSync{Source: "testSource", FlagData: `{
"flags": {
Expand Down Expand Up @@ -1561,7 +1569,7 @@ func TestFlagdAmbientProperties(t *testing.T) {

func TestTargetingVariantBehavior(t *testing.T) {
t.Run("missing variant error", func(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)

err := evaluator.SetState(sync.DataSync{Source: "testSource", FlagData: `{
"flags": {
Expand Down Expand Up @@ -1589,7 +1597,7 @@ func TestTargetingVariantBehavior(t *testing.T) {
})

t.Run("null fallback", func(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)

err := evaluator.SetState(sync.DataSync{Source: "testSource", FlagData: `{
"flags": {
Expand Down Expand Up @@ -1621,7 +1629,7 @@ func TestTargetingVariantBehavior(t *testing.T) {
})

t.Run("match booleans", func(t *testing.T) {
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
evaluator := newTestEvaluator(t)

//nolint:dupword
err := evaluator.SetState(sync.DataSync{Source: "testSource", FlagData: `{
Expand Down
4 changes: 2 additions & 2 deletions core/pkg/evaluator/semver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ func (svo SemVerOperator) compare(v1, v2 string) (bool, error) {
}

type SemVerComparison struct {
Logger *logger.Logger
Logger logger.Logger
}

func NewSemVerComparison(log *logger.Logger) *SemVerComparison {
func NewSemVerComparison(log logger.Logger) *SemVerComparison {
return &SemVerComparison{Logger: log}
}

Expand Down
4 changes: 2 additions & 2 deletions core/pkg/evaluator/semver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func TestSemVerOperator_Compare(t *testing.T) {

func TestJSONEvaluator_semVerEvaluation(t *testing.T) {
const source = "testSource"
var sources = []string{source}
sources := []string{source}
ctx := context.Background()

tests := map[string]struct {
Expand Down Expand Up @@ -790,7 +790,7 @@ func TestJSONEvaluator_semVerEvaluation(t *testing.T) {
const reqID = "default"
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
log := logger.NewLogger(nil, false)
log := logger.New("slog", false, "json")
s, err := store.NewStore(log, sources)
if err != nil {
t.Fatalf("NewStore failed: %v", err)
Expand Down
4 changes: 2 additions & 2 deletions core/pkg/evaluator/string_comparison.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ const (
)

type StringComparisonEvaluator struct {
Logger *logger.Logger
Logger logger.Logger
}

func NewStringComparisonEvaluator(log *logger.Logger) *StringComparisonEvaluator {
func NewStringComparisonEvaluator(log logger.Logger) *StringComparisonEvaluator {
return &StringComparisonEvaluator{Logger: log}
}

Expand Down
Loading