Skip to content

Commit

Permalink
Merge branch 'v2-evaluate-variant' into gm/boolean-its
Browse files Browse the repository at this point in the history
  • Loading branch information
GeorgeMac committed Jun 30, 2023
2 parents df2ba34 + c7f6aa1 commit 645b5f7
Show file tree
Hide file tree
Showing 11 changed files with 321 additions and 139 deletions.
2 changes: 1 addition & 1 deletion internal/cmd/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Check warning on line 347 in internal/cmd/grpc.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/grpc.go#L347

Added line #L347 was not covered by tests

// initialize grpc server
server.Server = grpc.NewServer(grpcOpts...)
Expand Down
32 changes: 21 additions & 11 deletions internal/server/evaluation/evaluation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}

Check warning on line 27 in internal/server/evaluation/evaluation.go

View check run for this annotation

Codecov / codecov/patch

internal/server/evaluation/evaluation.go#L26-L27

Added lines #L26 - L27 were not covered by tests

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,
Expand All @@ -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
}
30 changes: 0 additions & 30 deletions internal/server/evaluation/evaluation_server.go

This file was deleted.

170 changes: 170 additions & 0 deletions internal/server/evaluation/evaluation_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
22 changes: 2 additions & 20 deletions internal/server/evaluation/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package evaluation

import (
"context"
"errors"
"hash/crc32"
"sort"
"strconv"
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions internal/server/evaluation/evaluator_mock.go
Original file line number Diff line number Diff line change
@@ -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"

Check warning on line 15 in internal/server/evaluation/evaluator_mock.go

View check run for this annotation

Codecov / codecov/patch

internal/server/evaluation/evaluator_mock.go#L14-L15

Added lines #L14 - L15 were not covered by tests
}

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)
}
Loading

0 comments on commit 645b5f7

Please sign in to comment.