diff --git a/internal/cmd/grpc.go b/internal/cmd/grpc.go index 460f5897c0..739f0778c6 100644 --- a/internal/cmd/grpc.go +++ b/internal/cmd/grpc.go @@ -344,7 +344,7 @@ func NewGRPCServer( register.Add(fliptserver.New(logger, store)) register.Add(metadata.NewServer(cfg, info)) - register.Add(evaluation.NewEvaluateServer(logger, store)) + register.Add(evaluation.New(logger, store)) // initialize grpc server server.Server = grpc.NewServer(grpcOpts...) diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index eb571e3f28..be94ebf2f8 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -3,6 +3,7 @@ package evaluation import ( "context" + errs "go.flipt.io/flipt/errors" fliptotel "go.flipt.io/flipt/internal/server/otel" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" @@ -13,13 +14,24 @@ import ( ) // Variant evaluates a request for a multi-variate flag and entity. -func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.VariantEvaluationResponse, error) { - e.logger.Debug("evaluate", zap.Stringer("request", v)) +func (s *Server) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.VariantEvaluationResponse, error) { + var ver = &rpcEvaluation.VariantEvaluationResponse{} + + flag, err := s.store.GetFlag(ctx, v.NamespaceKey, v.FlagKey) + if err != nil { + return nil, err + } + + if flag.Type != flipt.FlagType_VARIANT_FLAG_TYPE { + return nil, errs.ErrInvalidf("flag type %s invalid", flag.Type) + } + + s.logger.Debug("variant", zap.Stringer("request", v)) if v.NamespaceKey == "" { v.NamespaceKey = storage.DefaultNamespace } - resp, err := e.evaluator.Evaluate(ctx, &flipt.EvaluationRequest{ + resp, err := s.evaluator.Evaluate(ctx, flag, &flipt.EvaluationRequest{ RequestId: v.RequestId, FlagKey: v.FlagKey, EntityId: v.EntityId, @@ -46,18 +58,16 @@ func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.Evaluatio ) } - ver := &rpcEvaluation.VariantEvaluationResponse{ - Match: resp.Match, - SegmentKey: resp.SegmentKey, - Reason: rpcEvaluation.EvaluationReason(resp.Reason), - VariantKey: resp.Value, - VariantAttachment: resp.Attachment, - } + ver.Match = resp.Match + ver.SegmentKey = resp.SegmentKey + ver.Reason = rpcEvaluation.EvaluationReason(resp.Reason) + ver.VariantKey = resp.Value + ver.VariantAttachment = resp.Attachment // add otel attributes to span span := trace.SpanFromContext(ctx) span.SetAttributes(spanAttrs...) - e.logger.Debug("evaluate", zap.Stringer("response", resp)) + s.logger.Debug("variant", zap.Stringer("response", resp)) return ver, nil } diff --git a/internal/server/evaluation/evaluation_server.go b/internal/server/evaluation/evaluation_server.go deleted file mode 100644 index 7f5de179b9..0000000000 --- a/internal/server/evaluation/evaluation_server.go +++ /dev/null @@ -1,30 +0,0 @@ -package evaluation - -import ( - "go.flipt.io/flipt/internal/storage" - rpcEvalution "go.flipt.io/flipt/rpc/flipt/evaluation" - "go.uber.org/zap" - "google.golang.org/grpc" -) - -// EvaluateServer serves the Flipt evaluate v2 gRPC Server. -type EvaluateServer struct { - logger *zap.Logger - store storage.Store - evaluator MultiVariateEvaluator - rpcEvalution.UnimplementedEvaluationServiceServer -} - -// NewEvaluateServer is constructs a new EvaluateServer. -func NewEvaluateServer(logger *zap.Logger, store storage.Store) *EvaluateServer { - return &EvaluateServer{ - logger: logger, - store: store, - evaluator: NewEvaluator(logger, store), - } -} - -// RegisterGRPC registers the EvaluateServer onto the provided gRPC Server. -func (e *EvaluateServer) RegisterGRPC(server *grpc.Server) { - rpcEvalution.RegisterEvaluationServiceServer(server, e) -} diff --git a/internal/server/evaluation/evaluation_test.go b/internal/server/evaluation/evaluation_test.go new file mode 100644 index 0000000000..8c01434ad3 --- /dev/null +++ b/internal/server/evaluation/evaluation_test.go @@ -0,0 +1,170 @@ +package evaluation + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + errs "go.flipt.io/flipt/errors" + "go.flipt.io/flipt/rpc/flipt" + rpcEvaluation "go.flipt.io/flipt/rpc/flipt/evaluation" + "go.uber.org/zap/zaptest" +) + +func TestVariant_FlagNotFound(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) + + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(&flipt.Flag{}, errs.ErrNotFound("test-flag")) + + v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.Nil(t, v) + + assert.EqualError(t, err, "test-flag not found") +} + +func TestVariant_NonVariantFlag(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) + + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(&flipt.Flag{ + NamespaceKey: namespaceKey, + Key: flagKey, + Enabled: true, + Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, + }, nil) + + v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.Nil(t, v) + + assert.EqualError(t, err, "flag type BOOLEAN_FLAG_TYPE invalid") +} + +func TestVariant_EvaluateFailure(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + evaluator = &evaluatorMock{} + logger = zaptest.NewLogger(t) + s = &Server{ + logger: logger, + store: store, + evaluator: evaluator, + } + flag = &flipt.Flag{ + NamespaceKey: namespaceKey, + Key: flagKey, + Enabled: true, + Type: flipt.FlagType_VARIANT_FLAG_TYPE, + } + ) + + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(flag, nil) + + evaluator.On("Evaluate", mock.Anything, flag, &flipt.EvaluationRequest{ + FlagKey: flagKey, + NamespaceKey: namespaceKey, + EntityId: "test-entity", + Context: map[string]string{ + "hello": "world", + }, + }).Return(&flipt.EvaluationResponse{}, errs.ErrInvalid("some error")) + + v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.Nil(t, v) + + assert.EqualError(t, err, "some error") +} + +func TestVariant_Success(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + evaluator = &evaluatorMock{} + logger = zaptest.NewLogger(t) + s = &Server{ + logger: logger, + store: store, + evaluator: evaluator, + } + flag = &flipt.Flag{ + NamespaceKey: namespaceKey, + Key: flagKey, + Enabled: true, + Type: flipt.FlagType_VARIANT_FLAG_TYPE, + } + ) + + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(flag, nil) + + evaluator.On("Evaluate", mock.Anything, flag, &flipt.EvaluationRequest{ + FlagKey: flagKey, + NamespaceKey: namespaceKey, + EntityId: "test-entity", + Context: map[string]string{ + "hello": "world", + }, + }).Return( + &flipt.EvaluationResponse{ + FlagKey: flagKey, + NamespaceKey: namespaceKey, + Value: "foo", + Match: true, + SegmentKey: "segment", + Reason: flipt.EvaluationReason(rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON), + }, nil) + + v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.NoError(t, err) + + assert.Equal(t, true, v.Match) + assert.Equal(t, "foo", v.VariantKey) + assert.Equal(t, "segment", v.SegmentKey) + assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, v.Reason) +} diff --git a/internal/server/evaluation/evaluator.go b/internal/server/evaluation/evaluator.go index 351faf01e4..da2ed97781 100644 --- a/internal/server/evaluation/evaluator.go +++ b/internal/server/evaluation/evaluator.go @@ -2,7 +2,6 @@ package evaluation import ( "context" - "errors" "hash/crc32" "sort" "strconv" @@ -25,7 +24,7 @@ type EvaluationStorer interface { GetEvaluationDistributions(ctx context.Context, ruleID string) ([]*storage.EvaluationDistribution, error) } -// Evaluator is an implementation of the MultiVariateEvaluator. +// Evaluator is responsible for legacy evaluations. type Evaluator struct { logger *zap.Logger store EvaluationStorer @@ -39,12 +38,7 @@ func NewEvaluator(logger *zap.Logger, store EvaluationStorer) *Evaluator { } } -// MultiVariateEvaluator is an abstraction for evaluating a flag against a set of rules for multi-variate flags. -type MultiVariateEvaluator interface { - Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (*flipt.EvaluationResponse, error) -} - -func (e *Evaluator) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (resp *flipt.EvaluationResponse, err error) { +func (e *Evaluator) Evaluate(ctx context.Context, flag *flipt.Flag, r *flipt.EvaluationRequest) (resp *flipt.EvaluationResponse, err error) { var ( startTime = time.Now().UTC() namespaceAttr = metrics.AttributeNamespace.String(r.NamespaceKey) @@ -91,18 +85,6 @@ func (e *Evaluator) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (r NamespaceKey: r.NamespaceKey, } - flag, err := e.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) - if err != nil { - resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON - - var errnf errs.ErrNotFound - if errors.As(err, &errnf) { - resp.Reason = flipt.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON - } - - return resp, err - } - if !flag.Enabled { resp.Match = false resp.Reason = flipt.EvaluationReason_FLAG_DISABLED_EVALUATION_REASON diff --git a/internal/server/evaluation/evaluator_mock.go b/internal/server/evaluation/evaluator_mock.go new file mode 100644 index 0000000000..f57e7464cf --- /dev/null +++ b/internal/server/evaluation/evaluator_mock.go @@ -0,0 +1,21 @@ +package evaluation + +import ( + "context" + + "github.com/stretchr/testify/mock" + "go.flipt.io/flipt/rpc/flipt" +) + +type evaluatorMock struct { + mock.Mock +} + +func (e *evaluatorMock) String() string { + return "mock" +} + +func (e *evaluatorMock) Evaluate(ctx context.Context, flag *flipt.Flag, er *flipt.EvaluationRequest) (*flipt.EvaluationResponse, error) { + args := e.Called(ctx, flag, er) + return args.Get(0).(*flipt.EvaluationResponse), args.Error(1) +} diff --git a/internal/server/evaluation/evaluator_test.go b/internal/server/evaluation/evaluator_test.go index e8649434e4..0f5e744f01 100644 --- a/internal/server/evaluation/evaluator_test.go +++ b/internal/server/evaluation/evaluator_test.go @@ -751,29 +751,6 @@ var ( } ) -func TestEvaluator_FlagNotFound(t *testing.T) { - var ( - store = &evaluationStoreMock{} - logger = zaptest.NewLogger(t) - s = NewEvaluator(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(&flipt.Flag{}, ferrors.ErrNotFoundf("flag %q", "foo")) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ - EntityId: "1", - FlagKey: "foo", - Context: map[string]string{ - "bar": "boz", - }, - }) - - assert.Error(t, err) - assert.EqualError(t, err, "flag \"foo\" not found") - assert.False(t, resp.Match) - assert.Equal(t, flipt.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON, resp.Reason) -} - func TestEvaluator_FlagDisabled(t *testing.T) { var ( store = &evaluationStoreMock{} @@ -781,9 +758,7 @@ func TestEvaluator_FlagDisabled(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(disabledFlag, nil) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + resp, err := s.Evaluate(context.TODO(), disabledFlag, &flipt.EvaluationRequest{ EntityId: "1", FlagKey: "foo", Context: map[string]string{ @@ -803,11 +778,9 @@ func TestEvaluator_FlagNoRules(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return([]*storage.EvaluationRule{}, nil) - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + resp, err := s.Evaluate(context.TODO(), enabledFlag, &flipt.EvaluationRequest{ EntityId: "1", FlagKey: "foo", Context: map[string]string{ @@ -827,11 +800,9 @@ func TestEvaluator_ErrorGettingRules(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return([]*storage.EvaluationRule{}, errors.New("error getting rules!")) - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + resp, err := s.Evaluate(context.TODO(), enabledFlag, &flipt.EvaluationRequest{ EntityId: "1", FlagKey: "foo", Context: map[string]string{ @@ -851,8 +822,6 @@ func TestEvaluator_RulesOutOfOrder(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -889,7 +858,7 @@ func TestEvaluator_RulesOutOfOrder(t *testing.T) { }, }, nil) - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + resp, err := s.Evaluate(context.TODO(), enabledFlag, &flipt.EvaluationRequest{ EntityId: "1", FlagKey: "foo", Context: map[string]string{ @@ -910,8 +879,6 @@ func TestEvaluator_ErrorGettingDistributions(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -934,7 +901,7 @@ func TestEvaluator_ErrorGettingDistributions(t *testing.T) { store.On("GetEvaluationDistributions", mock.Anything, "1").Return([]*storage.EvaluationDistribution{}, errors.New("error getting distributions!")) - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + resp, err := s.Evaluate(context.TODO(), enabledFlag, &flipt.EvaluationRequest{ EntityId: "1", FlagKey: "foo", Context: map[string]string{ @@ -955,8 +922,6 @@ func TestEvaluator_MatchAll_NoVariants_NoDistributions(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1014,7 +979,7 @@ func TestEvaluator_MatchAll_NoVariants_NoDistributions(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) @@ -1041,8 +1006,6 @@ func TestEvaluator_MatchAll_SingleVariantDistribution(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1140,7 +1103,7 @@ func TestEvaluator_MatchAll_SingleVariantDistribution(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) @@ -1168,8 +1131,6 @@ func TestEvaluator_MatchAll_RolloutDistribution(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1259,7 +1220,7 @@ func TestEvaluator_MatchAll_RolloutDistribution(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) @@ -1286,8 +1247,6 @@ func TestEvaluator_MatchAll_RolloutDistribution_MultiRule(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1333,7 +1292,7 @@ func TestEvaluator_MatchAll_RolloutDistribution_MultiRule(t *testing.T) { }, }, nil) - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + resp, err := s.Evaluate(context.TODO(), enabledFlag, &flipt.EvaluationRequest{ FlagKey: "foo", EntityId: uuid.Must(uuid.NewV4()).String(), Context: map[string]string{ @@ -1358,8 +1317,6 @@ func TestEvaluator_MatchAll_NoConstraints(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1437,7 +1394,7 @@ func TestEvaluator_MatchAll_NoConstraints(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) @@ -1466,8 +1423,6 @@ func TestEvaluator_MatchAny_NoVariants_NoDistributions(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1525,7 +1480,7 @@ func TestEvaluator_MatchAny_NoVariants_NoDistributions(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) @@ -1552,8 +1507,6 @@ func TestEvaluator_MatchAny_SingleVariantDistribution(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1684,7 +1637,7 @@ func TestEvaluator_MatchAny_SingleVariantDistribution(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) @@ -1711,8 +1664,6 @@ func TestEvaluator_MatchAny_RolloutDistribution(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1802,7 +1753,7 @@ func TestEvaluator_MatchAny_RolloutDistribution(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) @@ -1829,8 +1780,6 @@ func TestEvaluator_MatchAny_RolloutDistribution_MultiRule(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1876,7 +1825,7 @@ func TestEvaluator_MatchAny_RolloutDistribution_MultiRule(t *testing.T) { }, }, nil) - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + resp, err := s.Evaluate(context.TODO(), enabledFlag, &flipt.EvaluationRequest{ FlagKey: "foo", EntityId: uuid.Must(uuid.NewV4()).String(), Context: map[string]string{ @@ -1901,8 +1850,6 @@ func TestEvaluator_MatchAny_NoConstraints(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -1980,7 +1927,7 @@ func TestEvaluator_MatchAny_NoConstraints(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) @@ -2009,8 +1956,6 @@ func TestEvaluator_FirstRolloutRuleIsZero(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -2078,7 +2023,7 @@ func TestEvaluator_FirstRolloutRuleIsZero(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) @@ -2106,8 +2051,6 @@ func TestEvaluator_MultipleZeroRolloutDistributions(t *testing.T) { s = NewEvaluator(logger, store) ) - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( []*storage.EvaluationRule{ { @@ -2203,7 +2146,7 @@ func TestEvaluator_MultipleZeroRolloutDistributions(t *testing.T) { ) t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) + resp, err := s.Evaluate(context.TODO(), enabledFlag, req) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, "foo", resp.FlagKey) diff --git a/internal/server/evaluation/server.go b/internal/server/evaluation/server.go new file mode 100644 index 0000000000..ab7ff8feb6 --- /dev/null +++ b/internal/server/evaluation/server.go @@ -0,0 +1,45 @@ +package evaluation + +import ( + "context" + + "go.flipt.io/flipt/internal/storage" + "go.flipt.io/flipt/rpc/flipt" + rpcEvalution "go.flipt.io/flipt/rpc/flipt/evaluation" + "go.uber.org/zap" + "google.golang.org/grpc" +) + +// Storer is the server side contract for communicating with the storage layer. +type Storer interface { + GetFlag(ctx context.Context, namespaceKey, key string) (*flipt.Flag, error) + GetEvaluationRules(ctx context.Context, namespaceKey string, flagKey string) ([]*storage.EvaluationRule, error) + GetEvaluationDistributions(ctx context.Context, ruleID string) ([]*storage.EvaluationDistribution, error) +} + +// MultiVariateEvaluator is an abstraction for evaluating a flag against a set of rules for multi-variate flags. +type MultiVariateEvaluator interface { + Evaluate(ctx context.Context, flag *flipt.Flag, r *flipt.EvaluationRequest) (*flipt.EvaluationResponse, error) +} + +// Server serves the Flipt evaluate v2 gRPC Server. +type Server struct { + logger *zap.Logger + store Storer + evaluator MultiVariateEvaluator + rpcEvalution.UnimplementedEvaluationServiceServer +} + +// New is constructs a new Server. +func New(logger *zap.Logger, store Storer) *Server { + return &Server{ + logger: logger, + store: store, + evaluator: NewEvaluator(logger, store), + } +} + +// RegisterGRPC registers the EvaluateServer onto the provided gRPC Server. +func (s *Server) RegisterGRPC(server *grpc.Server) { + rpcEvalution.RegisterEvaluationServiceServer(server, s) +} diff --git a/internal/server/evaluator.go b/internal/server/evaluator.go index 787d6b1286..cebc4ad3be 100644 --- a/internal/server/evaluator.go +++ b/internal/server/evaluator.go @@ -21,7 +21,20 @@ func (s *Server) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (*fli r.NamespaceKey = storage.DefaultNamespace } - resp, err := s.evaluator.Evaluate(ctx, r) + flag, err := s.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) + if err != nil { + var resp = &flipt.EvaluationResponse{} + resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON + + var errnf errs.ErrNotFound + if errors.As(err, &errnf) { + resp.Reason = flipt.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON + } + + return resp, err + } + + resp, err := s.evaluator.Evaluate(ctx, flag, r) if err != nil { return resp, err } @@ -81,15 +94,23 @@ func (s *Server) batchEvaluate(ctx context.Context, r *flipt.BatchEvaluationRequ return &res, errs.InvalidFieldError("namespace_key", "must be the same for all requests if specified") } - // TODO: we also need to validate each request, we should likely do this in the validation middleware - f, err := s.evaluator.Evaluate(ctx, req) + flag, err := s.store.GetFlag(ctx, req.NamespaceKey, req.FlagKey) if err != nil { var errnf errs.ErrNotFound if r.GetExcludeNotFound() && errors.As(err, &errnf) { continue } + return &res, err } + + // TODO: we also need to validate each request, we should likely do this in the validation middleware + f, err := s.evaluator.Evaluate(ctx, flag, req) + if err != nil { + s.logger.Error("error evaluating flag", zap.Error(err)) + return &res, err + } + f.RequestId = "" res.Responses = append(res.Responses, f) } diff --git a/internal/server/server.go b/internal/server/server.go index b7b293daca..af954d1ad4 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1,6 +1,8 @@ package server import ( + "context" + "go.flipt.io/flipt/internal/server/evaluation" "go.flipt.io/flipt/internal/storage" flipt "go.flipt.io/flipt/rpc/flipt" @@ -10,6 +12,11 @@ import ( var _ flipt.FliptServer = &Server{} +// MultiVariateEvaluator is an abstraction for evaluating a flag against a set of rules for multi-variate flags. +type MultiVariateEvaluator interface { + Evaluate(ctx context.Context, flag *flipt.Flag, r *flipt.EvaluationRequest) (*flipt.EvaluationResponse, error) +} + // Server serves the Flipt backend type Server struct { logger *zap.Logger diff --git a/rpc/flipt/evaluation/evaluation.go b/rpc/flipt/evaluation/evaluation.go index 28e8fe9b7d..7720987308 100644 --- a/rpc/flipt/evaluation/evaluation.go +++ b/rpc/flipt/evaluation/evaluation.go @@ -37,6 +37,14 @@ func (x *EvaluationResponse) SetRequestIDIfNotBlank(id string) string { return "" } +func (x *VariantEvaluationResponse) SetRequestIDIfNotBlank(id string) string { + if x.RequestId == "" { + x.RequestId = id + } + + return x.RequestId +} + // SetRequestIDIfNotBlank attempts to set the provided ID on the instance // If the ID was blank, it returns the ID provided to this call. // If the ID was not blank, it returns the ID found on the instance. @@ -93,6 +101,11 @@ func (x *EvaluationResponse) SetTimestamps(start, end time.Time) { } } +func (x *VariantEvaluationResponse) SetTimestamps(start, end time.Time) { + x.Timestamp = timestamppb.New(end) + x.RequestDurationMillis = float64(end.Sub(start)) / float64(time.Millisecond) +} + // SetTimestamps records the start and end times on the target instance. func (x *BatchEvaluationResponse) SetTimestamps(start, end time.Time) { x.RequestDurationMillis = float64(end.Sub(start)) / float64(time.Millisecond)