Skip to content
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.

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

feat(decide): Adding support for decide options and decision reasons. #298

Merged
merged 38 commits into from
Dec 15, 2020
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
2f52077
initial commit.
yasirfolio3 Oct 14, 2020
e568bac
Adding unit tests.
yasirfolio3 Oct 16, 2020
5792f2c
linter warnings fixed.
yasirfolio3 Oct 16, 2020
26a25ec
nit fixed.
yasirfolio3 Oct 16, 2020
d98e193
fixes.
yasirfolio3 Oct 19, 2020
c7a0fd9
fixes.
yasirfolio3 Oct 19, 2020
431f97a
Merge branch 'master' into yasir/user-context-2
yasirfolio3 Oct 19, 2020
b223b56
Internal changes to accomodate decide api.
yasirfolio3 Oct 21, 2020
d8c4f49
Merge branch 'yasir/user-context-2' into yasir/decide-api
yasirfolio3 Oct 21, 2020
6157c71
linter warnings fixed.
yasirfolio3 Oct 21, 2020
880fb08
linter warnings fixed.
yasirfolio3 Oct 21, 2020
91c1beb
removing added reasons, to be added in a separate PR.
yasirfolio3 Oct 22, 2020
8fc5bc5
linter warnings fixed.
yasirfolio3 Oct 22, 2020
564679e
removing redundant info method.
yasirfolio3 Oct 23, 2020
f257a20
suggested changes made.
yasirfolio3 Oct 23, 2020
4ab5403
fixes.
yasirfolio3 Oct 23, 2020
7d688c1
Merge branch 'yasir/user-context-2' into yasir/decide-api-2
yasirfolio3 Oct 23, 2020
f08ac01
fixes.
yasirfolio3 Oct 26, 2020
23cb870
Merge branch 'yasir/user-context-2' into yasir/decide-api-2
yasirfolio3 Oct 26, 2020
b841442
Suggested changes made.
yasirfolio3 Oct 27, 2020
9b7153a
Merge branch 'yasir/user-context-2' into yasir/decide-api-2
yasirfolio3 Oct 27, 2020
d413572
removing unnecessary code.
yasirfolio3 Oct 27, 2020
55dec9b
nit fixed.
yasirfolio3 Oct 28, 2020
e28ed7c
Updating decision reasons to a pointer.
yasirfolio3 Oct 28, 2020
f63b7f5
nits fixed.
yasirfolio3 Oct 28, 2020
0b6d43a
Merge branch 'yasir/user-context-2' into yasir/decide-api-2
yasirfolio3 Oct 28, 2020
ea619fa
fixes.
yasirfolio3 Oct 28, 2020
bd34bd7
Merge branch 'yasir/user-context-2' into yasir/decide-api-2.
yasirfolio3 Oct 28, 2020
ef47d2a
suggested changes made.
yasirfolio3 Nov 3, 2020
6482bda
Merge branch 'master' into yasir/user-context-2
yasirfolio3 Nov 3, 2020
1ffd78c
Updates and fixes.
yasirfolio3 Nov 3, 2020
5f4a517
fixes.
yasirfolio3 Nov 9, 2020
319dfd2
linter warnings fixed.
yasirfolio3 Nov 9, 2020
b460a9a
Merge branch 'master' into yasir/user-context-2
yasirfolio3 Nov 24, 2020
e113a70
Merge branch 'yasir/user-context-2' into yasir/decide-api-2
yasirfolio3 Nov 24, 2020
7f4b735
Merge branch 'master' into yasir/decide-api-2
msohailhussain Dec 10, 2020
5fc5e9f
Added test for reasons with arguments.
yasirfolio3 Dec 11, 2020
5084ef8
suggested changes made.
yasirfolio3 Dec 15, 2020
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
6 changes: 4 additions & 2 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,8 @@ func (o *OptimizelyClient) getFeatureDecision(featureKey, variableKey string, us
}

decisionContext.Variable = variable
featureDecision, err = o.DecisionService.GetFeatureDecision(decisionContext, userContext)
options := decide.OptimizelyDecideOptions{}
featureDecision, err = o.DecisionService.GetFeatureDecision(decisionContext, userContext, options, decide.NewDecisionReasons(options))
if err != nil {
o.logger.Warning(fmt.Sprintf(`Received error while making a decision for feature "%s": %s`, featureKey, err))
return decisionContext, featureDecision, nil
Expand Down Expand Up @@ -687,7 +688,8 @@ func (o *OptimizelyClient) getExperimentDecision(experimentKey string, userConte
ProjectConfig: projectConfig,
}

experimentDecision, err = o.DecisionService.GetExperimentDecision(decisionContext, userContext)
options := decide.OptimizelyDecideOptions{}
experimentDecision, err = o.DecisionService.GetExperimentDecision(decisionContext, userContext, options, decide.NewDecisionReasons(options))
if err != nil {
o.logger.Warning(fmt.Sprintf(`Received error while making a decision for experiment "%s": %s`, experimentKey, err))
return decisionContext, experimentDecision, nil
Expand Down
67 changes: 34 additions & 33 deletions pkg/client/client_test.go

Large diffs are not rendered by default.

15 changes: 8 additions & 7 deletions pkg/client/fixtures_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/****************************************************************************
* Copyright 2019, Optimizely, Inc. and contributors *
* Copyright 2019-2020, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
Expand All @@ -21,6 +21,7 @@ import (
"fmt"

"github.com/optimizely/go-sdk/pkg/config"
"github.com/optimizely/go-sdk/pkg/decide"
"github.com/optimizely/go-sdk/pkg/decision"
"github.com/optimizely/go-sdk/pkg/entities"
"github.com/optimizely/go-sdk/pkg/event"
Expand Down Expand Up @@ -116,13 +117,13 @@ type MockDecisionService struct {
mock.Mock
}

func (m *MockDecisionService) GetFeatureDecision(decisionContext decision.FeatureDecisionContext, userContext entities.UserContext) (decision.FeatureDecision, error) {
args := m.Called(decisionContext, userContext)
func (m *MockDecisionService) GetFeatureDecision(decisionContext decision.FeatureDecisionContext, userContext entities.UserContext, options decide.OptimizelyDecideOptions, reasons decide.DecisionReasons) (decision.FeatureDecision, error) {
args := m.Called(decisionContext, userContext, options, reasons)
return args.Get(0).(decision.FeatureDecision), args.Error(1)
}

func (m *MockDecisionService) GetExperimentDecision(decisionContext decision.ExperimentDecisionContext, userContext entities.UserContext) (decision.ExperimentDecision, error) {
args := m.Called(decisionContext, userContext)
func (m *MockDecisionService) GetExperimentDecision(decisionContext decision.ExperimentDecisionContext, userContext entities.UserContext, options decide.OptimizelyDecideOptions, reasons decide.DecisionReasons) (decision.ExperimentDecision, error) {
args := m.Called(decisionContext, userContext, options, reasons)
return args.Get(0).(decision.ExperimentDecision), args.Error(1)
}

Expand Down Expand Up @@ -159,11 +160,11 @@ func (m *PanickingConfigManager) GetConfig() (config.ProjectConfig, error) {
type PanickingDecisionService struct {
}

func (m *PanickingDecisionService) GetFeatureDecision(decisionContext decision.FeatureDecisionContext, userContext entities.UserContext) (decision.FeatureDecision, error) {
func (m *PanickingDecisionService) GetFeatureDecision(decisionContext decision.FeatureDecisionContext, userContext entities.UserContext, options decide.OptimizelyDecideOptions, reasons decide.DecisionReasons) (decision.FeatureDecision, error) {
panic("I'm panicking")
}

func (m *PanickingDecisionService) GetExperimentDecision(decisionContext decision.ExperimentDecisionContext, userContext entities.UserContext) (decision.ExperimentDecision, error) {
func (m *PanickingDecisionService) GetExperimentDecision(decisionContext decision.ExperimentDecisionContext, userContext entities.UserContext, options decide.OptimizelyDecideOptions, reasons decide.DecisionReasons) (decision.ExperimentDecision, error) {
panic("I'm panicking")
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/decide/decide_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ type decideError string
const (
// SDKNotReady when sdk is not ready
SDKNotReady decideError = "Optimizely SDK not configured properly yet"
// FlagKeyInvalid when invalid flag key is provided
FlagKeyInvalid decideError = `No flag was found for key "%s".`
// VariableValueInvalid when invalid variable value is provided
VariableValueInvalid decideError = `Variable value for key "%s" is invalid or wrong type.`
)

// GetError returns error for decide error type
Expand Down
25 changes: 25 additions & 0 deletions pkg/decide/decision_reasons.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/****************************************************************************
* Copyright 2020, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
* You may obtain a copy of the License at *
* *
* http://www.apache.org/licenses/LICENSE-2.0 *
* *
* Unless required by applicable law or agreed to in writing, software *
* distributed under the License is distributed on an "AS IS" BASIS, *
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. *
* See the License for the specific language governing permissions and *
* limitations under the License. *
***************************************************************************/

// Package decide //
package decide

// DecisionReasons defines the reasons for which the decision was made.
type DecisionReasons interface {
AddError(format string, arguments ...interface{})
AddInfo(format string, arguments ...interface{}) string
ToReport() []string
}
70 changes: 70 additions & 0 deletions pkg/decide/decision_reasons_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/****************************************************************************
* Copyright 2020, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
* You may obtain a copy of the License at *
* *
* http://www.apache.org/licenses/LICENSE-2.0 *
* *
* Unless required by applicable law or agreed to in writing, software *
* distributed under the License is distributed on an "AS IS" BASIS, *
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. *
* See the License for the specific language governing permissions and *
* limitations under the License. *
***************************************************************************/

package decide

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestNewDecisionReasonsWithEmptyOptions(t *testing.T) {
reasons := NewDecisionReasons(OptimizelyDecideOptions{})
assert.Equal(t, 0, len(reasons.ToReport()))
}

func TestAddErrorWorksWithEveryOption(t *testing.T) {
options := OptimizelyDecideOptions{
DisableDecisionEvent: true,
EnabledFlagsOnly: true,
IgnoreUserProfileService: true,
ExcludeVariables: true,
IncludeReasons: true,
}
reasons := NewDecisionReasons(options)
reasons.AddError("error message")

reportedReasons := reasons.ToReport()
assert.Equal(t, 1, len(reportedReasons))
assert.Equal(t, "error message", reportedReasons[0])
}

func TestInfoLogsAreOnlyReportedWhenIncludeReasonsOptionIsSet(t *testing.T) {
options := OptimizelyDecideOptions{
DisableDecisionEvent: true,
EnabledFlagsOnly: true,
IgnoreUserProfileService: true,
ExcludeVariables: true,
}
reasons := NewDecisionReasons(options)
reasons.AddError("error message")
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a test with arguments ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 👍

reasons.AddInfo("info message")

reportedReasons := reasons.ToReport()
assert.Equal(t, 1, len(reportedReasons))
assert.Equal(t, "error message", reportedReasons[0])

options.IncludeReasons = true
reasons = NewDecisionReasons(options)
reasons.AddError("error message")
reasons.AddInfo("info message")

reportedReasons = reasons.ToReport()
assert.Equal(t, 2, len(reportedReasons))
assert.Equal(t, "error message", reportedReasons[0])
assert.Equal(t, "info message", reportedReasons[1])
}
61 changes: 61 additions & 0 deletions pkg/decide/default_decision_reasons.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/****************************************************************************
* Copyright 2020, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
* You may obtain a copy of the License at *
* *
* http://www.apache.org/licenses/LICENSE-2.0 *
* *
* Unless required by applicable law or agreed to in writing, software *
* distributed under the License is distributed on an "AS IS" BASIS, *
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. *
* See the License for the specific language governing permissions and *
* limitations under the License. *
***************************************************************************/

// Package decide //
package decide

import (
"fmt"
)

// DefaultDecisionReasons provides the default implementation of DecisionReasons.
type DefaultDecisionReasons struct {
errors, logs []string
includeReasons bool
}

// NewDecisionReasons returns a new instance of DecisionReasons.
func NewDecisionReasons(options OptimizelyDecideOptions) *DefaultDecisionReasons {
return &DefaultDecisionReasons{
errors: []string{},
logs: []string{},
includeReasons: options.IncludeReasons,
}
}

// AddError appends given message to the error list.
func (o *DefaultDecisionReasons) AddError(format string, arguments ...interface{}) {
o.errors = append(o.errors, fmt.Sprintf(format, arguments...))
}

// AddInfo appends given info message to the info list after formatting.
func (o *DefaultDecisionReasons) AddInfo(format string, arguments ...interface{}) string {
message := fmt.Sprintf(format, arguments...)
if !o.includeReasons {
return message
}
o.logs = append(o.logs, message)
return message
}

// ToReport returns reasons to be reported.
func (o *DefaultDecisionReasons) ToReport() []string {
reasons := o.errors
if !o.includeReasons {
return reasons
}
return append(reasons, o.logs...)
}
11 changes: 6 additions & 5 deletions pkg/decision/composite_experiment_service.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/****************************************************************************
* Copyright 2019, Optimizely, Inc. and contributors *
* Copyright 2019-2020, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
Expand All @@ -19,6 +19,8 @@ package decision

import (
"fmt"

"github.com/optimizely/go-sdk/pkg/decide"
"github.com/optimizely/go-sdk/pkg/entities"
"github.com/optimizely/go-sdk/pkg/logging"
)
Expand Down Expand Up @@ -54,7 +56,7 @@ func NewCompositeExperimentService(sdkKey string, options ...CESOptionFunc) *Com
// 1. Overrides (if supplied)
// 2. Whitelist
// 3. Bucketing (with User profile integration if supplied)
compositeExperimentService := &CompositeExperimentService{logger:logging.GetLogger(sdkKey, "CompositeExperimentService")}
compositeExperimentService := &CompositeExperimentService{logger: logging.GetLogger(sdkKey, "CompositeExperimentService")}
for _, opt := range options {
opt(compositeExperimentService)
}
Expand All @@ -81,11 +83,10 @@ func NewCompositeExperimentService(sdkKey string, options ...CESOptionFunc) *Com
}

// GetDecision returns a decision for the given experiment and user context
func (s CompositeExperimentService) GetDecision(decisionContext ExperimentDecisionContext, userContext entities.UserContext) (decision ExperimentDecision, err error) {

func (s CompositeExperimentService) GetDecision(decisionContext ExperimentDecisionContext, userContext entities.UserContext, options decide.OptimizelyDecideOptions, reasons decide.DecisionReasons) (decision ExperimentDecision, err error) {
// Run through the various decision services until we get a decision
for _, experimentService := range s.experimentServices {
decision, err = experimentService.GetDecision(decisionContext, userContext)
decision, err = experimentService.GetDecision(decisionContext, userContext, options, reasons)
if err != nil {
s.logger.Debug(fmt.Sprintf("%v", err))
}
Expand Down
Loading