Skip to content

Commit b9c14b4

Browse files
yasirfolio3yasirfolio3
authored
refact(decide-reasons): Adding new info and error messages to decide reasons. (#304)
1 parent 7c2b190 commit b9c14b4

36 files changed

+468
-331
lines changed

‎pkg/client/optimizely_user_context_test.go‎

Lines changed: 114 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2020, Optimizely, Inc. and contributors *
2+
* Copyright 2020-2021, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -174,15 +174,17 @@ func (s *OptimizelyUserContextTestSuite) TestDecideFeatureTest() {
174174
s.Nil(err)
175175

176176
user := s.OptimizelyClient.CreateUserContext(s.userID, nil)
177-
decision := user.Decide(flagKey, nil)
177+
decision := user.Decide(flagKey, []decide.OptimizelyDecideOptions{decide.IncludeReasons})
178178

179179
s.Equal(variationKey, decision.VariationKey)
180180
s.Equal(true, decision.Enabled)
181181
s.Equal(variablesExpected.ToMap(), decision.Variables.ToMap())
182182
s.Equal(ruleKey, decision.RuleKey)
183183
s.Equal(flagKey, decision.FlagKey)
184184
s.Equal(user, decision.UserContext)
185-
s.Len(decision.Reasons, 0)
185+
reasons := decision.Reasons
186+
s.Len(reasons, 1)
187+
s.Equal(`Audiences for experiment exp_no_audience collectively evaluated to true.`, reasons[0])
186188

187189
s.True(len(s.eventProcessor.Events) == 1)
188190
s.Equal(s.userID, s.eventProcessor.Events[0].VisitorID)
@@ -205,15 +207,32 @@ func (s *OptimizelyUserContextTestSuite) TestDecideRollout() {
205207
s.Nil(err)
206208

207209
user := s.OptimizelyClient.CreateUserContext(s.userID, nil)
208-
decision := user.Decide(flagKey, nil)
210+
decision := user.Decide(flagKey, []decide.OptimizelyDecideOptions{decide.IncludeReasons})
209211

210212
s.Equal(variationKey, decision.VariationKey)
211213
s.Equal(true, decision.Enabled)
212214
s.Equal(variablesExpected.ToMap(), decision.Variables.ToMap())
213215
s.Equal(ruleKey, decision.RuleKey)
214216
s.Equal(flagKey, decision.FlagKey)
215217
s.Equal(user, decision.UserContext)
216-
s.Len(decision.Reasons, 3)
218+
reasons := decision.Reasons
219+
s.Len(reasons, 9)
220+
221+
expectedLogs := []string{
222+
`an error occurred while evaluating nested tree for audience ID "13389141123"`,
223+
`Audiences for experiment exp_with_audience collectively evaluated to false.`,
224+
`User "tester" does not meet conditions to be in experiment "exp_with_audience".`,
225+
`an error occurred while evaluating nested tree for audience ID "13389130056"`,
226+
`User "tester" does not meet conditions for targeting rule 1.`,
227+
`an error occurred while evaluating nested tree for audience ID "12208130097"`,
228+
`User "tester" does not meet conditions for targeting rule 2.`,
229+
`Audiences for experiment 18322080788 collectively evaluated to true.`,
230+
`User "tester" meets conditions for targeting rule "Everyone Else".`,
231+
}
232+
233+
for index, log := range expectedLogs {
234+
s.Equal(log, reasons[index])
235+
}
217236

218237
s.True(len(s.eventProcessor.Events) == 1)
219238
s.Equal(s.userID, s.eventProcessor.Events[0].VisitorID)
@@ -233,15 +252,17 @@ func (s *OptimizelyUserContextTestSuite) TestDecideNullVariation() {
233252
variablesExpected := optimizelyjson.NewOptimizelyJSONfromMap(map[string]interface{}{})
234253

235254
user := s.OptimizelyClient.CreateUserContext(s.userID, nil)
236-
decision := user.Decide(flagKey, nil)
255+
decision := user.Decide(flagKey, []decide.OptimizelyDecideOptions{decide.IncludeReasons})
237256

238257
s.Equal("", decision.VariationKey)
239258
s.Equal(false, decision.Enabled)
240259
s.Equal(variablesExpected.ToMap(), decision.Variables.ToMap())
241260
s.Equal("", decision.RuleKey)
242261
s.Equal("feature_3", decision.FlagKey)
243262
s.Equal(user, decision.UserContext)
244-
s.Len(decision.Reasons, 0)
263+
reasons := decision.Reasons
264+
s.Len(reasons, 1)
265+
s.Equal(`Rollout with ID "" is not in the datafile.`, reasons[0])
245266

246267
s.True(len(s.eventProcessor.Events) == 1)
247268
s.Equal(s.userID, s.eventProcessor.Events[0].VisitorID)
@@ -265,17 +286,21 @@ func (s *OptimizelyUserContextTestSuite) TestDecideForKeysOneFlag() {
265286
s.Nil(err)
266287

267288
user := s.OptimizelyClient.CreateUserContext(s.userID, nil)
268-
decisions := user.DecideForKeys(flagKeys, nil)
289+
decisions := user.DecideForKeys(flagKeys, []decide.OptimizelyDecideOptions{decide.IncludeReasons})
269290
s.Len(decisions, 1)
270291

271292
decision1 := decisions[flagKey]
293+
272294
s.Equal(variationKey, decision1.VariationKey)
273295
s.Equal(true, decision1.Enabled)
274296
s.Equal(variablesExpected.ToMap(), decision1.Variables.ToMap())
275297
s.Equal(ruleKey, decision1.RuleKey)
276298
s.Equal(flagKey, decision1.FlagKey)
277299
s.Equal(user, decision1.UserContext)
278-
s.Len(decision1.Reasons, 0)
300+
301+
reasons := decision1.Reasons
302+
s.Len(reasons, 1)
303+
s.Equal(`Audiences for experiment exp_no_audience collectively evaluated to true.`, reasons[0])
279304

280305
s.True(len(s.eventProcessor.Events) == 1)
281306
s.Equal(s.userID, s.eventProcessor.Events[0].VisitorID)
@@ -304,7 +329,7 @@ func (s *OptimizelyUserContextTestSuite) TestDecideForKeysWithMultipleFlags() {
304329
s.Nil(err)
305330

306331
user := s.OptimizelyClient.CreateUserContext(s.userID, map[string]interface{}{"gender": "f"})
307-
decisions := user.DecideForKeys(flagKeys, nil)
332+
decisions := user.DecideForKeys(flagKeys, []decide.OptimizelyDecideOptions{decide.IncludeReasons})
308333
s.Len(decisions, 2)
309334

310335
decision1 := decisions[flagKey1]
@@ -314,7 +339,9 @@ func (s *OptimizelyUserContextTestSuite) TestDecideForKeysWithMultipleFlags() {
314339
s.Equal(ruleKey1, decision1.RuleKey)
315340
s.Equal(flagKey1, decision1.FlagKey)
316341
s.Equal(user, decision1.UserContext)
317-
s.Len(decision1.Reasons, 0)
342+
reasons := decision1.Reasons
343+
s.Len(reasons, 1)
344+
s.Equal(`Audiences for experiment exp_with_audience collectively evaluated to true.`, reasons[0])
318345

319346
decision2 := decisions[flagKey2]
320347
s.Equal(variationKey2, decision2.VariationKey)
@@ -323,7 +350,9 @@ func (s *OptimizelyUserContextTestSuite) TestDecideForKeysWithMultipleFlags() {
323350
s.Equal(ruleKey2, decision2.RuleKey)
324351
s.Equal(flagKey2, decision2.FlagKey)
325352
s.Equal(user, decision2.UserContext)
326-
s.Len(decision2.Reasons, 0)
353+
reasons = decision2.Reasons
354+
s.Len(reasons, 1)
355+
s.Equal(`Audiences for experiment exp_no_audience collectively evaluated to true.`, reasons[0])
327356

328357
s.True(len(s.eventProcessor.Events) == 2)
329358
s.Equal(s.userID, s.eventProcessor.Events[0].VisitorID)
@@ -454,7 +483,7 @@ func (s *OptimizelyUserContextTestSuite) TestDecideAllEnabledFlagsOnly() {
454483
s.Nil(err)
455484

456485
user := s.OptimizelyClient.CreateUserContext(s.userID, map[string]interface{}{"gender": "f"})
457-
decisions := user.DecideAll([]decide.OptimizelyDecideOptions{decide.EnabledFlagsOnly})
486+
decisions := user.DecideAll([]decide.OptimizelyDecideOptions{decide.EnabledFlagsOnly, decide.IncludeReasons})
458487
s.Len(decisions, 2)
459488

460489
decision1 := decisions[flagKey1]
@@ -464,7 +493,9 @@ func (s *OptimizelyUserContextTestSuite) TestDecideAllEnabledFlagsOnly() {
464493
s.Equal("exp_with_audience", decision1.RuleKey)
465494
s.Equal(flagKey1, decision1.FlagKey)
466495
s.Equal(user, decision1.UserContext)
467-
s.Len(decision1.Reasons, 0)
496+
reasons := decision1.Reasons
497+
s.Len(reasons, 1)
498+
s.Equal(`Audiences for experiment exp_with_audience collectively evaluated to true.`, reasons[0])
468499
}
469500

470501
func (s *OptimizelyUserContextTestSuite) TestTrackEvent() {
@@ -580,6 +611,7 @@ func (s *OptimizelyUserContextTestSuite) TestDecideOptionsBypassUps() {
580611
variationID2 := "10418510624"
581612
variationKey1 := "variation_with_traffic"
582613
variationKey2 := "variation_no_traffic"
614+
options := []decide.OptimizelyDecideOptions{decide.IncludeReasons}
583615

584616
userProfileService := new(MockUserProfileService)
585617
s.OptimizelyClient, _ = s.factory.Client(
@@ -596,13 +628,20 @@ func (s *OptimizelyUserContextTestSuite) TestDecideOptionsBypassUps() {
596628
userProfileService.On("Save", mock.Anything)
597629

598630
userContext := s.OptimizelyClient.CreateUserContext(s.userID, map[string]interface{}{})
599-
decision := userContext.Decide(flagKey, nil)
631+
decision := userContext.Decide(flagKey, options)
632+
reasons := decision.Reasons
633+
s.Len(reasons, 1)
634+
s.Equal(`User "tester" was previously bucketed into variation "variation_no_traffic" of experiment "exp_no_audience".`, reasons[0])
600635
// should return variationId2 set by UPS
601636
s.Equal(variationKey2, decision.VariationKey)
602637
userProfileService.AssertCalled(s.T(), "Lookup", s.userID)
603638
userProfileService.AssertNotCalled(s.T(), "Save", mock.Anything)
604639

605-
decision = userContext.Decide(flagKey, []decide.OptimizelyDecideOptions{decide.IgnoreUserProfileService})
640+
options = append(options, decide.IgnoreUserProfileService)
641+
decision = userContext.Decide(flagKey, options)
642+
reasons = decision.Reasons
643+
s.Len(reasons, 1)
644+
s.Equal(`Audiences for experiment exp_no_audience collectively evaluated to true.`, reasons[0])
606645
// should not lookup, ignore variationId2 set by UPS and return variationId1
607646
s.Equal(variationKey1, decision.VariationKey)
608647
userProfileService.AssertNumberOfCalls(s.T(), "Lookup", 1)
@@ -641,8 +680,25 @@ func (s *OptimizelyUserContextTestSuite) TestDecideOptionsIncludeReasons() {
641680

642681
// valid flag key
643682
flagKey = "feature_1"
644-
decision = user.Decide(flagKey, nil)
645-
s.Len(decision.Reasons, 3)
683+
decision = user.Decide(flagKey, options)
684+
reasons := decision.Reasons
685+
s.Len(reasons, 9)
686+
687+
expectedLogs := []string{
688+
`an error occurred while evaluating nested tree for audience ID "13389141123"`,
689+
`Audiences for experiment exp_with_audience collectively evaluated to false.`,
690+
`User "tester" does not meet conditions to be in experiment "exp_with_audience".`,
691+
`an error occurred while evaluating nested tree for audience ID "13389130056"`,
692+
`User "tester" does not meet conditions for targeting rule 1.`,
693+
`an error occurred while evaluating nested tree for audience ID "12208130097"`,
694+
`User "tester" does not meet conditions for targeting rule 2.`,
695+
`Audiences for experiment 18322080788 collectively evaluated to true.`,
696+
`User "tester" meets conditions for targeting rule "Everyone Else".`,
697+
}
698+
699+
for index, log := range expectedLogs {
700+
s.Equal(log, reasons[index])
701+
}
646702
}
647703

648704
func (s *OptimizelyUserContextTestSuite) TestDefaultDecideOptionsExcludeVariables() {
@@ -654,14 +710,38 @@ func (s *OptimizelyUserContextTestSuite) TestDefaultDecideOptionsExcludeVariable
654710
// should be excluded by DefaultDecideOption
655711
decision := userContext.Decide(flagKey, nil)
656712
s.Len(decision.Variables.ToMap(), 0)
713+
reasons := decision.Reasons
714+
s.Len(reasons, 0)
715+
716+
options = append(options, decide.IncludeReasons)
717+
client, _ = s.factory.Client(WithEventProcessor(s.eventProcessor), WithDefaultDecideOptions(options))
718+
userContext = client.CreateUserContext(s.userID, nil)
719+
720+
decision = userContext.Decide(flagKey, nil)
721+
reasons = decision.Reasons
722+
s.Len(reasons, 9)
723+
724+
expectedLogs := []string{
725+
`an error occurred while evaluating nested tree for audience ID "13389141123"`,
726+
`Audiences for experiment exp_with_audience collectively evaluated to false.`,
727+
`User "tester" does not meet conditions to be in experiment "exp_with_audience".`,
728+
`an error occurred while evaluating nested tree for audience ID "13389130056"`,
729+
`User "tester" does not meet conditions for targeting rule 1.`,
730+
`an error occurred while evaluating nested tree for audience ID "12208130097"`,
731+
`User "tester" does not meet conditions for targeting rule 2.`,
732+
`Audiences for experiment 18322080788 collectively evaluated to true.`,
733+
`User "tester" meets conditions for targeting rule "Everyone Else".`,
734+
}
657735

658-
// @TODO: Need one more case: IncludeReasons = true and flagKey = "feature_1". Then reasons.count > 0
736+
for index, log := range expectedLogs {
737+
s.Equal(log, reasons[index])
738+
}
659739
}
660740

661741
func (s *OptimizelyUserContextTestSuite) TestDefaultDecideOptionsEnabledFlagsOnly() {
662742
flagKey := "feature_1"
663743
variablesExpected, _ := s.OptimizelyClient.GetAllFeatureVariables(flagKey, entities.UserContext{ID: s.userID})
664-
options := []decide.OptimizelyDecideOptions{decide.EnabledFlagsOnly}
744+
options := []decide.OptimizelyDecideOptions{decide.EnabledFlagsOnly, decide.IncludeReasons}
665745
client, _ := s.factory.Client(WithEventProcessor(s.eventProcessor), WithDefaultDecideOptions(options))
666746
user := client.CreateUserContext(s.userID, map[string]interface{}{"gender": "f"})
667747

@@ -676,7 +756,9 @@ func (s *OptimizelyUserContextTestSuite) TestDefaultDecideOptionsEnabledFlagsOnl
676756
s.Equal("exp_with_audience", decision1.RuleKey)
677757
s.Equal(flagKey, decision1.FlagKey)
678758
s.Equal(user, decision1.UserContext)
679-
s.Len(decision1.Reasons, 0)
759+
reasons := decision1.Reasons
760+
s.Len(reasons, 1)
761+
s.Equal("Audiences for experiment exp_with_audience collectively evaluated to true.", reasons[0])
680762
}
681763

682764
func (s *OptimizelyUserContextTestSuite) TestDefaultDecideOptionsIncludeReasons() {
@@ -714,7 +796,8 @@ func (s *OptimizelyUserContextTestSuite) TestDefaultDecideOptionsBypassUps() {
714796
options := []decide.OptimizelyDecideOptions{decide.IgnoreUserProfileService}
715797
client, _ := s.factory.Client(WithEventProcessor(s.eventProcessor), WithDefaultDecideOptions(options))
716798
user := client.CreateUserContext(s.userID, nil)
717-
decision := user.Decide(flagKey, nil)
799+
decision := user.Decide(flagKey, []decide.OptimizelyDecideOptions{decide.IncludeReasons})
800+
s.Len(decision.Reasons, 1)
718801

719802
// should get IgnoreUserProfileService by DefaultDecideOption
720803
// should not lookup, ignore variationId2 set by UPS and return variationId1
@@ -763,7 +846,7 @@ func (s *OptimizelyUserContextTestSuite) TestDecideSDKNotReady() {
763846
func (s *OptimizelyUserContextTestSuite) TestDecideInvalidFeatureKey() {
764847
flagKey := "invalid_key"
765848
userContext := s.OptimizelyClient.CreateUserContext(s.userID, nil)
766-
decision := userContext.Decide(flagKey, nil)
849+
decision := userContext.Decide(flagKey, []decide.OptimizelyDecideOptions{decide.IncludeReasons})
767850

768851
s.Equal("", decision.VariationKey)
769852
s.False(decision.Enabled)
@@ -779,7 +862,7 @@ func (s *OptimizelyUserContextTestSuite) TestDecideForKeySDKNotReady() {
779862
factory := OptimizelyFactory{SDKKey: "121"}
780863
client, _ := factory.Client()
781864
userContext := client.CreateUserContext(s.userID, nil)
782-
decisions := userContext.DecideForKeys(flagKeys, nil)
865+
decisions := userContext.DecideForKeys(flagKeys, []decide.OptimizelyDecideOptions{decide.IncludeReasons})
783866

784867
s.Len(decisions, 0)
785868
}
@@ -801,7 +884,7 @@ func (s *OptimizelyUserContextTestSuite) TestDecideForKeysErrorDecisionIncluded(
801884
s.Nil(err)
802885

803886
user := s.OptimizelyClient.CreateUserContext(s.userID, nil)
804-
decisions := user.DecideForKeys(flagKeys, nil)
887+
decisions := user.DecideForKeys(flagKeys, []decide.OptimizelyDecideOptions{decide.IncludeReasons})
805888
s.Len(decisions, 2)
806889

807890
decision := decisions[flagKey1]
@@ -811,13 +894,16 @@ func (s *OptimizelyUserContextTestSuite) TestDecideForKeysErrorDecisionIncluded(
811894
s.Equal("exp_no_audience", decision.RuleKey)
812895
s.Equal(flagKey1, decision.FlagKey)
813896
s.Equal(user, decision.UserContext)
814-
s.Len(decision.Reasons, 0)
897+
reasons := decision.Reasons
898+
s.Len(reasons, 1)
899+
s.Equal(`Audiences for experiment exp_no_audience collectively evaluated to true.`, reasons[0])
815900

816901
decision = decisions[flagKey2]
817902
s.Equal(flagKey2, decision.FlagKey)
818903
s.Equal(user, decision.UserContext)
819-
s.Len(decision.Reasons, 1)
820-
s.Equal(decide.GetDecideMessage(decide.FlagKeyInvalid, flagKey2), decision.Reasons[0])
904+
reasons = decision.Reasons
905+
s.Len(reasons, 1)
906+
s.Equal(decide.GetDecideMessage(decide.FlagKeyInvalid, flagKey2), reasons[0])
821907
}
822908

823909
func TestOptimizelyUserContextTestSuite(t *testing.T) {

‎pkg/decide/default_decision_reasons.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2020, Optimizely, Inc. and contributors *
2+
* Copyright 2020-2021, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -23,19 +23,19 @@ import (
2323

2424
// DefaultDecisionReasons provides the default implementation of DecisionReasons.
2525
type DefaultDecisionReasons struct {
26-
errors, logs []string
26+
errors, infos []string
2727
includeReasons bool
2828
}
2929

3030
// NewDecisionReasons returns a new instance of DecisionReasons.
3131
func NewDecisionReasons(options *Options) *DefaultDecisionReasons {
32-
includeReasons := false
32+
var includeReasons bool
3333
if options != nil {
3434
includeReasons = options.IncludeReasons
3535
}
3636
return &DefaultDecisionReasons{
3737
errors: []string{},
38-
logs: []string{},
38+
infos: []string{},
3939
includeReasons: includeReasons,
4040
}
4141
}
@@ -51,7 +51,7 @@ func (o *DefaultDecisionReasons) AddInfo(format string, arguments ...interface{}
5151
if !o.includeReasons {
5252
return message
5353
}
54-
o.logs = append(o.logs, message)
54+
o.infos = append(o.infos, message)
5555
return message
5656
}
5757

@@ -60,7 +60,7 @@ func (o *DefaultDecisionReasons) Append(reasons DecisionReasons) {
6060
if decisionReasons, ok := reasons.(*DefaultDecisionReasons); ok {
6161
o.errors = append(o.errors, decisionReasons.errors...)
6262
if o.includeReasons {
63-
o.logs = append(o.logs, decisionReasons.logs...)
63+
o.infos = append(o.infos, decisionReasons.infos...)
6464
}
6565
}
6666
}
@@ -71,5 +71,5 @@ func (o *DefaultDecisionReasons) ToReport() []string {
7171
if !o.includeReasons {
7272
return reasons
7373
}
74-
return append(reasons, o.logs...)
74+
return append(reasons, o.infos...)
7575
}

‎pkg/decision/bucketer/experiment_bucketer.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type MurmurhashExperimentBucketer struct {
3636
// NewMurmurhashExperimentBucketer returns a new instance of the murmurhash experiment bucketer
3737
func NewMurmurhashExperimentBucketer(logger logging.OptimizelyLogProducer, hashSeed uint32) *MurmurhashExperimentBucketer {
3838
return &MurmurhashExperimentBucketer{
39-
bucketer: MurmurhashBucketer{hashSeed: hashSeed, logger:logger},
39+
bucketer: MurmurhashBucketer{hashSeed: hashSeed, logger: logger},
4040
}
4141
}
4242

0 commit comments

Comments
 (0)