Skip to content

Commit 6632f73

Browse files
Mat001Mat001
authored
[FSSDK-11990] Add Redis cache support for CMAB following ODP cache pattern (#447)
* Add Redis cache support for CMAB following ODP cache pattern Implement Redis caching for CMAB (Contextual Multi-Armed Bandit) decisions using the same plugin-based architecture as ODP cache. Changes: - Add cmabcache plugin with registry and service implementations - Implement in-memory LRU cache (size: 10000, TTL: 30m) - Implement Redis cache with JSON serialization - Update CMABCacheConfig from struct to service-based map config - Add comprehensive unit and integration tests (all passing) - Update config.yaml with new service-based CMAB cache format - Add cmabcache plugin import to main.go - Fix cache.go to initialize CMAB cache via plugin system - Add tests to improve CMAB config parsing coverage Test coverage: - In-memory cache tests: 8/8 passing - Redis cache tests: 8/8 passing - Integration tests: 8/8 passing - Config parsing coverage improved for lines 149-167 - All package tests: passing * Apply linter formatting fixes * move redis cmab config into existing config * expose cmab prediction endpoint in config
1 parent 0a81450 commit 6632f73

File tree

15 files changed

+1052
-407
lines changed

15 files changed

+1052
-407
lines changed

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ Below is a comprehensive list of available configuration properties.
127127
| sdkKeys | OPTIMIZELY_SDKKEYS | Comma delimited list of SDK keys used to initialize on startup |
128128
| cmab | OPTIMIZELY_CMAB | Complete JSON configuration for CMAB. Format: see example below |
129129
| cmab.cache | OPTIMIZELY_CMAB_CACHE | JSON configuration for just the CMAB cache section. Format: see example below |
130+
| cmab.predictionEndpoint | OPTIMIZELY_CMAB_PREDICTIONENDPOINT | URL template for CMAB prediction API with %s placeholder for experimentId. Default: https://prediction.cmab.optimizely.com/predict/%s |
130131
| cmab.retryConfig | OPTIMIZELY_CMAB_RETRYCONFIG | JSON configuration for just the CMAB retry settings. Format: see example below |
131132
| server.allowedHosts | OPTIMIZELY_SERVER_ALLOWEDHOSTS | List of allowed request host values. Requests whose host value does not match either the configured server.host, or one of these, will be rejected with a 404 response. To match all subdomains, you can use a leading dot (for example `.example.com` matches `my.example.com`, `hello.world.example.com`, etc.). You can use the value `.` to disable allowed host checking, allowing requests with any host. Request host is determined in the following priority order: 1. X-Forwarded-Host header value, 2. Forwarded header host= directive value, 3. Host property of request (see Host under https://pkg.go.dev/net/http#Request). Note: don't include port in these hosts values - port is stripped from the request host before comparing against these. |
132133
| server.batchRequests.maxConcurrency | OPTIMIZELY_SERVER_BATCHREQUESTS_MAXCONCURRENCY | Number of requests running in parallel. Default: 10 |
@@ -150,6 +151,7 @@ Below is a comprehensive list of available configuration properties.
150151
```json
151152
{
152153
"requestTimeout": "5s",
154+
"predictionEndpoint": "https://prediction.cmab.optimizely.com/predict/%s",
153155
"cache": {
154156
"type": "memory",
155157
"size": 2000,

‎cmd/optimizely/main.go‎

Lines changed: 17 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import (
4848
"github.com/optimizely/agent/pkg/optimizely"
4949
"github.com/optimizely/agent/pkg/routers"
5050
"github.com/optimizely/agent/pkg/server"
51+
_ "github.com/optimizely/agent/plugins/cmabcache/all" // Initiate the loading of the cmabCache plugins
5152
_ "github.com/optimizely/agent/plugins/interceptors/all" // Initiate the loading of the userprofileservice plugins
5253
_ "github.com/optimizely/agent/plugins/odpcache/all" // Initiate the loading of the odpCache plugins
5354
_ "github.com/optimizely/agent/plugins/userprofileservice/all" // Initiate the loading of the interceptor plugins
@@ -109,81 +110,59 @@ func loadConfig(v *viper.Viper) *config.AgentConfig {
109110
}
110111

111112
// Handle CMAB configuration using the same approach as UserProfileService
112-
// Check for complete CMAB configuration first
113-
if cmab := v.GetStringMap("cmab"); len(cmab) > 0 {
113+
// Check for complete CMAB configuration first (now under client.cmab)
114+
if cmab := v.GetStringMap("client.cmab"); len(cmab) > 0 {
114115
if timeout, ok := cmab["requestTimeout"].(string); ok {
115116
if duration, err := time.ParseDuration(timeout); err == nil {
116-
conf.CMAB.RequestTimeout = duration
117+
conf.Client.CMAB.RequestTimeout = duration
117118
}
118119
}
119120
if cache, ok := cmab["cache"].(map[string]interface{}); ok {
120-
if cacheType, ok := cache["type"].(string); ok {
121-
conf.CMAB.Cache.Type = cacheType
122-
}
123-
if cacheSize, ok := cache["size"].(float64); ok {
124-
conf.CMAB.Cache.Size = int(cacheSize)
125-
}
126-
if cacheTTL, ok := cache["ttl"].(string); ok {
127-
if duration, err := time.ParseDuration(cacheTTL); err == nil {
128-
conf.CMAB.Cache.TTL = duration
129-
}
130-
}
121+
conf.Client.CMAB.Cache = cache
131122
}
132123
if retryConfig, ok := cmab["retryConfig"].(map[string]interface{}); ok {
133124
if maxRetries, ok := retryConfig["maxRetries"].(float64); ok {
134-
conf.CMAB.RetryConfig.MaxRetries = int(maxRetries)
125+
conf.Client.CMAB.RetryConfig.MaxRetries = int(maxRetries)
135126
}
136127
if initialBackoff, ok := retryConfig["initialBackoff"].(string); ok {
137128
if duration, err := time.ParseDuration(initialBackoff); err == nil {
138-
conf.CMAB.RetryConfig.InitialBackoff = duration
129+
conf.Client.CMAB.RetryConfig.InitialBackoff = duration
139130
}
140131
}
141132
if maxBackoff, ok := retryConfig["maxBackoff"].(string); ok {
142133
if duration, err := time.ParseDuration(maxBackoff); err == nil {
143-
conf.CMAB.RetryConfig.MaxBackoff = duration
134+
conf.Client.CMAB.RetryConfig.MaxBackoff = duration
144135
}
145136
}
146137
if backoffMultiplier, ok := retryConfig["backoffMultiplier"].(float64); ok {
147-
conf.CMAB.RetryConfig.BackoffMultiplier = backoffMultiplier
138+
conf.Client.CMAB.RetryConfig.BackoffMultiplier = backoffMultiplier
148139
}
149140
}
150141
}
151142

152143
// Check for individual map sections
153-
if cmabCache := v.GetStringMap("cmab.cache"); len(cmabCache) > 0 {
154-
if cacheType, ok := cmabCache["type"].(string); ok {
155-
conf.CMAB.Cache.Type = cacheType
156-
}
157-
if cacheSize, ok := cmabCache["size"].(int); ok {
158-
conf.CMAB.Cache.Size = cacheSize
159-
} else if cacheSize, ok := cmabCache["size"].(float64); ok {
160-
conf.CMAB.Cache.Size = int(cacheSize)
161-
}
162-
if cacheTTL, ok := cmabCache["ttl"].(string); ok {
163-
if duration, err := time.ParseDuration(cacheTTL); err == nil {
164-
conf.CMAB.Cache.TTL = duration
165-
}
166-
}
144+
if cmabCache := v.GetStringMap("client.cmab.cache"); len(cmabCache) > 0 {
145+
conf.Client.CMAB.Cache = cmabCache
167146
}
168147

169-
if cmabRetryConfig := v.GetStringMap("cmab.retryConfig"); len(cmabRetryConfig) > 0 {
148+
if cmabRetryConfig := v.GetStringMap("client.cmab.retryConfig"); len(cmabRetryConfig) > 0 {
170149
if maxRetries, ok := cmabRetryConfig["maxRetries"].(int); ok {
171-
conf.CMAB.RetryConfig.MaxRetries = maxRetries
150+
conf.Client.CMAB.RetryConfig.MaxRetries = maxRetries
172151
} else if maxRetries, ok := cmabRetryConfig["maxRetries"].(float64); ok {
173-
conf.CMAB.RetryConfig.MaxRetries = int(maxRetries)
152+
conf.Client.CMAB.RetryConfig.MaxRetries = int(maxRetries)
174153
}
175154
if initialBackoff, ok := cmabRetryConfig["initialBackoff"].(string); ok {
176155
if duration, err := time.ParseDuration(initialBackoff); err == nil {
177-
conf.CMAB.RetryConfig.InitialBackoff = duration
156+
conf.Client.CMAB.RetryConfig.InitialBackoff = duration
178157
}
179158
}
180159
if maxBackoff, ok := cmabRetryConfig["maxBackoff"].(string); ok {
181160
if duration, err := time.ParseDuration(maxBackoff); err == nil {
182-
conf.CMAB.RetryConfig.MaxBackoff = duration
161+
conf.Client.CMAB.RetryConfig.MaxBackoff = duration
183162
}
184163
}
185164
if backoffMultiplier, ok := cmabRetryConfig["backoffMultiplier"].(float64); ok {
186-
conf.CMAB.RetryConfig.BackoffMultiplier = backoffMultiplier
165+
conf.Client.CMAB.RetryConfig.BackoffMultiplier = backoffMultiplier
187166
}
188167
}
189168

‎cmd/optimizely/main_test.go‎

Lines changed: 113 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -189,11 +189,20 @@ func assertCMAB(t *testing.T, cmab config.CMABConfig) {
189189
// Base assertions
190190
assert.Equal(t, 15*time.Second, cmab.RequestTimeout)
191191

192-
// Check cache configuration
192+
// Check cache configuration (now a map[string]interface{})
193193
cache := cmab.Cache
194-
assert.Equal(t, "redis", cache.Type)
195-
assert.Equal(t, 2000, cache.Size)
196-
assert.Equal(t, 45*time.Minute, cache.TTL)
194+
assert.NotNil(t, cache)
195+
assert.Equal(t, "redis", cache["default"])
196+
197+
// Check services configuration
198+
if services, ok := cache["services"].(map[string]interface{}); ok {
199+
if redisConfig, ok := services["redis"].(map[string]interface{}); ok {
200+
// Redis config should have host, database, and timeout fields
201+
assert.NotNil(t, redisConfig["host"])
202+
assert.NotNil(t, redisConfig["database"])
203+
assert.NotNil(t, redisConfig["timeout"])
204+
}
205+
}
197206

198207
// Check retry configuration
199208
retry := cmab.RetryConfig
@@ -204,12 +213,17 @@ func assertCMAB(t *testing.T, cmab config.CMABConfig) {
204213
}
205214

206215
func TestCMABEnvDebug(t *testing.T) {
207-
_ = os.Setenv("OPTIMIZELY_CMAB", `{
216+
_ = os.Setenv("OPTIMIZELY_CLIENT_CMAB", `{
208217
"requestTimeout": "15s",
209218
"cache": {
210-
"type": "redis",
211-
"size": 2000,
212-
"ttl": "45m"
219+
"default": "redis",
220+
"services": {
221+
"redis": {
222+
"host": "localhost:6379",
223+
"database": 0,
224+
"timeout": "45m"
225+
}
226+
}
213227
},
214228
"retryConfig": {
215229
"maxRetries": 5,
@@ -231,40 +245,87 @@ func TestCMABEnvDebug(t *testing.T) {
231245

232246
// Debug: Print the parsed config
233247
fmt.Println("Parsed CMAB config from JSON env var:")
234-
fmt.Printf(" RequestTimeout: %v\n", conf.CMAB.RequestTimeout)
235-
fmt.Printf(" Cache: %+v\n", conf.CMAB.Cache)
236-
fmt.Printf(" RetryConfig: %+v\n", conf.CMAB.RetryConfig)
248+
fmt.Printf(" RequestTimeout: %v\n", conf.Client.CMAB.RequestTimeout)
249+
fmt.Printf(" Cache: %+v\n", conf.Client.CMAB.Cache)
250+
fmt.Printf(" RetryConfig: %+v\n", conf.Client.CMAB.RetryConfig)
237251

238252
// Call assertCMAB
239-
assertCMAB(t, conf.CMAB)
253+
assertCMAB(t, conf.Client.CMAB)
240254
}
241255

242256
func TestCMABPartialConfig(t *testing.T) {
243257
// Clean any existing environment variables
244-
os.Unsetenv("OPTIMIZELY_CMAB")
245-
os.Unsetenv("OPTIMIZELY_CMAB_CACHE")
246-
os.Unsetenv("OPTIMIZELY_CMAB_RETRYCONFIG")
258+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB")
259+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE")
260+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_RETRYCONFIG")
247261

248262
// Set partial configuration through CMAB_CACHE and CMAB_RETRYCONFIG
249-
_ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type": "redis", "size": 3000}`)
250-
_ = os.Setenv("OPTIMIZELY_CMAB_RETRYCONFIG", `{"maxRetries": 10}`)
263+
// Note: Cache is now a service-based map config
264+
_ = os.Setenv("OPTIMIZELY_CLIENT_CMAB_CACHE", `{"default": "redis", "services": {"redis": {"host": "localhost:6379", "database": 0}}}`)
265+
_ = os.Setenv("OPTIMIZELY_CLIENT_CMAB_RETRYCONFIG", `{"maxRetries": 10}`)
251266

252267
// Load config
253268
v := viper.New()
254269
assert.NoError(t, initConfig(v))
255270
conf := loadConfig(v)
256271

257-
// Cache assertions
258-
assert.Equal(t, "redis", conf.CMAB.Cache.Type)
259-
assert.Equal(t, 3000, conf.CMAB.Cache.Size)
272+
// Cache assertions (cache is now map[string]interface{})
273+
assert.NotNil(t, conf.Client.CMAB.Cache)
274+
if defaultCache, ok := conf.Client.CMAB.Cache["default"].(string); ok {
275+
assert.Equal(t, "redis", defaultCache)
276+
}
260277

261278
// RetryConfig assertions
262-
assert.Equal(t, 10, conf.CMAB.RetryConfig.MaxRetries)
279+
assert.Equal(t, 10, conf.Client.CMAB.RetryConfig.MaxRetries)
263280

264281
// Clean up
265-
os.Unsetenv("OPTIMIZELY_CMAB")
266-
os.Unsetenv("OPTIMIZELY_CMAB_CACHE")
267-
os.Unsetenv("OPTIMIZELY_CMAB_RETRYCONFIG")
282+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB")
283+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE")
284+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_RETRYCONFIG")
285+
}
286+
287+
func TestCMABRetryConfigAllFields(t *testing.T) {
288+
// Clean any existing environment variables
289+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB")
290+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE")
291+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_RETRYCONFIG")
292+
293+
// Set all retry config fields via CMAB_RETRYCONFIG to cover lines 154-165
294+
_ = os.Setenv("OPTIMIZELY_CLIENT_CMAB_RETRYCONFIG", `{
295+
"maxRetries": 5,
296+
"initialBackoff": "500ms",
297+
"maxBackoff": "45s",
298+
"backoffMultiplier": 2.5
299+
}`)
300+
301+
defer func() {
302+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_RETRYCONFIG")
303+
}()
304+
305+
v := viper.New()
306+
assert.NoError(t, initConfig(v))
307+
conf := loadConfig(v)
308+
309+
// Verify all retry config fields were parsed correctly
310+
assert.Equal(t, 5, conf.Client.CMAB.RetryConfig.MaxRetries)
311+
assert.Equal(t, 500*time.Millisecond, conf.Client.CMAB.RetryConfig.InitialBackoff)
312+
assert.Equal(t, 45*time.Second, conf.Client.CMAB.RetryConfig.MaxBackoff)
313+
assert.Equal(t, 2.5, conf.Client.CMAB.RetryConfig.BackoffMultiplier)
314+
}
315+
316+
func TestCMABRetryConfigIntMaxRetries(t *testing.T) {
317+
// Test the int type path for maxRetries (line 150) by using viper's Set method
318+
// which will preserve the int type instead of converting to float64
319+
v := viper.New()
320+
assert.NoError(t, initConfig(v))
321+
322+
// Set via viper directly to ensure it's an int, not float64
323+
v.Set("client.cmab.retryConfig.maxRetries", 7)
324+
325+
conf := loadConfig(v)
326+
327+
// Verify maxRetries was parsed as int
328+
assert.Equal(t, 7, conf.Client.CMAB.RetryConfig.MaxRetries)
268329
}
269330

270331
func TestViperYaml(t *testing.T) {
@@ -481,12 +542,17 @@ func TestViperEnv(t *testing.T) {
481542
_ = os.Setenv("OPTIMIZELY_WEBHOOK_PROJECTS_20000_SDKKEYS", "xxx,yyy,zzz")
482543
_ = os.Setenv("OPTIMIZELY_WEBHOOK_PROJECTS_20000_SKIPSIGNATURECHECK", "false")
483544

484-
_ = os.Setenv("OPTIMIZELY_CMAB", `{
545+
_ = os.Setenv("OPTIMIZELY_CLIENT_CMAB", `{
485546
"requestTimeout": "15s",
486547
"cache": {
487-
"type": "redis",
488-
"size": 2000,
489-
"ttl": "45m"
548+
"default": "redis",
549+
"services": {
550+
"redis": {
551+
"host": "localhost:6379",
552+
"database": 0,
553+
"timeout": "45m"
554+
}
555+
}
490556
},
491557
"retryConfig": {
492558
"maxRetries": 5,
@@ -511,7 +577,7 @@ func TestViperEnv(t *testing.T) {
511577
assertAPI(t, actual.API)
512578
//assertWebhook(t, actual.Webhook) // Maps don't appear to be supported
513579
assertRuntime(t, actual.Runtime)
514-
assertCMAB(t, actual.CMAB)
580+
assertCMAB(t, actual.Client.CMAB)
515581
}
516582

517583
func TestLoggingWithIncludeSdkKey(t *testing.T) {
@@ -615,28 +681,32 @@ func Test_initTracing(t *testing.T) {
615681

616682
func TestCMABComplexJSON(t *testing.T) {
617683
// Clean any existing environment variables for CMAB
618-
os.Unsetenv("OPTIMIZELY_CMAB_CACHE_TYPE")
619-
os.Unsetenv("OPTIMIZELY_CMAB_CACHE_SIZE")
620-
os.Unsetenv("OPTIMIZELY_CMAB_CACHE_TTL")
621-
os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_HOST")
622-
os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_PASSWORD")
623-
os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_DATABASE")
684+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE_TYPE")
685+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE_SIZE")
686+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE_TTL")
687+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE_REDIS_HOST")
688+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE_REDIS_PASSWORD")
689+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE_REDIS_DATABASE")
624690

625-
// Set complex JSON environment variable for CMAB cache
626-
_ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type":"redis","size":5000,"ttl":"3h"}`)
691+
// Set complex JSON environment variable for CMAB cache (using new service-based format)
692+
_ = os.Setenv("OPTIMIZELY_CLIENT_CMAB_CACHE", `{"default":"redis","services":{"redis":{"host":"localhost:6379","database":0,"timeout":"3h"}}}`)
627693

628694
defer func() {
629695
// Clean up
630-
os.Unsetenv("OPTIMIZELY_CMAB_CACHE")
696+
os.Unsetenv("OPTIMIZELY_CLIENT_CMAB_CACHE")
631697
}()
632698

633699
v := viper.New()
634700
assert.NoError(t, initConfig(v))
635701
actual := loadConfig(v)
636702

637-
// Test cache settings from JSON environment variable
638-
cache := actual.CMAB.Cache
639-
assert.Equal(t, "redis", cache.Type)
640-
assert.Equal(t, 5000, cache.Size)
641-
assert.Equal(t, 3*time.Hour, cache.TTL)
703+
// Test cache settings from JSON environment variable (cache is now map[string]interface{})
704+
cache := actual.Client.CMAB.Cache
705+
assert.NotNil(t, cache)
706+
if defaultCache, ok := cache["default"].(string); ok {
707+
assert.Equal(t, "redis", defaultCache)
708+
}
709+
if services, ok := cache["services"].(map[string]interface{}); ok {
710+
assert.NotNil(t, services["redis"])
711+
}
642712
}

0 commit comments

Comments
 (0)