Skip to content

Commit 87f0718

Browse files
authored
[FSSDK-11169] Implement Decision Service methods to handle CMAB (#403)
* add decision service methods to support cmab * refactor composite exper service for cmab and add tests * update failing tests * update experiment cmab service * fix lint errors * more lint fixes * traffic allocation in cmab * remove duplicate line * refactor to support traffic allocation * fixed cmab/service.go * add unit tests * fix lint errors and failed test * add package name * remove adapter, make client handle cmab error, fix EntityID string * revert cmab service to cleaner version from previous pr * fix remianing comments * fix linting * fix coveralls
1 parent debb46c commit 87f0718

19 files changed

+2116
-347
lines changed

pkg/client/client.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ func (o *OptimizelyClient) decide(userContext *OptimizelyUserContext, key string
206206
}
207207

208208
if err != nil {
209-
o.logger.Warning(fmt.Sprintf(`Received error while making a decision for feature %q: %s`, key, err))
209+
return o.handleDecisionServiceError(err, key, *userContext)
210210
}
211211

212212
if featureDecision.Variation != nil {
@@ -1248,3 +1248,9 @@ func (o *OptimizelyClient) getDecisionVariableMap(feature entities.Feature, vari
12481248
func isNil(v interface{}) bool {
12491249
return v == nil || (reflect.ValueOf(v).Kind() == reflect.Ptr && reflect.ValueOf(v).IsNil())
12501250
}
1251+
1252+
func (o *OptimizelyClient) handleDecisionServiceError(err error, key string, userContext OptimizelyUserContext) OptimizelyDecision {
1253+
o.logger.Warning(fmt.Sprintf(`Received error while making a decision for feature %q: %s`, key, err))
1254+
1255+
return NewErrorDecision(key, userContext, err)
1256+
}

pkg/client/client_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,8 +1682,7 @@ func TestGetFeatureDecisionErrFeatureDecision(t *testing.T) {
16821682
ConfigManager: mockConfigManager,
16831683
DecisionService: mockDecisionService,
16841684
logger: logging.GetLogger("", ""),
1685-
tracer: &MockTracer{},
1686-
}
1685+
tracer: &MockTracer{}}
16871686

16881687
_, decision, err := client.getFeatureDecision(testFeatureKey, testVariableKey, testUserContext)
16891688
assert.Equal(t, expectedFeatureDecision, decision)

pkg/decision/cmab_client.go renamed to pkg/cmab/client.go

Lines changed: 41 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
* limitations under the License. *
1515
***************************************************************************/
1616

17-
// Package decision provides CMAB client implementation
18-
package decision
17+
// Package cmab provides contextual multi-armed bandit functionality
18+
package cmab
1919

2020
import (
2121
"bytes"
@@ -44,34 +44,34 @@ const (
4444
DefaultBackoffMultiplier = 2.0
4545
)
4646

47-
// CMABAttribute represents an attribute in a CMAB request
48-
type CMABAttribute struct {
47+
// Attribute represents an attribute in a CMAB request
48+
type Attribute struct {
4949
ID string `json:"id"`
5050
Value interface{} `json:"value"`
5151
Type string `json:"type"`
5252
}
5353

54-
// CMABInstance represents an instance in a CMAB request
55-
type CMABInstance struct {
56-
VisitorID string `json:"visitorId"`
57-
ExperimentID string `json:"experimentId"`
58-
Attributes []CMABAttribute `json:"attributes"`
59-
CmabUUID string `json:"cmabUUID"`
54+
// Instance represents an instance in a CMAB request
55+
type Instance struct {
56+
VisitorID string `json:"visitorId"`
57+
ExperimentID string `json:"experimentId"`
58+
Attributes []Attribute `json:"attributes"`
59+
CmabUUID string `json:"cmabUUID"`
6060
}
6161

62-
// CMABRequest represents a request to the CMAB API
63-
type CMABRequest struct {
64-
Instances []CMABInstance `json:"instances"`
62+
// Request represents a request to the CMAB API
63+
type Request struct {
64+
Instances []Instance `json:"instances"`
6565
}
6666

67-
// CMABPrediction represents a prediction in a CMAB response
68-
type CMABPrediction struct {
67+
// Prediction represents a prediction in a CMAB response
68+
type Prediction struct {
6969
VariationID string `json:"variation_id"`
7070
}
7171

72-
// CMABResponse represents a response from the CMAB API
73-
type CMABResponse struct {
74-
Predictions []CMABPrediction `json:"predictions"`
72+
// Response represents a response from the CMAB API
73+
type Response struct {
74+
Predictions []Prediction `json:"predictions"`
7575
}
7676

7777
// RetryConfig defines configuration for retry behavior
@@ -93,15 +93,15 @@ type DefaultCmabClient struct {
9393
logger logging.OptimizelyLogProducer
9494
}
9595

96-
// CmabClientOptions defines options for creating a CMAB client
97-
type CmabClientOptions struct {
96+
// ClientOptions defines options for creating a CMAB client
97+
type ClientOptions struct {
9898
HTTPClient *http.Client
9999
RetryConfig *RetryConfig
100100
Logger logging.OptimizelyLogProducer
101101
}
102102

103103
// NewDefaultCmabClient creates a new instance of DefaultCmabClient
104-
func NewDefaultCmabClient(options CmabClientOptions) *DefaultCmabClient {
104+
func NewDefaultCmabClient(options ClientOptions) *DefaultCmabClient {
105105
httpClient := options.HTTPClient
106106
if httpClient == nil {
107107
httpClient = &http.Client{
@@ -127,33 +127,28 @@ func NewDefaultCmabClient(options CmabClientOptions) *DefaultCmabClient {
127127

128128
// FetchDecision fetches a decision from the CMAB API
129129
func (c *DefaultCmabClient) FetchDecision(
130-
ctx context.Context,
131130
ruleID string,
132131
userID string,
133132
attributes map[string]interface{},
134133
cmabUUID string,
135134
) (string, error) {
136-
// If no context is provided, create a background context
137-
if ctx == nil {
138-
ctx = context.Background()
139-
}
140135

141136
// Create the URL
142137
url := fmt.Sprintf(CMABPredictionEndpoint, ruleID)
143138

144139
// Convert attributes to CMAB format
145-
cmabAttributes := make([]CMABAttribute, 0, len(attributes))
140+
cmabAttributes := make([]Attribute, 0, len(attributes))
146141
for key, value := range attributes {
147-
cmabAttributes = append(cmabAttributes, CMABAttribute{
142+
cmabAttributes = append(cmabAttributes, Attribute{
148143
ID: key,
149144
Value: value,
150145
Type: "custom_attribute",
151146
})
152147
}
153148

154149
// Create the request body
155-
requestBody := CMABRequest{
156-
Instances: []CMABInstance{
150+
requestBody := Request{
151+
Instances: []Instance{
157152
{
158153
VisitorID: userID,
159154
ExperimentID: ruleID,
@@ -171,26 +166,26 @@ func (c *DefaultCmabClient) FetchDecision(
171166

172167
// If no retry config, just do a single fetch
173168
if c.retryConfig == nil {
174-
return c.doFetch(ctx, url, bodyBytes)
169+
return c.doFetch(context.Background(), url, bodyBytes)
175170
}
176171

177172
// Retry sending request with exponential backoff
173+
var lastErr error
178174
for i := 0; i <= c.retryConfig.MaxRetries; i++ {
179-
// Check if context is done
180-
if ctx.Err() != nil {
181-
return "", fmt.Errorf("context canceled or timed out: %w", ctx.Err())
182-
}
183-
184175
// Make the request
185-
result, err := c.doFetch(ctx, url, bodyBytes)
176+
result, err := c.doFetch(context.Background(), url, bodyBytes)
186177
if err == nil {
187178
return result, nil
188179
}
189180

190-
// If this is the last retry, return the error
191-
if i == c.retryConfig.MaxRetries {
192-
return "", fmt.Errorf("failed to fetch CMAB decision after %d attempts: %w",
193-
c.retryConfig.MaxRetries, err)
181+
lastErr = err
182+
183+
// Don't wait after the last attempt
184+
if i < c.retryConfig.MaxRetries {
185+
backoffDuration := c.retryConfig.InitialBackoff * time.Duration(1<<i)
186+
187+
// Wait for backoff duration
188+
time.Sleep(backoffDuration)
194189
}
195190

196191
// Calculate backoff duration
@@ -204,8 +199,8 @@ func (c *DefaultCmabClient) FetchDecision(
204199

205200
// Wait for backoff duration with context awareness
206201
select {
207-
case <-ctx.Done():
208-
return "", fmt.Errorf("context canceled or timed out during backoff: %w", ctx.Err())
202+
case <-context.Background().Done():
203+
return "", fmt.Errorf("context canceled or timed out during backoff: %w", context.Background().Err())
209204
case <-time.After(backoffDuration):
210205
// Continue with retry
211206
}
@@ -215,7 +210,7 @@ func (c *DefaultCmabClient) FetchDecision(
215210
}
216211

217212
// This should never be reached due to the return in the loop above
218-
return "", fmt.Errorf("unexpected error in retry loop")
213+
return "", fmt.Errorf("failed to fetch CMAB decision after %d attempts: %w", c.retryConfig.MaxRetries, lastErr)
219214
}
220215

221216
// doFetch performs a single fetch operation to the CMAB API
@@ -248,7 +243,7 @@ func (c *DefaultCmabClient) doFetch(ctx context.Context, url string, bodyBytes [
248243
}
249244

250245
// Parse response
251-
var cmabResponse CMABResponse
246+
var cmabResponse Response
252247
if err := json.Unmarshal(respBody, &cmabResponse); err != nil {
253248
return "", fmt.Errorf("failed to unmarshal CMAB response: %w", err)
254249
}
@@ -263,6 +258,6 @@ func (c *DefaultCmabClient) doFetch(ctx context.Context, url string, bodyBytes [
263258
}
264259

265260
// validateResponse validates the CMAB response
266-
func (c *DefaultCmabClient) validateResponse(response CMABResponse) bool {
261+
func (c *DefaultCmabClient) validateResponse(response Response) bool {
267262
return len(response.Predictions) > 0 && response.Predictions[0].VariationID != ""
268263
}

0 commit comments

Comments
 (0)