From 5187d1de76490fd62cfaef5196b8dec2a1fca5a9 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 20 Jun 2025 11:46:11 -0700 Subject: [PATCH 01/25] add cmab config --- README.md | 53 ++++++++- cmd/optimizely/main.go | 36 +++++- cmd/optimizely/main_test.go | 231 +++++++++++++++++++++++++++++++++++- config.yaml | 34 ++++++ config/config.go | 43 ++++++- config/config_test.go | 52 +++++++- 6 files changed, 442 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 5961a26b..187a468c 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,10 @@ Below is a comprehensive list of available configuration properties. | log.level | OPTIMIZELY_LOG_LEVEL | The log [level](https://github.com/rs/zerolog#leveled-logging) for the agent. Default: info | | log.pretty | OPTIMIZELY_LOG_PRETTY | Flag used to set colorized console output as opposed to structured json logs. Default: false | | name | OPTIMIZELY_NAME | Agent name. Default: optimizely | -| sdkKeys | OPTIMIZELY_SDKKEYS | Comma delimited list of SDK keys used to initialize on startup | +| sdkKeys | OPTIMIZELY_SDKKEYS | Comma delimited list of SDK keys used to initialize on startup | +| cmab | OPTIMIZELY_CMAB | Complete JSON configuration for CMAB. Format: see example below | +| cmab.cache | OPTIMIZELY_CMAB_CACHE | JSON configuration for just the CMAB cache section. Format: see example below | +| cmab.retryConfig | OPTIMIZELY_CMAB_RETRYCONFIG | JSON configuration for just the CMAB retry settings. Format: see example below | | 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. | | server.batchRequests.maxConcurrency | OPTIMIZELY_SERVER_BATCHREQUESTS_MAXCONCURRENCY | Number of requests running in parallel. Default: 10 | | server.batchRequests.operationsLimit | OPTIMIZELY_SERVER_BATCHREQUESTS_OPERATIONSLIMIT | Number of allowed operations. ( will flag an error if the number of operations exeeds this parameter) Default: 500 | @@ -142,6 +145,54 @@ Below is a comprehensive list of available configuration properties. | webhook.projects.<_projectId_>.secret | N/A | Webhook secret used to validate webhook requests originating from the respective projectId | | webhook.projects.<_projectId_>.skipSignatureCheck | N/A | Boolean to indicate whether the signature should be validated. TODO remove in favor of empty secret. | +### CMAB Configuration Examples + +**Complete CMAB Configuration (OPTIMIZELY_CMAB)**: +```json +{ + "enabled": true, + "predictionEndpoint": "https://custom-endpoint.com", + "requestTimeout": "5s", + "cache": { + "type": "redis", + "size": 2000, + "ttl": "45m", + "redis": { + "host": "redis:6379", + "password": "", + "database": 0 + } + }, + "retryConfig": { + "maxRetries": 3, + "initialBackoff": "100ms", + "maxBackoff": "10s", + "backoffMultiplier": 2.0 + } +} + +CMAB Cache Configuration (OPTIMIZELY_CMAB_CACHE): + +{ + "type": "redis", + "size": 2000, + "ttl": "45m", + "redis": { + "host": "redis:6379", + "password": "", + "database": 0 + } +} + +CMAB Retry Configuration (OPTIMIZELY_CMAB_RETRYCONFIG): + +{ + "maxRetries": 3, + "initialBackoff": "100ms", + "maxBackoff": "10s", + "backoffMultiplier": 2.0 +} + More information about configuring Agent can be found in the [Advanced Configuration Notes](https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/advanced-configuration). ### API diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index 5784a5d1..b7ad6976 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -25,6 +25,7 @@ import ( "runtime" "strings" "syscall" + "time" "github.com/rs/zerolog" "github.com/rs/zerolog/log" @@ -98,15 +99,46 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { } // Check if JSON string was set using OPTIMIZELY_CLIENT_USERPROFILESERVICE environment variable - if userProfileService := v.GetStringMap("client.userprofileservice"); userProfileService != nil { + if userProfileService := v.GetStringMap("client.userprofileservice"); len(userProfileService) > 0 { conf.Client.UserProfileService = userProfileService } // Check if JSON string was set using OPTIMIZELY_CLIENT_ODP_SEGMENTSCACHE environment variable - if odpSegmentsCache := v.GetStringMap("client.odp.segmentsCache"); odpSegmentsCache != nil { + if odpSegmentsCache := v.GetStringMap("client.odp.segmentsCache"); len(odpSegmentsCache) > 0 { conf.Client.ODP.SegmentsCache = odpSegmentsCache } + // Handle CMAB configuration using the same approach as UserProfileService + // Check for complete CMAB configuration first + if cmab := v.GetStringMap("cmab"); len(cmab) > 0 { + if enabled, ok := cmab["enabled"].(bool); ok { + conf.CMAB.Enabled = enabled + } + if endpoint, ok := cmab["predictionEndpoint"].(string); ok { + conf.CMAB.PredictionEndpoint = endpoint + } + if timeout, ok := cmab["requestTimeout"].(string); ok { + if duration, err := time.ParseDuration(timeout); err == nil { + conf.CMAB.RequestTimeout = duration + } + } + if cache, ok := cmab["cache"].(map[string]interface{}); ok { + conf.CMAB.Cache = cache + } + if retryConfig, ok := cmab["retryConfig"].(map[string]interface{}); ok { + conf.CMAB.RetryConfig = retryConfig + } + } + + // Check for individual map sections + if cmabCache := v.GetStringMap("cmab.cache"); len(cmabCache) > 0 { + conf.CMAB.Cache = cmabCache + } + + if cmabRetryConfig := v.GetStringMap("cmab.retryConfig"); len(cmabRetryConfig) > 0 { + conf.CMAB.RetryConfig = cmabRetryConfig + } + return conf } diff --git a/cmd/optimizely/main_test.go b/cmd/optimizely/main_test.go index 72ae36fa..42ea8d3b 100644 --- a/cmd/optimizely/main_test.go +++ b/cmd/optimizely/main_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020,2022-2023, Optimizely, Inc. and contributors * + * Copyright 2019-2020,2022-2025, 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. * @@ -17,7 +17,9 @@ package main import ( + "fmt" "os" + "strings" "testing" "time" @@ -178,6 +180,168 @@ func assertWebhook(t *testing.T, actual config.WebhookConfig) { assert.False(t, actual.Projects[20000].SkipSignatureCheck) } +func assertCMAB(t *testing.T, cmab config.CMABConfig) { + fmt.Println("In assertCMAB, received CMAB config:") + fmt.Printf(" Enabled: %v\n", cmab.Enabled) + fmt.Printf(" PredictionEndpoint: %s\n", cmab.PredictionEndpoint) + fmt.Printf(" RequestTimeout: %v\n", cmab.RequestTimeout) + fmt.Printf(" Cache: %#v\n", cmab.Cache) + fmt.Printf(" RetryConfig: %#v\n", cmab.RetryConfig) + + // Base assertions + assert.True(t, cmab.Enabled) + assert.Equal(t, "https://custom-cmab-endpoint.example.com", cmab.PredictionEndpoint) + assert.Equal(t, 15*time.Second, cmab.RequestTimeout) + + // Check if cache map is initialized + cacheMap := cmab.Cache + if cacheMap == nil { + t.Fatal("Cache map is nil") + } + + // Debug cache type + cacheTypeValue := cacheMap["type"] + fmt.Printf("Cache type: %v (%T)\n", cacheTypeValue, cacheTypeValue) + assert.Equal(t, "redis", cacheTypeValue) + + // Debug cache size + cacheSizeValue := cacheMap["size"] + fmt.Printf("Cache size: %v (%T)\n", cacheSizeValue, cacheSizeValue) + sizeValue, ok := cacheSizeValue.(float64) + assert.True(t, ok, "Cache size should be float64") + assert.Equal(t, float64(2000), sizeValue) + + // Cache TTL + cacheTTLValue := cacheMap["ttl"] + fmt.Printf("Cache TTL: %v (%T)\n", cacheTTLValue, cacheTTLValue) + assert.Equal(t, "45m", cacheTTLValue) + + // Redis settings + redisValue := cacheMap["redis"] + fmt.Printf("Redis: %v (%T)\n", redisValue, redisValue) + redisMap, ok := redisValue.(map[string]interface{}) + assert.True(t, ok, "Redis should be a map") + + if !ok { + t.Fatal("Redis is not a map") + } + + redisHost := redisMap["host"] + fmt.Printf("Redis host: %v (%T)\n", redisHost, redisHost) + assert.Equal(t, "redis.example.com:6379", redisHost) + + redisPassword := redisMap["password"] + fmt.Printf("Redis password: %v (%T)\n", redisPassword, redisPassword) + assert.Equal(t, "password123", redisPassword) + + redisDBValue := redisMap["database"] + fmt.Printf("Redis DB: %v (%T)\n", redisDBValue, redisDBValue) + dbValue, ok := redisDBValue.(float64) + assert.True(t, ok, "Redis DB should be float64") + assert.Equal(t, float64(2), dbValue) + + // Retry settings + retryMap := cmab.RetryConfig + if retryMap == nil { + t.Fatal("RetryConfig map is nil") + } + + // Max retries + maxRetriesValue := retryMap["maxRetries"] + fmt.Printf("maxRetries: %v (%T)\n", maxRetriesValue, maxRetriesValue) + maxRetries, ok := maxRetriesValue.(float64) + assert.True(t, ok, "maxRetries should be float64") + assert.Equal(t, float64(5), maxRetries) + + // Check other retry settings + fmt.Printf("initialBackoff: %v (%T)\n", retryMap["initialBackoff"], retryMap["initialBackoff"]) + assert.Equal(t, "200ms", retryMap["initialBackoff"]) + + fmt.Printf("maxBackoff: %v (%T)\n", retryMap["maxBackoff"], retryMap["maxBackoff"]) + assert.Equal(t, "30s", retryMap["maxBackoff"]) + + fmt.Printf("backoffMultiplier: %v (%T)\n", retryMap["backoffMultiplier"], retryMap["backoffMultiplier"]) + assert.Equal(t, 3.0, retryMap["backoffMultiplier"]) +} + +func TestCMABEnvDebug(t *testing.T) { + _ = os.Setenv("OPTIMIZELY_CMAB", `{ + "enabled": true, + "predictionEndpoint": "https://custom-cmab-endpoint.example.com", + "requestTimeout": "15s", + "cache": { + "type": "redis", + "size": 2000, + "ttl": "45m", + "redis": { + "host": "redis.example.com:6379", + "password": "password123", + "database": 2 + } + }, + "retryConfig": { + "maxRetries": 5, + "initialBackoff": "200ms", + "maxBackoff": "30s", + "backoffMultiplier": 3.0 + } + }`) + + // Load config using Viper + v := viper.New() + v.SetEnvPrefix("optimizely") + v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) + v.AutomaticEnv() + + // Create config + assert.NoError(t, initConfig(v)) + conf := loadConfig(v) + + // Debug: Print the parsed config + fmt.Println("Parsed CMAB config from JSON env var:") + fmt.Printf(" Enabled: %v\n", conf.CMAB.Enabled) + fmt.Printf(" PredictionEndpoint: %s\n", conf.CMAB.PredictionEndpoint) + fmt.Printf(" RequestTimeout: %v\n", conf.CMAB.RequestTimeout) + fmt.Printf(" Cache: %+v\n", conf.CMAB.Cache) + fmt.Printf(" RetryConfig: %+v\n", conf.CMAB.RetryConfig) + + // Call assertCMAB + assertCMAB(t, conf.CMAB) +} + +func TestCMABPartialConfig(t *testing.T) { + // Clean any existing environment variables + os.Unsetenv("OPTIMIZELY_CMAB") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE") + os.Unsetenv("OPTIMIZELY_CMAB_RETRYCONFIG") + + // Set partial configuration through CMAB_CACHE and CMAB_RETRYCONFIG + _ = os.Setenv("OPTIMIZELY_CMAB", `{"enabled": true, "predictionEndpoint": "https://base-endpoint.example.com"}`) + _ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type": "redis", "size": 3000}`) + _ = os.Setenv("OPTIMIZELY_CMAB_RETRYCONFIG", `{"maxRetries": 10}`) + + // Load config + v := viper.New() + assert.NoError(t, initConfig(v)) + conf := loadConfig(v) + + // Base assertions + assert.True(t, conf.CMAB.Enabled) + assert.Equal(t, "https://base-endpoint.example.com", conf.CMAB.PredictionEndpoint) + + // Cache assertions + assert.Equal(t, "redis", conf.CMAB.Cache["type"]) + assert.Equal(t, float64(3000), conf.CMAB.Cache["size"]) + + // RetryConfig assertions + assert.Equal(t, float64(10), conf.CMAB.RetryConfig["maxRetries"]) + + // Clean up + os.Unsetenv("OPTIMIZELY_CMAB") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE") + os.Unsetenv("OPTIMIZELY_CMAB_RETRYCONFIG") +} + func TestViperYaml(t *testing.T) { v := viper.New() v.Set("config.filename", "./testdata/default.yaml") @@ -392,6 +556,28 @@ func TestViperEnv(t *testing.T) { _ = os.Setenv("OPTIMIZELY_WEBHOOK_PROJECTS_20000_SDKKEYS", "xxx,yyy,zzz") _ = os.Setenv("OPTIMIZELY_WEBHOOK_PROJECTS_20000_SKIPSIGNATURECHECK", "false") + _ = os.Setenv("OPTIMIZELY_CMAB", `{ + "enabled": true, + "predictionEndpoint": "https://custom-cmab-endpoint.example.com", + "requestTimeout": "15s", + "cache": { + "type": "redis", + "size": 2000, + "ttl": "45m", + "redis": { + "host": "redis.example.com:6379", + "password": "password123", + "database": 2 + } + }, + "retryConfig": { + "maxRetries": 5, + "initialBackoff": "200ms", + "maxBackoff": "30s", + "backoffMultiplier": 3.0 + } + }`) + _ = os.Setenv("OPTIMIZELY_RUNTIME_BLOCKPROFILERATE", "1") _ = os.Setenv("OPTIMIZELY_RUNTIME_MUTEXPROFILEFRACTION", "2") @@ -407,6 +593,7 @@ func TestViperEnv(t *testing.T) { assertAPI(t, actual.API) //assertWebhook(t, actual.Webhook) // Maps don't appear to be supported assertRuntime(t, actual.Runtime) + assertCMAB(t, actual.CMAB) } func TestLoggingWithIncludeSdkKey(t *testing.T) { @@ -507,3 +694,45 @@ func Test_initTracing(t *testing.T) { }) } } + +func TestCMABComplexJSON(t *testing.T) { + // Clean any existing environment variables for CMAB + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_TYPE") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_SIZE") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_TTL") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_HOST") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_PASSWORD") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_DATABASE") + + // Set complex JSON environment variable for CMAB cache + _ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type":"redis","size":5000,"ttl":"3h","redis":{"host":"json-redis.example.com:6379","password":"json-password","database":4}}`) + + defer func() { + // Clean up + os.Unsetenv("OPTIMIZELY_CMAB_CACHE") + }() + + v := viper.New() + assert.NoError(t, initConfig(v)) + actual := loadConfig(v) + + // Test cache settings from JSON environment variable + cacheMap := actual.CMAB.Cache + assert.Equal(t, "redis", cacheMap["type"]) + + // Account for JSON unmarshaling to float64 + size, ok := cacheMap["size"].(float64) + assert.True(t, ok, "Size should be a float64") + assert.Equal(t, float64(5000), size) + + assert.Equal(t, "3h", cacheMap["ttl"]) + + redisMap, ok := cacheMap["redis"].(map[string]interface{}) + assert.True(t, ok, "Redis config should be a map") + assert.Equal(t, "json-redis.example.com:6379", redisMap["host"]) + assert.Equal(t, "json-password", redisMap["password"]) + + db, ok := redisMap["database"].(float64) + assert.True(t, ok, "Database should be a float64") + assert.Equal(t, float64(4), db) +} diff --git a/config.yaml b/config.yaml index d3145d3b..082075ff 100644 --- a/config.yaml +++ b/config.yaml @@ -262,3 +262,37 @@ synchronization: datafile: enable: false default: "redis" + +## +## cmab: Contextual Multi-Armed Bandit configuration +## +cmab: + ## enable or disable CMAB functionality + enabled: false + ## URL for CMAB prediction service + predictionEndpoint: "https://prediction.cmab.optimizely.com" + ## timeout for CMAB API requests + requestTimeout: 10s + ## CMAB cache configuration + cache: + ## cache type (memory or redis) + type: "memory" + ## maximum number of entries for in-memory cache + size: 1000 + ## time-to-live for cached decisions + ttl: 30m + ## Redis configuration (if using redis cache) + redis: + host: "localhost:6379" + password: "" + database: 0 + ## retry configuration for CMAB API requests + retryConfig: + ## maximum number of retry attempts + maxRetries: 3 + ## initial backoff duration + initialBackoff: 100ms + ## maximum backoff duration + maxBackoff: 10s + ## multiplier for exponential backoff + backoffMultiplier: 2.0 diff --git a/config/config.go b/config/config.go index 9e652910..f3291d3e 100644 --- a/config/config.go +++ b/config/config.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020,2022-2023, Optimizely, Inc. and contributors * + * Copyright 2019-2020,2022-2025, 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. * @@ -140,8 +140,28 @@ func NewDefaultConfig() *AgentConfig { Default: "redis", }, }, + CMAB: CMABConfig{ + Enabled: false, + PredictionEndpoint: "https://prediction.cmab.optimizely.com", + RequestTimeout: 10 * time.Second, + Cache: map[string]interface{}{ + "type": "memory", + "size": 1000, + "ttl": "30m", + "redis": map[string]interface{}{ + "host": "localhost:6379", + "password": "", + "database": 0, + }, + }, + RetryConfig: map[string]interface{}{ + "maxRetries": 3, + "initialBackoff": "100ms", + "maxBackoff": "10s", + "backoffMultiplier": 2.0, + }, + }, } - return &config } @@ -162,6 +182,7 @@ type AgentConfig struct { Server ServerConfig `json:"server"` Webhook WebhookConfig `json:"webhook"` Synchronization SyncConfig `json:"synchronization"` + CMAB CMABConfig `json:"cmab"` } // SyncConfig contains Synchronization configuration for the multiple Agent nodes @@ -387,3 +408,21 @@ type RuntimeConfig struct { // (For n>1 the details of sampling may change.) MutexProfileFraction int `json:"mutexProfileFraction"` } + +// CMABConfig holds configuration for the Contextual Multi-Armed Bandit functionality +type CMABConfig struct { + // Enabled indicates whether the CMAB functionality is enabled + Enabled bool `json:"enabled"` + + // PredictionEndpoint is the URL for CMAB predictions + PredictionEndpoint string `json:"predictionEndpoint"` + + // RequestTimeout is the timeout for CMAB API requests + RequestTimeout time.Duration `json:"requestTimeout"` + + // Cache configuration + Cache map[string]interface{} `json:"cache"` + + // RetryConfig for CMAB API requests + RetryConfig map[string]interface{} `json:"retryConfig"` +} diff --git a/config/config_test.go b/config/config_test.go index 917cd498..a5f3a8a4 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020,2022-2023, Optimizely, Inc. and contributors * + * Copyright 2019-2020,2022-2025, 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. * @@ -99,6 +99,29 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(t, 0, conf.Runtime.BlockProfileRate) assert.Equal(t, 0, conf.Runtime.MutexProfileFraction) + + // CMAB configuration + assert.False(t, conf.CMAB.Enabled) + assert.Equal(t, "https://prediction.cmab.optimizely.com", conf.CMAB.PredictionEndpoint) + assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) + + // Test cache settings as maps + cacheMap := conf.CMAB.Cache + assert.Equal(t, "memory", cacheMap["type"]) + assert.Equal(t, 1000, cacheMap["size"]) + assert.Equal(t, "30m", cacheMap["ttl"]) + + redisMap := cacheMap["redis"].(map[string]interface{}) + assert.Equal(t, "localhost:6379", redisMap["host"]) + assert.Equal(t, "", redisMap["password"]) + assert.Equal(t, 0, redisMap["database"]) + + // Test retry settings as maps + retryMap := conf.CMAB.RetryConfig + assert.Equal(t, 3, retryMap["maxRetries"]) + assert.Equal(t, "100ms", retryMap["initialBackoff"]) + assert.Equal(t, "10s", retryMap["maxBackoff"]) + assert.Equal(t, 2.0, retryMap["backoffMultiplier"]) } type logObservation struct { @@ -233,3 +256,30 @@ func TestServerConfig_GetAllowedHosts(t *testing.T) { assert.Contains(t, allowedHosts, "localhost") assert.Contains(t, allowedHosts, "special.test.host") } + +func TestDefaultCMABConfig(t *testing.T) { + conf := NewDefaultConfig() + + // Test default values + assert.False(t, conf.CMAB.Enabled) + assert.Equal(t, "https://prediction.cmab.optimizely.com", conf.CMAB.PredictionEndpoint) + assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) + + // Test default cache settings as maps + cacheMap := conf.CMAB.Cache + assert.Equal(t, "memory", cacheMap["type"]) + assert.Equal(t, 1000, cacheMap["size"]) + assert.Equal(t, "30m", cacheMap["ttl"]) + + redisMap := cacheMap["redis"].(map[string]interface{}) + assert.Equal(t, "localhost:6379", redisMap["host"]) + assert.Equal(t, "", redisMap["password"]) + assert.Equal(t, 0, redisMap["database"]) + + // Test default retry settings as maps + retryMap := conf.CMAB.RetryConfig + assert.Equal(t, 3, retryMap["maxRetries"]) + assert.Equal(t, "100ms", retryMap["initialBackoff"]) + assert.Equal(t, "10s", retryMap["maxBackoff"]) + assert.Equal(t, 2.0, retryMap["backoffMultiplier"]) +} From c2563795f2a78374cb14d9df6523c608878ef4d1 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 20 Jun 2025 15:28:07 -0700 Subject: [PATCH 02/25] remove enabled config, remove 3 ENV vars for cmab, only use a single one --- README.md | 39 +++++-------------------------------- cmd/optimizely/main.go | 3 --- cmd/optimizely/main_test.go | 4 ---- config.yaml | 9 +-------- config/config.go | 11 +---------- config/config_test.go | 16 ++------------- 6 files changed, 9 insertions(+), 73 deletions(-) diff --git a/README.md b/README.md index 187a468c..2c23aa84 100644 --- a/README.md +++ b/README.md @@ -145,23 +145,16 @@ Below is a comprehensive list of available configuration properties. | webhook.projects.<_projectId_>.secret | N/A | Webhook secret used to validate webhook requests originating from the respective projectId | | webhook.projects.<_projectId_>.skipSignatureCheck | N/A | Boolean to indicate whether the signature should be validated. TODO remove in favor of empty secret. | -### CMAB Configuration Examples +### CMAB Configuration Example -**Complete CMAB Configuration (OPTIMIZELY_CMAB)**: ```json { - "enabled": true, - "predictionEndpoint": "https://custom-endpoint.com", + "predictionEndpoint": "https://prediction.cmab.optimizely.com/predict/%s", "requestTimeout": "5s", "cache": { - "type": "redis", + "type": "memory", "size": 2000, - "ttl": "45m", - "redis": { - "host": "redis:6379", - "password": "", - "database": 0 - } + "ttl": "45m" }, "retryConfig": { "maxRetries": 3, @@ -169,29 +162,7 @@ Below is a comprehensive list of available configuration properties. "maxBackoff": "10s", "backoffMultiplier": 2.0 } -} - -CMAB Cache Configuration (OPTIMIZELY_CMAB_CACHE): - -{ - "type": "redis", - "size": 2000, - "ttl": "45m", - "redis": { - "host": "redis:6379", - "password": "", - "database": 0 - } -} - -CMAB Retry Configuration (OPTIMIZELY_CMAB_RETRYCONFIG): - -{ - "maxRetries": 3, - "initialBackoff": "100ms", - "maxBackoff": "10s", - "backoffMultiplier": 2.0 -} +}``` More information about configuring Agent can be found in the [Advanced Configuration Notes](https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/advanced-configuration). diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index b7ad6976..1f00d09e 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -111,9 +111,6 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { // Handle CMAB configuration using the same approach as UserProfileService // Check for complete CMAB configuration first if cmab := v.GetStringMap("cmab"); len(cmab) > 0 { - if enabled, ok := cmab["enabled"].(bool); ok { - conf.CMAB.Enabled = enabled - } if endpoint, ok := cmab["predictionEndpoint"].(string); ok { conf.CMAB.PredictionEndpoint = endpoint } diff --git a/cmd/optimizely/main_test.go b/cmd/optimizely/main_test.go index 42ea8d3b..88c837d5 100644 --- a/cmd/optimizely/main_test.go +++ b/cmd/optimizely/main_test.go @@ -182,14 +182,12 @@ func assertWebhook(t *testing.T, actual config.WebhookConfig) { func assertCMAB(t *testing.T, cmab config.CMABConfig) { fmt.Println("In assertCMAB, received CMAB config:") - fmt.Printf(" Enabled: %v\n", cmab.Enabled) fmt.Printf(" PredictionEndpoint: %s\n", cmab.PredictionEndpoint) fmt.Printf(" RequestTimeout: %v\n", cmab.RequestTimeout) fmt.Printf(" Cache: %#v\n", cmab.Cache) fmt.Printf(" RetryConfig: %#v\n", cmab.RetryConfig) // Base assertions - assert.True(t, cmab.Enabled) assert.Equal(t, "https://custom-cmab-endpoint.example.com", cmab.PredictionEndpoint) assert.Equal(t, 15*time.Second, cmab.RequestTimeout) @@ -299,7 +297,6 @@ func TestCMABEnvDebug(t *testing.T) { // Debug: Print the parsed config fmt.Println("Parsed CMAB config from JSON env var:") - fmt.Printf(" Enabled: %v\n", conf.CMAB.Enabled) fmt.Printf(" PredictionEndpoint: %s\n", conf.CMAB.PredictionEndpoint) fmt.Printf(" RequestTimeout: %v\n", conf.CMAB.RequestTimeout) fmt.Printf(" Cache: %+v\n", conf.CMAB.Cache) @@ -326,7 +323,6 @@ func TestCMABPartialConfig(t *testing.T) { conf := loadConfig(v) // Base assertions - assert.True(t, conf.CMAB.Enabled) assert.Equal(t, "https://base-endpoint.example.com", conf.CMAB.PredictionEndpoint) // Cache assertions diff --git a/config.yaml b/config.yaml index 082075ff..76de2b40 100644 --- a/config.yaml +++ b/config.yaml @@ -267,10 +267,8 @@ synchronization: ## cmab: Contextual Multi-Armed Bandit configuration ## cmab: - ## enable or disable CMAB functionality - enabled: false ## URL for CMAB prediction service - predictionEndpoint: "https://prediction.cmab.optimizely.com" + predictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s" ## timeout for CMAB API requests requestTimeout: 10s ## CMAB cache configuration @@ -281,11 +279,6 @@ cmab: size: 1000 ## time-to-live for cached decisions ttl: 30m - ## Redis configuration (if using redis cache) - redis: - host: "localhost:6379" - password: "" - database: 0 ## retry configuration for CMAB API requests retryConfig: ## maximum number of retry attempts diff --git a/config/config.go b/config/config.go index f3291d3e..4da7e95d 100644 --- a/config/config.go +++ b/config/config.go @@ -141,18 +141,12 @@ func NewDefaultConfig() *AgentConfig { }, }, CMAB: CMABConfig{ - Enabled: false, - PredictionEndpoint: "https://prediction.cmab.optimizely.com", + PredictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s", RequestTimeout: 10 * time.Second, Cache: map[string]interface{}{ "type": "memory", "size": 1000, "ttl": "30m", - "redis": map[string]interface{}{ - "host": "localhost:6379", - "password": "", - "database": 0, - }, }, RetryConfig: map[string]interface{}{ "maxRetries": 3, @@ -411,9 +405,6 @@ type RuntimeConfig struct { // CMABConfig holds configuration for the Contextual Multi-Armed Bandit functionality type CMABConfig struct { - // Enabled indicates whether the CMAB functionality is enabled - Enabled bool `json:"enabled"` - // PredictionEndpoint is the URL for CMAB predictions PredictionEndpoint string `json:"predictionEndpoint"` diff --git a/config/config_test.go b/config/config_test.go index a5f3a8a4..42de8f74 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -101,8 +101,7 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(t, 0, conf.Runtime.MutexProfileFraction) // CMAB configuration - assert.False(t, conf.CMAB.Enabled) - assert.Equal(t, "https://prediction.cmab.optimizely.com", conf.CMAB.PredictionEndpoint) + assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) // Test cache settings as maps @@ -111,11 +110,6 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(t, 1000, cacheMap["size"]) assert.Equal(t, "30m", cacheMap["ttl"]) - redisMap := cacheMap["redis"].(map[string]interface{}) - assert.Equal(t, "localhost:6379", redisMap["host"]) - assert.Equal(t, "", redisMap["password"]) - assert.Equal(t, 0, redisMap["database"]) - // Test retry settings as maps retryMap := conf.CMAB.RetryConfig assert.Equal(t, 3, retryMap["maxRetries"]) @@ -261,8 +255,7 @@ func TestDefaultCMABConfig(t *testing.T) { conf := NewDefaultConfig() // Test default values - assert.False(t, conf.CMAB.Enabled) - assert.Equal(t, "https://prediction.cmab.optimizely.com", conf.CMAB.PredictionEndpoint) + assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) // Test default cache settings as maps @@ -271,11 +264,6 @@ func TestDefaultCMABConfig(t *testing.T) { assert.Equal(t, 1000, cacheMap["size"]) assert.Equal(t, "30m", cacheMap["ttl"]) - redisMap := cacheMap["redis"].(map[string]interface{}) - assert.Equal(t, "localhost:6379", redisMap["host"]) - assert.Equal(t, "", redisMap["password"]) - assert.Equal(t, 0, redisMap["database"]) - // Test default retry settings as maps retryMap := conf.CMAB.RetryConfig assert.Equal(t, 3, retryMap["maxRetries"]) From f3430cf8d3bf1ae021affd5f12936ab1cd092576 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 20 Jun 2025 15:30:12 -0700 Subject: [PATCH 03/25] cleanup readme --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2c23aa84..a5f9ded6 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,8 @@ Below is a comprehensive list of available configuration properties. "maxBackoff": "10s", "backoffMultiplier": 2.0 } -}``` +} +``` More information about configuring Agent can be found in the [Advanced Configuration Notes](https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/advanced-configuration). From 67382b04a3a1fecc545f5b6e862b3505867710ec Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Wed, 25 Jun 2025 14:54:18 -0700 Subject: [PATCH 04/25] add cmab logic to agent --- README.md | 1 - cmd/optimizely/main.go | 3 - cmd/optimizely/main_test.go | 11 - config.yaml | 2 - config/config.go | 7 +- config/config_test.go | 2 - pkg/optimizely/cache.go | 66 ++++- pkg/optimizely/cache_test.go | 282 ++++++++++++++++++- pkg/optimizely/client.go | 2 +- pkg/optimizely/optimizelytest/config.go | 20 ++ plugins/odpcache/registry.go | 2 +- plugins/odpcache/registry_test.go | 2 +- plugins/odpcache/services/in_memory_cache.go | 2 +- plugins/odpcache/services/redis_cache.go | 2 +- 14 files changed, 373 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index a5f9ded6..15ff2e36 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,6 @@ Below is a comprehensive list of available configuration properties. ```json { - "predictionEndpoint": "https://prediction.cmab.optimizely.com/predict/%s", "requestTimeout": "5s", "cache": { "type": "memory", diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index 1f00d09e..29dfaadf 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -111,9 +111,6 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { // Handle CMAB configuration using the same approach as UserProfileService // Check for complete CMAB configuration first if cmab := v.GetStringMap("cmab"); len(cmab) > 0 { - if endpoint, ok := cmab["predictionEndpoint"].(string); ok { - conf.CMAB.PredictionEndpoint = endpoint - } if timeout, ok := cmab["requestTimeout"].(string); ok { if duration, err := time.ParseDuration(timeout); err == nil { conf.CMAB.RequestTimeout = duration diff --git a/cmd/optimizely/main_test.go b/cmd/optimizely/main_test.go index 88c837d5..48336a6f 100644 --- a/cmd/optimizely/main_test.go +++ b/cmd/optimizely/main_test.go @@ -182,13 +182,11 @@ func assertWebhook(t *testing.T, actual config.WebhookConfig) { func assertCMAB(t *testing.T, cmab config.CMABConfig) { fmt.Println("In assertCMAB, received CMAB config:") - fmt.Printf(" PredictionEndpoint: %s\n", cmab.PredictionEndpoint) fmt.Printf(" RequestTimeout: %v\n", cmab.RequestTimeout) fmt.Printf(" Cache: %#v\n", cmab.Cache) fmt.Printf(" RetryConfig: %#v\n", cmab.RetryConfig) // Base assertions - assert.Equal(t, "https://custom-cmab-endpoint.example.com", cmab.PredictionEndpoint) assert.Equal(t, 15*time.Second, cmab.RequestTimeout) // Check if cache map is initialized @@ -264,8 +262,6 @@ func assertCMAB(t *testing.T, cmab config.CMABConfig) { func TestCMABEnvDebug(t *testing.T) { _ = os.Setenv("OPTIMIZELY_CMAB", `{ - "enabled": true, - "predictionEndpoint": "https://custom-cmab-endpoint.example.com", "requestTimeout": "15s", "cache": { "type": "redis", @@ -297,7 +293,6 @@ func TestCMABEnvDebug(t *testing.T) { // Debug: Print the parsed config fmt.Println("Parsed CMAB config from JSON env var:") - fmt.Printf(" PredictionEndpoint: %s\n", conf.CMAB.PredictionEndpoint) fmt.Printf(" RequestTimeout: %v\n", conf.CMAB.RequestTimeout) fmt.Printf(" Cache: %+v\n", conf.CMAB.Cache) fmt.Printf(" RetryConfig: %+v\n", conf.CMAB.RetryConfig) @@ -313,7 +308,6 @@ func TestCMABPartialConfig(t *testing.T) { os.Unsetenv("OPTIMIZELY_CMAB_RETRYCONFIG") // Set partial configuration through CMAB_CACHE and CMAB_RETRYCONFIG - _ = os.Setenv("OPTIMIZELY_CMAB", `{"enabled": true, "predictionEndpoint": "https://base-endpoint.example.com"}`) _ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type": "redis", "size": 3000}`) _ = os.Setenv("OPTIMIZELY_CMAB_RETRYCONFIG", `{"maxRetries": 10}`) @@ -322,9 +316,6 @@ func TestCMABPartialConfig(t *testing.T) { assert.NoError(t, initConfig(v)) conf := loadConfig(v) - // Base assertions - assert.Equal(t, "https://base-endpoint.example.com", conf.CMAB.PredictionEndpoint) - // Cache assertions assert.Equal(t, "redis", conf.CMAB.Cache["type"]) assert.Equal(t, float64(3000), conf.CMAB.Cache["size"]) @@ -553,8 +544,6 @@ func TestViperEnv(t *testing.T) { _ = os.Setenv("OPTIMIZELY_WEBHOOK_PROJECTS_20000_SKIPSIGNATURECHECK", "false") _ = os.Setenv("OPTIMIZELY_CMAB", `{ - "enabled": true, - "predictionEndpoint": "https://custom-cmab-endpoint.example.com", "requestTimeout": "15s", "cache": { "type": "redis", diff --git a/config.yaml b/config.yaml index 76de2b40..6dc98b31 100644 --- a/config.yaml +++ b/config.yaml @@ -267,8 +267,6 @@ synchronization: ## cmab: Contextual Multi-Armed Bandit configuration ## cmab: - ## URL for CMAB prediction service - predictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s" ## timeout for CMAB API requests requestTimeout: 10s ## CMAB cache configuration diff --git a/config/config.go b/config/config.go index 4da7e95d..9a90f08a 100644 --- a/config/config.go +++ b/config/config.go @@ -141,8 +141,7 @@ func NewDefaultConfig() *AgentConfig { }, }, CMAB: CMABConfig{ - PredictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s", - RequestTimeout: 10 * time.Second, + RequestTimeout: 10 * time.Second, Cache: map[string]interface{}{ "type": "memory", "size": 1000, @@ -230,6 +229,7 @@ type ClientConfig struct { SdkKeyRegex string `json:"sdkKeyRegex"` UserProfileService UserProfileServiceConfigs `json:"userProfileService"` ODP OdpConfig `json:"odp"` + CMAB CMABConfig `json:"cmab" mapstructure:"cmab"` } // OdpConfig holds the odp configuration @@ -405,9 +405,6 @@ type RuntimeConfig struct { // CMABConfig holds configuration for the Contextual Multi-Armed Bandit functionality type CMABConfig struct { - // PredictionEndpoint is the URL for CMAB predictions - PredictionEndpoint string `json:"predictionEndpoint"` - // RequestTimeout is the timeout for CMAB API requests RequestTimeout time.Duration `json:"requestTimeout"` diff --git a/config/config_test.go b/config/config_test.go index 42de8f74..1f23eed0 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -101,7 +101,6 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(t, 0, conf.Runtime.MutexProfileFraction) // CMAB configuration - assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) // Test cache settings as maps @@ -255,7 +254,6 @@ func TestDefaultCMABConfig(t *testing.T) { conf := NewDefaultConfig() // Test default values - assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) // Test default cache settings as maps diff --git a/pkg/optimizely/cache.go b/pkg/optimizely/cache.go index 093f6585..17763d54 100644 --- a/pkg/optimizely/cache.go +++ b/pkg/optimizely/cache.go @@ -21,9 +21,11 @@ import ( "context" "encoding/json" "errors" + "net/http" "regexp" "strings" "sync" + "time" cmap "github.com/orcaman/concurrent-map" "github.com/rs/zerolog/log" @@ -33,13 +35,14 @@ import ( "github.com/optimizely/agent/pkg/syncer" "github.com/optimizely/agent/plugins/odpcache" "github.com/optimizely/agent/plugins/userprofileservice" + odpCachePkg "github.com/optimizely/go-sdk/v2/pkg/cache" "github.com/optimizely/go-sdk/v2/pkg/client" + cmab "github.com/optimizely/go-sdk/v2/pkg/cmab" sdkconfig "github.com/optimizely/go-sdk/v2/pkg/config" "github.com/optimizely/go-sdk/v2/pkg/decision" "github.com/optimizely/go-sdk/v2/pkg/event" "github.com/optimizely/go-sdk/v2/pkg/logging" "github.com/optimizely/go-sdk/v2/pkg/odp" - odpCachePkg "github.com/optimizely/go-sdk/v2/pkg/odp/cache" odpEventPkg "github.com/optimizely/go-sdk/v2/pkg/odp/event" odpSegmentPkg "github.com/optimizely/go-sdk/v2/pkg/odp/segment" "github.com/optimizely/go-sdk/v2/pkg/tracing" @@ -312,6 +315,67 @@ func defaultLoader( ) clientOptions = append(clientOptions, client.WithOdpManager(odpManager)) + // Parse CMAB cache configuration + cacheSize := 1000 // default + cacheTTL := 30 * time.Minute // default + + if cacheConfig, ok := clientConf.CMAB.Cache["size"].(int); ok { + cacheSize = cacheConfig + } + + if cacheTTLStr, ok := clientConf.CMAB.Cache["ttl"].(string); ok { + if parsedTTL, err := time.ParseDuration(cacheTTLStr); err == nil { + cacheTTL = parsedTTL + } else { + log.Warn().Err(err).Msgf("Failed to parse CMAB cache TTL: %s, using default", cacheTTLStr) + } + } + + // Parse retry configuration + retryConfig := &cmab.RetryConfig{ + MaxRetries: 3, + InitialBackoff: 100 * time.Millisecond, + MaxBackoff: 10 * time.Second, + BackoffMultiplier: 2.0, + } + + if maxRetries, ok := clientConf.CMAB.RetryConfig["maxRetries"].(int); ok { + retryConfig.MaxRetries = maxRetries + } + + if initialBackoffStr, ok := clientConf.CMAB.RetryConfig["initialBackoff"].(string); ok { + if parsedBackoff, err := time.ParseDuration(initialBackoffStr); err == nil { + retryConfig.InitialBackoff = parsedBackoff + } + } + + if maxBackoffStr, ok := clientConf.CMAB.RetryConfig["maxBackoff"].(string); ok { + if parsedBackoff, err := time.ParseDuration(maxBackoffStr); err == nil { + retryConfig.MaxBackoff = parsedBackoff + } + } + + if multiplier, ok := clientConf.CMAB.RetryConfig["backoffMultiplier"].(float64); ok { + retryConfig.BackoffMultiplier = multiplier + } + + // Create CMAB client and service + cmabClient := cmab.NewDefaultCmabClient(cmab.ClientOptions{ + HTTPClient: &http.Client{ + Timeout: clientConf.CMAB.RequestTimeout, + }, + RetryConfig: retryConfig, + Logger: logging.GetLogger(sdkKey, "CmabClient"), + }) + + cmabService := cmab.NewDefaultCmabService(cmab.ServiceOptions{ + Logger: logging.GetLogger(sdkKey, "CmabService"), + CmabCache: odpCachePkg.NewLRUCache(cacheSize, cacheTTL), + CmabClient: cmabClient, + }) + + clientOptions = append(clientOptions, client.WithCmabService(cmabService)) + optimizelyClient, err := optimizelyFactory.Client( clientOptions..., ) diff --git a/pkg/optimizely/cache_test.go b/pkg/optimizely/cache_test.go index ec2b8ddb..62a44b5b 100644 --- a/pkg/optimizely/cache_test.go +++ b/pkg/optimizely/cache_test.go @@ -34,10 +34,10 @@ import ( odpCacheServices "github.com/optimizely/agent/plugins/odpcache/services" "github.com/optimizely/agent/plugins/userprofileservice" "github.com/optimizely/agent/plugins/userprofileservice/services" + "github.com/optimizely/go-sdk/v2/pkg/cache" sdkconfig "github.com/optimizely/go-sdk/v2/pkg/config" "github.com/optimizely/go-sdk/v2/pkg/decision" "github.com/optimizely/go-sdk/v2/pkg/event" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" ) var counter int @@ -795,6 +795,286 @@ func (s *DefaultLoaderTestSuite) TestDefaultRegexValidator() { } } +// Add these tests to your existing cache_test.go file + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationParsing() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": 500, + "ttl": "15m", + }, + RetryConfig: map[string]interface{}{ + "maxRetries": 5, + "initialBackoff": "200ms", + "maxBackoff": "30s", + "backoffMultiplier": 1.5, + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) + // Note: We can't directly test the CMAB service since it's internal to the OptimizelyClient + // But we can verify the loader doesn't error with valid CMAB config +} + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationDefaults() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + // Empty cache and retry config should use defaults + Cache: map[string]interface{}{}, + RetryConfig: map[string]interface{}{}, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABCacheConfigInvalidTTL() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": 1000, + "ttl": "invalid-duration", // This should trigger warning and use default + }, + RetryConfig: map[string]interface{}{}, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) // Should not error, just use defaults + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABCacheConfigWrongTypes() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": "not-an-int", // Wrong type, should use default + "ttl": 123, // Wrong type, should use default + }, + RetryConfig: map[string]interface{}{ + "maxRetries": "not-an-int", // Wrong type, should use default + "backoffMultiplier": "not-a-float", // Wrong type, should use default + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) // Should not error, just use defaults + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABRetryConfigInvalidDurations() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{}, + RetryConfig: map[string]interface{}{ + "maxRetries": 3, + "initialBackoff": "invalid-duration", + "maxBackoff": "also-invalid", + "backoffMultiplier": 2.0, + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) // Should not error, just use defaults for invalid durations + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationAllValidValues() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 10 * time.Second, + Cache: map[string]interface{}{ + "size": 2000, + "ttl": "45m", + }, + RetryConfig: map[string]interface{}{ + "maxRetries": 10, + "initialBackoff": "500ms", + "maxBackoff": "1m", + "backoffMultiplier": 3.0, + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABWithZeroRequestTimeout() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 0, // Zero timeout + Cache: map[string]interface{}{}, + RetryConfig: map[string]interface{}{}, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationEdgeCases() { + testCases := []struct { + name string + config config.CMABConfig + }{ + { + name: "Zero cache size", + config: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": 0, + "ttl": "30m", + }, + RetryConfig: map[string]interface{}{}, + }, + }, + { + name: "Zero max retries", + config: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{}, + RetryConfig: map[string]interface{}{ + "maxRetries": 0, + }, + }, + }, + { + name: "Very short TTL", + config: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "ttl": "1ms", + }, + RetryConfig: map[string]interface{}{}, + }, + }, + { + name: "Very long TTL", + config: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "ttl": "24h", + }, + RetryConfig: map[string]interface{}{}, + }, + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: tc.config, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err, "Should not error for case: %s", tc.name) + s.NotNil(client, "Client should not be nil for case: %s", tc.name) + }) + } +} + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationNilMaps() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: nil, // nil map + RetryConfig: nil, // nil map + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) +} + +// Test that CMAB configuration doesn't interfere with existing functionality +func (s *DefaultLoaderTestSuite) TestCMABWithExistingServices() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + UserProfileService: map[string]interface{}{ + "default": "in-memory", + "services": map[string]interface{}{ + "in-memory": map[string]interface{}{ + "capacity": 100, + "storageStrategy": "fifo", + }, + }, + }, + ODP: config.OdpConfig{ + SegmentsCache: map[string]interface{}{ + "default": "in-memory", + "services": map[string]interface{}{ + "in-memory": map[string]interface{}{ + "size": 50, + "timeout": "10s", + }, + }, + }, + }, + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": 1000, + "ttl": "30m", + }, + RetryConfig: map[string]interface{}{ + "maxRetries": 5, + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) + s.NotNil(client.UserProfileService, "UPS should still be configured") + s.NotNil(client.odpCache, "ODP Cache should still be configured") +} + func TestDefaultLoaderTestSuite(t *testing.T) { suite.Run(t, new(DefaultLoaderTestSuite)) } diff --git a/pkg/optimizely/client.go b/pkg/optimizely/client.go index 374171c1..850f413e 100644 --- a/pkg/optimizely/client.go +++ b/pkg/optimizely/client.go @@ -27,7 +27,7 @@ import ( optimizelyclient "github.com/optimizely/go-sdk/v2/pkg/client" "github.com/optimizely/go-sdk/v2/pkg/decision" "github.com/optimizely/go-sdk/v2/pkg/entities" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" ) // ErrEntityNotFound is returned when no entity exists with a given key diff --git a/pkg/optimizely/optimizelytest/config.go b/pkg/optimizely/optimizelytest/config.go index f8bde648..e5cb0a57 100644 --- a/pkg/optimizely/optimizelytest/config.go +++ b/pkg/optimizely/optimizelytest/config.go @@ -518,6 +518,26 @@ func (c *TestProjectConfig) GetFlagVariationsMap() map[string][]entities.Variati return c.flagVariationsMap } +// GetAttributeKeyByID returns the attribute key for the given ID +func (c *TestProjectConfig) GetAttributeKeyByID(id string) (string, error) { + for _, attr := range c.AttributeMap { + if attr.ID == id { + return attr.Key, nil + } + } + return "", fmt.Errorf(`attribute with ID "%s" not found`, id) +} + +// GetExperimentByID returns the experiment with the given ID +func (c *TestProjectConfig) GetExperimentByID(experimentID string) (entities.Experiment, error) { + for _, experiment := range c.ExperimentMap { + if experiment.ID == experimentID { + return experiment, nil + } + } + return entities.Experiment{}, fmt.Errorf(`experiment with ID "%s" not found`, experimentID) +} + // NewConfig initializes a new datafile from a json byte array using the default JSON datafile parser func NewConfig() *TestProjectConfig { config := &TestProjectConfig{ diff --git a/plugins/odpcache/registry.go b/plugins/odpcache/registry.go index 20573067..326bd52d 100644 --- a/plugins/odpcache/registry.go +++ b/plugins/odpcache/registry.go @@ -20,7 +20,7 @@ package odpcache import ( "fmt" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" ) // Creator type defines a function for creating an instance of a Cache diff --git a/plugins/odpcache/registry_test.go b/plugins/odpcache/registry_test.go index 08e8b315..4b58727d 100644 --- a/plugins/odpcache/registry_test.go +++ b/plugins/odpcache/registry_test.go @@ -20,7 +20,7 @@ package odpcache import ( "testing" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" "github.com/stretchr/testify/assert" ) diff --git a/plugins/odpcache/services/in_memory_cache.go b/plugins/odpcache/services/in_memory_cache.go index dad28fda..1e1f4123 100644 --- a/plugins/odpcache/services/in_memory_cache.go +++ b/plugins/odpcache/services/in_memory_cache.go @@ -20,7 +20,7 @@ package services import ( "github.com/optimizely/agent/plugins/odpcache" "github.com/optimizely/agent/plugins/utils" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" ) // InMemoryCache represents the in-memory implementation of Cache interface diff --git a/plugins/odpcache/services/redis_cache.go b/plugins/odpcache/services/redis_cache.go index 73612670..c52f847c 100644 --- a/plugins/odpcache/services/redis_cache.go +++ b/plugins/odpcache/services/redis_cache.go @@ -24,7 +24,7 @@ import ( "github.com/go-redis/redis/v8" "github.com/optimizely/agent/plugins/odpcache" "github.com/optimizely/agent/plugins/utils" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" "github.com/rs/zerolog/log" ) From f20c537f21c6c429eb53bee9f0757ae6d80463df Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 20 Jun 2025 11:46:11 -0700 Subject: [PATCH 05/25] add cmab config --- README.md | 53 ++++++++- cmd/optimizely/main.go | 36 +++++- cmd/optimizely/main_test.go | 231 +++++++++++++++++++++++++++++++++++- config.yaml | 34 ++++++ config/config.go | 43 ++++++- config/config_test.go | 52 +++++++- 6 files changed, 442 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 5961a26b..187a468c 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,10 @@ Below is a comprehensive list of available configuration properties. | log.level | OPTIMIZELY_LOG_LEVEL | The log [level](https://github.com/rs/zerolog#leveled-logging) for the agent. Default: info | | log.pretty | OPTIMIZELY_LOG_PRETTY | Flag used to set colorized console output as opposed to structured json logs. Default: false | | name | OPTIMIZELY_NAME | Agent name. Default: optimizely | -| sdkKeys | OPTIMIZELY_SDKKEYS | Comma delimited list of SDK keys used to initialize on startup | +| sdkKeys | OPTIMIZELY_SDKKEYS | Comma delimited list of SDK keys used to initialize on startup | +| cmab | OPTIMIZELY_CMAB | Complete JSON configuration for CMAB. Format: see example below | +| cmab.cache | OPTIMIZELY_CMAB_CACHE | JSON configuration for just the CMAB cache section. Format: see example below | +| cmab.retryConfig | OPTIMIZELY_CMAB_RETRYCONFIG | JSON configuration for just the CMAB retry settings. Format: see example below | | 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. | | server.batchRequests.maxConcurrency | OPTIMIZELY_SERVER_BATCHREQUESTS_MAXCONCURRENCY | Number of requests running in parallel. Default: 10 | | server.batchRequests.operationsLimit | OPTIMIZELY_SERVER_BATCHREQUESTS_OPERATIONSLIMIT | Number of allowed operations. ( will flag an error if the number of operations exeeds this parameter) Default: 500 | @@ -142,6 +145,54 @@ Below is a comprehensive list of available configuration properties. | webhook.projects.<_projectId_>.secret | N/A | Webhook secret used to validate webhook requests originating from the respective projectId | | webhook.projects.<_projectId_>.skipSignatureCheck | N/A | Boolean to indicate whether the signature should be validated. TODO remove in favor of empty secret. | +### CMAB Configuration Examples + +**Complete CMAB Configuration (OPTIMIZELY_CMAB)**: +```json +{ + "enabled": true, + "predictionEndpoint": "https://custom-endpoint.com", + "requestTimeout": "5s", + "cache": { + "type": "redis", + "size": 2000, + "ttl": "45m", + "redis": { + "host": "redis:6379", + "password": "", + "database": 0 + } + }, + "retryConfig": { + "maxRetries": 3, + "initialBackoff": "100ms", + "maxBackoff": "10s", + "backoffMultiplier": 2.0 + } +} + +CMAB Cache Configuration (OPTIMIZELY_CMAB_CACHE): + +{ + "type": "redis", + "size": 2000, + "ttl": "45m", + "redis": { + "host": "redis:6379", + "password": "", + "database": 0 + } +} + +CMAB Retry Configuration (OPTIMIZELY_CMAB_RETRYCONFIG): + +{ + "maxRetries": 3, + "initialBackoff": "100ms", + "maxBackoff": "10s", + "backoffMultiplier": 2.0 +} + More information about configuring Agent can be found in the [Advanced Configuration Notes](https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/advanced-configuration). ### API diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index 5784a5d1..b7ad6976 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -25,6 +25,7 @@ import ( "runtime" "strings" "syscall" + "time" "github.com/rs/zerolog" "github.com/rs/zerolog/log" @@ -98,15 +99,46 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { } // Check if JSON string was set using OPTIMIZELY_CLIENT_USERPROFILESERVICE environment variable - if userProfileService := v.GetStringMap("client.userprofileservice"); userProfileService != nil { + if userProfileService := v.GetStringMap("client.userprofileservice"); len(userProfileService) > 0 { conf.Client.UserProfileService = userProfileService } // Check if JSON string was set using OPTIMIZELY_CLIENT_ODP_SEGMENTSCACHE environment variable - if odpSegmentsCache := v.GetStringMap("client.odp.segmentsCache"); odpSegmentsCache != nil { + if odpSegmentsCache := v.GetStringMap("client.odp.segmentsCache"); len(odpSegmentsCache) > 0 { conf.Client.ODP.SegmentsCache = odpSegmentsCache } + // Handle CMAB configuration using the same approach as UserProfileService + // Check for complete CMAB configuration first + if cmab := v.GetStringMap("cmab"); len(cmab) > 0 { + if enabled, ok := cmab["enabled"].(bool); ok { + conf.CMAB.Enabled = enabled + } + if endpoint, ok := cmab["predictionEndpoint"].(string); ok { + conf.CMAB.PredictionEndpoint = endpoint + } + if timeout, ok := cmab["requestTimeout"].(string); ok { + if duration, err := time.ParseDuration(timeout); err == nil { + conf.CMAB.RequestTimeout = duration + } + } + if cache, ok := cmab["cache"].(map[string]interface{}); ok { + conf.CMAB.Cache = cache + } + if retryConfig, ok := cmab["retryConfig"].(map[string]interface{}); ok { + conf.CMAB.RetryConfig = retryConfig + } + } + + // Check for individual map sections + if cmabCache := v.GetStringMap("cmab.cache"); len(cmabCache) > 0 { + conf.CMAB.Cache = cmabCache + } + + if cmabRetryConfig := v.GetStringMap("cmab.retryConfig"); len(cmabRetryConfig) > 0 { + conf.CMAB.RetryConfig = cmabRetryConfig + } + return conf } diff --git a/cmd/optimizely/main_test.go b/cmd/optimizely/main_test.go index 72ae36fa..42ea8d3b 100644 --- a/cmd/optimizely/main_test.go +++ b/cmd/optimizely/main_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020,2022-2023, Optimizely, Inc. and contributors * + * Copyright 2019-2020,2022-2025, 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. * @@ -17,7 +17,9 @@ package main import ( + "fmt" "os" + "strings" "testing" "time" @@ -178,6 +180,168 @@ func assertWebhook(t *testing.T, actual config.WebhookConfig) { assert.False(t, actual.Projects[20000].SkipSignatureCheck) } +func assertCMAB(t *testing.T, cmab config.CMABConfig) { + fmt.Println("In assertCMAB, received CMAB config:") + fmt.Printf(" Enabled: %v\n", cmab.Enabled) + fmt.Printf(" PredictionEndpoint: %s\n", cmab.PredictionEndpoint) + fmt.Printf(" RequestTimeout: %v\n", cmab.RequestTimeout) + fmt.Printf(" Cache: %#v\n", cmab.Cache) + fmt.Printf(" RetryConfig: %#v\n", cmab.RetryConfig) + + // Base assertions + assert.True(t, cmab.Enabled) + assert.Equal(t, "https://custom-cmab-endpoint.example.com", cmab.PredictionEndpoint) + assert.Equal(t, 15*time.Second, cmab.RequestTimeout) + + // Check if cache map is initialized + cacheMap := cmab.Cache + if cacheMap == nil { + t.Fatal("Cache map is nil") + } + + // Debug cache type + cacheTypeValue := cacheMap["type"] + fmt.Printf("Cache type: %v (%T)\n", cacheTypeValue, cacheTypeValue) + assert.Equal(t, "redis", cacheTypeValue) + + // Debug cache size + cacheSizeValue := cacheMap["size"] + fmt.Printf("Cache size: %v (%T)\n", cacheSizeValue, cacheSizeValue) + sizeValue, ok := cacheSizeValue.(float64) + assert.True(t, ok, "Cache size should be float64") + assert.Equal(t, float64(2000), sizeValue) + + // Cache TTL + cacheTTLValue := cacheMap["ttl"] + fmt.Printf("Cache TTL: %v (%T)\n", cacheTTLValue, cacheTTLValue) + assert.Equal(t, "45m", cacheTTLValue) + + // Redis settings + redisValue := cacheMap["redis"] + fmt.Printf("Redis: %v (%T)\n", redisValue, redisValue) + redisMap, ok := redisValue.(map[string]interface{}) + assert.True(t, ok, "Redis should be a map") + + if !ok { + t.Fatal("Redis is not a map") + } + + redisHost := redisMap["host"] + fmt.Printf("Redis host: %v (%T)\n", redisHost, redisHost) + assert.Equal(t, "redis.example.com:6379", redisHost) + + redisPassword := redisMap["password"] + fmt.Printf("Redis password: %v (%T)\n", redisPassword, redisPassword) + assert.Equal(t, "password123", redisPassword) + + redisDBValue := redisMap["database"] + fmt.Printf("Redis DB: %v (%T)\n", redisDBValue, redisDBValue) + dbValue, ok := redisDBValue.(float64) + assert.True(t, ok, "Redis DB should be float64") + assert.Equal(t, float64(2), dbValue) + + // Retry settings + retryMap := cmab.RetryConfig + if retryMap == nil { + t.Fatal("RetryConfig map is nil") + } + + // Max retries + maxRetriesValue := retryMap["maxRetries"] + fmt.Printf("maxRetries: %v (%T)\n", maxRetriesValue, maxRetriesValue) + maxRetries, ok := maxRetriesValue.(float64) + assert.True(t, ok, "maxRetries should be float64") + assert.Equal(t, float64(5), maxRetries) + + // Check other retry settings + fmt.Printf("initialBackoff: %v (%T)\n", retryMap["initialBackoff"], retryMap["initialBackoff"]) + assert.Equal(t, "200ms", retryMap["initialBackoff"]) + + fmt.Printf("maxBackoff: %v (%T)\n", retryMap["maxBackoff"], retryMap["maxBackoff"]) + assert.Equal(t, "30s", retryMap["maxBackoff"]) + + fmt.Printf("backoffMultiplier: %v (%T)\n", retryMap["backoffMultiplier"], retryMap["backoffMultiplier"]) + assert.Equal(t, 3.0, retryMap["backoffMultiplier"]) +} + +func TestCMABEnvDebug(t *testing.T) { + _ = os.Setenv("OPTIMIZELY_CMAB", `{ + "enabled": true, + "predictionEndpoint": "https://custom-cmab-endpoint.example.com", + "requestTimeout": "15s", + "cache": { + "type": "redis", + "size": 2000, + "ttl": "45m", + "redis": { + "host": "redis.example.com:6379", + "password": "password123", + "database": 2 + } + }, + "retryConfig": { + "maxRetries": 5, + "initialBackoff": "200ms", + "maxBackoff": "30s", + "backoffMultiplier": 3.0 + } + }`) + + // Load config using Viper + v := viper.New() + v.SetEnvPrefix("optimizely") + v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) + v.AutomaticEnv() + + // Create config + assert.NoError(t, initConfig(v)) + conf := loadConfig(v) + + // Debug: Print the parsed config + fmt.Println("Parsed CMAB config from JSON env var:") + fmt.Printf(" Enabled: %v\n", conf.CMAB.Enabled) + fmt.Printf(" PredictionEndpoint: %s\n", conf.CMAB.PredictionEndpoint) + fmt.Printf(" RequestTimeout: %v\n", conf.CMAB.RequestTimeout) + fmt.Printf(" Cache: %+v\n", conf.CMAB.Cache) + fmt.Printf(" RetryConfig: %+v\n", conf.CMAB.RetryConfig) + + // Call assertCMAB + assertCMAB(t, conf.CMAB) +} + +func TestCMABPartialConfig(t *testing.T) { + // Clean any existing environment variables + os.Unsetenv("OPTIMIZELY_CMAB") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE") + os.Unsetenv("OPTIMIZELY_CMAB_RETRYCONFIG") + + // Set partial configuration through CMAB_CACHE and CMAB_RETRYCONFIG + _ = os.Setenv("OPTIMIZELY_CMAB", `{"enabled": true, "predictionEndpoint": "https://base-endpoint.example.com"}`) + _ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type": "redis", "size": 3000}`) + _ = os.Setenv("OPTIMIZELY_CMAB_RETRYCONFIG", `{"maxRetries": 10}`) + + // Load config + v := viper.New() + assert.NoError(t, initConfig(v)) + conf := loadConfig(v) + + // Base assertions + assert.True(t, conf.CMAB.Enabled) + assert.Equal(t, "https://base-endpoint.example.com", conf.CMAB.PredictionEndpoint) + + // Cache assertions + assert.Equal(t, "redis", conf.CMAB.Cache["type"]) + assert.Equal(t, float64(3000), conf.CMAB.Cache["size"]) + + // RetryConfig assertions + assert.Equal(t, float64(10), conf.CMAB.RetryConfig["maxRetries"]) + + // Clean up + os.Unsetenv("OPTIMIZELY_CMAB") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE") + os.Unsetenv("OPTIMIZELY_CMAB_RETRYCONFIG") +} + func TestViperYaml(t *testing.T) { v := viper.New() v.Set("config.filename", "./testdata/default.yaml") @@ -392,6 +556,28 @@ func TestViperEnv(t *testing.T) { _ = os.Setenv("OPTIMIZELY_WEBHOOK_PROJECTS_20000_SDKKEYS", "xxx,yyy,zzz") _ = os.Setenv("OPTIMIZELY_WEBHOOK_PROJECTS_20000_SKIPSIGNATURECHECK", "false") + _ = os.Setenv("OPTIMIZELY_CMAB", `{ + "enabled": true, + "predictionEndpoint": "https://custom-cmab-endpoint.example.com", + "requestTimeout": "15s", + "cache": { + "type": "redis", + "size": 2000, + "ttl": "45m", + "redis": { + "host": "redis.example.com:6379", + "password": "password123", + "database": 2 + } + }, + "retryConfig": { + "maxRetries": 5, + "initialBackoff": "200ms", + "maxBackoff": "30s", + "backoffMultiplier": 3.0 + } + }`) + _ = os.Setenv("OPTIMIZELY_RUNTIME_BLOCKPROFILERATE", "1") _ = os.Setenv("OPTIMIZELY_RUNTIME_MUTEXPROFILEFRACTION", "2") @@ -407,6 +593,7 @@ func TestViperEnv(t *testing.T) { assertAPI(t, actual.API) //assertWebhook(t, actual.Webhook) // Maps don't appear to be supported assertRuntime(t, actual.Runtime) + assertCMAB(t, actual.CMAB) } func TestLoggingWithIncludeSdkKey(t *testing.T) { @@ -507,3 +694,45 @@ func Test_initTracing(t *testing.T) { }) } } + +func TestCMABComplexJSON(t *testing.T) { + // Clean any existing environment variables for CMAB + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_TYPE") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_SIZE") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_TTL") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_HOST") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_PASSWORD") + os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_DATABASE") + + // Set complex JSON environment variable for CMAB cache + _ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type":"redis","size":5000,"ttl":"3h","redis":{"host":"json-redis.example.com:6379","password":"json-password","database":4}}`) + + defer func() { + // Clean up + os.Unsetenv("OPTIMIZELY_CMAB_CACHE") + }() + + v := viper.New() + assert.NoError(t, initConfig(v)) + actual := loadConfig(v) + + // Test cache settings from JSON environment variable + cacheMap := actual.CMAB.Cache + assert.Equal(t, "redis", cacheMap["type"]) + + // Account for JSON unmarshaling to float64 + size, ok := cacheMap["size"].(float64) + assert.True(t, ok, "Size should be a float64") + assert.Equal(t, float64(5000), size) + + assert.Equal(t, "3h", cacheMap["ttl"]) + + redisMap, ok := cacheMap["redis"].(map[string]interface{}) + assert.True(t, ok, "Redis config should be a map") + assert.Equal(t, "json-redis.example.com:6379", redisMap["host"]) + assert.Equal(t, "json-password", redisMap["password"]) + + db, ok := redisMap["database"].(float64) + assert.True(t, ok, "Database should be a float64") + assert.Equal(t, float64(4), db) +} diff --git a/config.yaml b/config.yaml index d3145d3b..082075ff 100644 --- a/config.yaml +++ b/config.yaml @@ -262,3 +262,37 @@ synchronization: datafile: enable: false default: "redis" + +## +## cmab: Contextual Multi-Armed Bandit configuration +## +cmab: + ## enable or disable CMAB functionality + enabled: false + ## URL for CMAB prediction service + predictionEndpoint: "https://prediction.cmab.optimizely.com" + ## timeout for CMAB API requests + requestTimeout: 10s + ## CMAB cache configuration + cache: + ## cache type (memory or redis) + type: "memory" + ## maximum number of entries for in-memory cache + size: 1000 + ## time-to-live for cached decisions + ttl: 30m + ## Redis configuration (if using redis cache) + redis: + host: "localhost:6379" + password: "" + database: 0 + ## retry configuration for CMAB API requests + retryConfig: + ## maximum number of retry attempts + maxRetries: 3 + ## initial backoff duration + initialBackoff: 100ms + ## maximum backoff duration + maxBackoff: 10s + ## multiplier for exponential backoff + backoffMultiplier: 2.0 diff --git a/config/config.go b/config/config.go index 9e652910..f3291d3e 100644 --- a/config/config.go +++ b/config/config.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020,2022-2023, Optimizely, Inc. and contributors * + * Copyright 2019-2020,2022-2025, 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. * @@ -140,8 +140,28 @@ func NewDefaultConfig() *AgentConfig { Default: "redis", }, }, + CMAB: CMABConfig{ + Enabled: false, + PredictionEndpoint: "https://prediction.cmab.optimizely.com", + RequestTimeout: 10 * time.Second, + Cache: map[string]interface{}{ + "type": "memory", + "size": 1000, + "ttl": "30m", + "redis": map[string]interface{}{ + "host": "localhost:6379", + "password": "", + "database": 0, + }, + }, + RetryConfig: map[string]interface{}{ + "maxRetries": 3, + "initialBackoff": "100ms", + "maxBackoff": "10s", + "backoffMultiplier": 2.0, + }, + }, } - return &config } @@ -162,6 +182,7 @@ type AgentConfig struct { Server ServerConfig `json:"server"` Webhook WebhookConfig `json:"webhook"` Synchronization SyncConfig `json:"synchronization"` + CMAB CMABConfig `json:"cmab"` } // SyncConfig contains Synchronization configuration for the multiple Agent nodes @@ -387,3 +408,21 @@ type RuntimeConfig struct { // (For n>1 the details of sampling may change.) MutexProfileFraction int `json:"mutexProfileFraction"` } + +// CMABConfig holds configuration for the Contextual Multi-Armed Bandit functionality +type CMABConfig struct { + // Enabled indicates whether the CMAB functionality is enabled + Enabled bool `json:"enabled"` + + // PredictionEndpoint is the URL for CMAB predictions + PredictionEndpoint string `json:"predictionEndpoint"` + + // RequestTimeout is the timeout for CMAB API requests + RequestTimeout time.Duration `json:"requestTimeout"` + + // Cache configuration + Cache map[string]interface{} `json:"cache"` + + // RetryConfig for CMAB API requests + RetryConfig map[string]interface{} `json:"retryConfig"` +} diff --git a/config/config_test.go b/config/config_test.go index 917cd498..a5f3a8a4 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020,2022-2023, Optimizely, Inc. and contributors * + * Copyright 2019-2020,2022-2025, 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. * @@ -99,6 +99,29 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(t, 0, conf.Runtime.BlockProfileRate) assert.Equal(t, 0, conf.Runtime.MutexProfileFraction) + + // CMAB configuration + assert.False(t, conf.CMAB.Enabled) + assert.Equal(t, "https://prediction.cmab.optimizely.com", conf.CMAB.PredictionEndpoint) + assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) + + // Test cache settings as maps + cacheMap := conf.CMAB.Cache + assert.Equal(t, "memory", cacheMap["type"]) + assert.Equal(t, 1000, cacheMap["size"]) + assert.Equal(t, "30m", cacheMap["ttl"]) + + redisMap := cacheMap["redis"].(map[string]interface{}) + assert.Equal(t, "localhost:6379", redisMap["host"]) + assert.Equal(t, "", redisMap["password"]) + assert.Equal(t, 0, redisMap["database"]) + + // Test retry settings as maps + retryMap := conf.CMAB.RetryConfig + assert.Equal(t, 3, retryMap["maxRetries"]) + assert.Equal(t, "100ms", retryMap["initialBackoff"]) + assert.Equal(t, "10s", retryMap["maxBackoff"]) + assert.Equal(t, 2.0, retryMap["backoffMultiplier"]) } type logObservation struct { @@ -233,3 +256,30 @@ func TestServerConfig_GetAllowedHosts(t *testing.T) { assert.Contains(t, allowedHosts, "localhost") assert.Contains(t, allowedHosts, "special.test.host") } + +func TestDefaultCMABConfig(t *testing.T) { + conf := NewDefaultConfig() + + // Test default values + assert.False(t, conf.CMAB.Enabled) + assert.Equal(t, "https://prediction.cmab.optimizely.com", conf.CMAB.PredictionEndpoint) + assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) + + // Test default cache settings as maps + cacheMap := conf.CMAB.Cache + assert.Equal(t, "memory", cacheMap["type"]) + assert.Equal(t, 1000, cacheMap["size"]) + assert.Equal(t, "30m", cacheMap["ttl"]) + + redisMap := cacheMap["redis"].(map[string]interface{}) + assert.Equal(t, "localhost:6379", redisMap["host"]) + assert.Equal(t, "", redisMap["password"]) + assert.Equal(t, 0, redisMap["database"]) + + // Test default retry settings as maps + retryMap := conf.CMAB.RetryConfig + assert.Equal(t, 3, retryMap["maxRetries"]) + assert.Equal(t, "100ms", retryMap["initialBackoff"]) + assert.Equal(t, "10s", retryMap["maxBackoff"]) + assert.Equal(t, 2.0, retryMap["backoffMultiplier"]) +} From 04815d934c05ba0a4bb10f9123b0654834e4b684 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 20 Jun 2025 15:28:07 -0700 Subject: [PATCH 06/25] remove enabled config, remove 3 ENV vars for cmab, only use a single one --- README.md | 39 +++++-------------------------------- cmd/optimizely/main.go | 3 --- cmd/optimizely/main_test.go | 4 ---- config.yaml | 9 +-------- config/config.go | 11 +---------- config/config_test.go | 16 ++------------- 6 files changed, 9 insertions(+), 73 deletions(-) diff --git a/README.md b/README.md index 187a468c..2c23aa84 100644 --- a/README.md +++ b/README.md @@ -145,23 +145,16 @@ Below is a comprehensive list of available configuration properties. | webhook.projects.<_projectId_>.secret | N/A | Webhook secret used to validate webhook requests originating from the respective projectId | | webhook.projects.<_projectId_>.skipSignatureCheck | N/A | Boolean to indicate whether the signature should be validated. TODO remove in favor of empty secret. | -### CMAB Configuration Examples +### CMAB Configuration Example -**Complete CMAB Configuration (OPTIMIZELY_CMAB)**: ```json { - "enabled": true, - "predictionEndpoint": "https://custom-endpoint.com", + "predictionEndpoint": "https://prediction.cmab.optimizely.com/predict/%s", "requestTimeout": "5s", "cache": { - "type": "redis", + "type": "memory", "size": 2000, - "ttl": "45m", - "redis": { - "host": "redis:6379", - "password": "", - "database": 0 - } + "ttl": "45m" }, "retryConfig": { "maxRetries": 3, @@ -169,29 +162,7 @@ Below is a comprehensive list of available configuration properties. "maxBackoff": "10s", "backoffMultiplier": 2.0 } -} - -CMAB Cache Configuration (OPTIMIZELY_CMAB_CACHE): - -{ - "type": "redis", - "size": 2000, - "ttl": "45m", - "redis": { - "host": "redis:6379", - "password": "", - "database": 0 - } -} - -CMAB Retry Configuration (OPTIMIZELY_CMAB_RETRYCONFIG): - -{ - "maxRetries": 3, - "initialBackoff": "100ms", - "maxBackoff": "10s", - "backoffMultiplier": 2.0 -} +}``` More information about configuring Agent can be found in the [Advanced Configuration Notes](https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/advanced-configuration). diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index b7ad6976..1f00d09e 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -111,9 +111,6 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { // Handle CMAB configuration using the same approach as UserProfileService // Check for complete CMAB configuration first if cmab := v.GetStringMap("cmab"); len(cmab) > 0 { - if enabled, ok := cmab["enabled"].(bool); ok { - conf.CMAB.Enabled = enabled - } if endpoint, ok := cmab["predictionEndpoint"].(string); ok { conf.CMAB.PredictionEndpoint = endpoint } diff --git a/cmd/optimizely/main_test.go b/cmd/optimizely/main_test.go index 42ea8d3b..88c837d5 100644 --- a/cmd/optimizely/main_test.go +++ b/cmd/optimizely/main_test.go @@ -182,14 +182,12 @@ func assertWebhook(t *testing.T, actual config.WebhookConfig) { func assertCMAB(t *testing.T, cmab config.CMABConfig) { fmt.Println("In assertCMAB, received CMAB config:") - fmt.Printf(" Enabled: %v\n", cmab.Enabled) fmt.Printf(" PredictionEndpoint: %s\n", cmab.PredictionEndpoint) fmt.Printf(" RequestTimeout: %v\n", cmab.RequestTimeout) fmt.Printf(" Cache: %#v\n", cmab.Cache) fmt.Printf(" RetryConfig: %#v\n", cmab.RetryConfig) // Base assertions - assert.True(t, cmab.Enabled) assert.Equal(t, "https://custom-cmab-endpoint.example.com", cmab.PredictionEndpoint) assert.Equal(t, 15*time.Second, cmab.RequestTimeout) @@ -299,7 +297,6 @@ func TestCMABEnvDebug(t *testing.T) { // Debug: Print the parsed config fmt.Println("Parsed CMAB config from JSON env var:") - fmt.Printf(" Enabled: %v\n", conf.CMAB.Enabled) fmt.Printf(" PredictionEndpoint: %s\n", conf.CMAB.PredictionEndpoint) fmt.Printf(" RequestTimeout: %v\n", conf.CMAB.RequestTimeout) fmt.Printf(" Cache: %+v\n", conf.CMAB.Cache) @@ -326,7 +323,6 @@ func TestCMABPartialConfig(t *testing.T) { conf := loadConfig(v) // Base assertions - assert.True(t, conf.CMAB.Enabled) assert.Equal(t, "https://base-endpoint.example.com", conf.CMAB.PredictionEndpoint) // Cache assertions diff --git a/config.yaml b/config.yaml index 082075ff..76de2b40 100644 --- a/config.yaml +++ b/config.yaml @@ -267,10 +267,8 @@ synchronization: ## cmab: Contextual Multi-Armed Bandit configuration ## cmab: - ## enable or disable CMAB functionality - enabled: false ## URL for CMAB prediction service - predictionEndpoint: "https://prediction.cmab.optimizely.com" + predictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s" ## timeout for CMAB API requests requestTimeout: 10s ## CMAB cache configuration @@ -281,11 +279,6 @@ cmab: size: 1000 ## time-to-live for cached decisions ttl: 30m - ## Redis configuration (if using redis cache) - redis: - host: "localhost:6379" - password: "" - database: 0 ## retry configuration for CMAB API requests retryConfig: ## maximum number of retry attempts diff --git a/config/config.go b/config/config.go index f3291d3e..4da7e95d 100644 --- a/config/config.go +++ b/config/config.go @@ -141,18 +141,12 @@ func NewDefaultConfig() *AgentConfig { }, }, CMAB: CMABConfig{ - Enabled: false, - PredictionEndpoint: "https://prediction.cmab.optimizely.com", + PredictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s", RequestTimeout: 10 * time.Second, Cache: map[string]interface{}{ "type": "memory", "size": 1000, "ttl": "30m", - "redis": map[string]interface{}{ - "host": "localhost:6379", - "password": "", - "database": 0, - }, }, RetryConfig: map[string]interface{}{ "maxRetries": 3, @@ -411,9 +405,6 @@ type RuntimeConfig struct { // CMABConfig holds configuration for the Contextual Multi-Armed Bandit functionality type CMABConfig struct { - // Enabled indicates whether the CMAB functionality is enabled - Enabled bool `json:"enabled"` - // PredictionEndpoint is the URL for CMAB predictions PredictionEndpoint string `json:"predictionEndpoint"` diff --git a/config/config_test.go b/config/config_test.go index a5f3a8a4..42de8f74 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -101,8 +101,7 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(t, 0, conf.Runtime.MutexProfileFraction) // CMAB configuration - assert.False(t, conf.CMAB.Enabled) - assert.Equal(t, "https://prediction.cmab.optimizely.com", conf.CMAB.PredictionEndpoint) + assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) // Test cache settings as maps @@ -111,11 +110,6 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(t, 1000, cacheMap["size"]) assert.Equal(t, "30m", cacheMap["ttl"]) - redisMap := cacheMap["redis"].(map[string]interface{}) - assert.Equal(t, "localhost:6379", redisMap["host"]) - assert.Equal(t, "", redisMap["password"]) - assert.Equal(t, 0, redisMap["database"]) - // Test retry settings as maps retryMap := conf.CMAB.RetryConfig assert.Equal(t, 3, retryMap["maxRetries"]) @@ -261,8 +255,7 @@ func TestDefaultCMABConfig(t *testing.T) { conf := NewDefaultConfig() // Test default values - assert.False(t, conf.CMAB.Enabled) - assert.Equal(t, "https://prediction.cmab.optimizely.com", conf.CMAB.PredictionEndpoint) + assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) // Test default cache settings as maps @@ -271,11 +264,6 @@ func TestDefaultCMABConfig(t *testing.T) { assert.Equal(t, 1000, cacheMap["size"]) assert.Equal(t, "30m", cacheMap["ttl"]) - redisMap := cacheMap["redis"].(map[string]interface{}) - assert.Equal(t, "localhost:6379", redisMap["host"]) - assert.Equal(t, "", redisMap["password"]) - assert.Equal(t, 0, redisMap["database"]) - // Test default retry settings as maps retryMap := conf.CMAB.RetryConfig assert.Equal(t, 3, retryMap["maxRetries"]) From a892d8a7c991060a0ebc86f7bf4e8ca371f91393 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 20 Jun 2025 15:30:12 -0700 Subject: [PATCH 07/25] cleanup readme --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2c23aa84..a5f9ded6 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,8 @@ Below is a comprehensive list of available configuration properties. "maxBackoff": "10s", "backoffMultiplier": 2.0 } -}``` +} +``` More information about configuring Agent can be found in the [Advanced Configuration Notes](https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/advanced-configuration). From f3f334d79fadb92a1f96880f98570243f95a121a Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Wed, 25 Jun 2025 14:54:18 -0700 Subject: [PATCH 08/25] add cmab logic to agent --- README.md | 1 - cmd/optimizely/main.go | 3 - cmd/optimizely/main_test.go | 11 - config.yaml | 2 - config/config.go | 7 +- config/config_test.go | 2 - pkg/optimizely/cache.go | 66 ++++- pkg/optimizely/cache_test.go | 282 ++++++++++++++++++- pkg/optimizely/client.go | 2 +- pkg/optimizely/optimizelytest/config.go | 20 ++ plugins/odpcache/registry.go | 2 +- plugins/odpcache/registry_test.go | 2 +- plugins/odpcache/services/in_memory_cache.go | 2 +- plugins/odpcache/services/redis_cache.go | 2 +- 14 files changed, 373 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index a5f9ded6..15ff2e36 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,6 @@ Below is a comprehensive list of available configuration properties. ```json { - "predictionEndpoint": "https://prediction.cmab.optimizely.com/predict/%s", "requestTimeout": "5s", "cache": { "type": "memory", diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index 1f00d09e..29dfaadf 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -111,9 +111,6 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { // Handle CMAB configuration using the same approach as UserProfileService // Check for complete CMAB configuration first if cmab := v.GetStringMap("cmab"); len(cmab) > 0 { - if endpoint, ok := cmab["predictionEndpoint"].(string); ok { - conf.CMAB.PredictionEndpoint = endpoint - } if timeout, ok := cmab["requestTimeout"].(string); ok { if duration, err := time.ParseDuration(timeout); err == nil { conf.CMAB.RequestTimeout = duration diff --git a/cmd/optimizely/main_test.go b/cmd/optimizely/main_test.go index 88c837d5..48336a6f 100644 --- a/cmd/optimizely/main_test.go +++ b/cmd/optimizely/main_test.go @@ -182,13 +182,11 @@ func assertWebhook(t *testing.T, actual config.WebhookConfig) { func assertCMAB(t *testing.T, cmab config.CMABConfig) { fmt.Println("In assertCMAB, received CMAB config:") - fmt.Printf(" PredictionEndpoint: %s\n", cmab.PredictionEndpoint) fmt.Printf(" RequestTimeout: %v\n", cmab.RequestTimeout) fmt.Printf(" Cache: %#v\n", cmab.Cache) fmt.Printf(" RetryConfig: %#v\n", cmab.RetryConfig) // Base assertions - assert.Equal(t, "https://custom-cmab-endpoint.example.com", cmab.PredictionEndpoint) assert.Equal(t, 15*time.Second, cmab.RequestTimeout) // Check if cache map is initialized @@ -264,8 +262,6 @@ func assertCMAB(t *testing.T, cmab config.CMABConfig) { func TestCMABEnvDebug(t *testing.T) { _ = os.Setenv("OPTIMIZELY_CMAB", `{ - "enabled": true, - "predictionEndpoint": "https://custom-cmab-endpoint.example.com", "requestTimeout": "15s", "cache": { "type": "redis", @@ -297,7 +293,6 @@ func TestCMABEnvDebug(t *testing.T) { // Debug: Print the parsed config fmt.Println("Parsed CMAB config from JSON env var:") - fmt.Printf(" PredictionEndpoint: %s\n", conf.CMAB.PredictionEndpoint) fmt.Printf(" RequestTimeout: %v\n", conf.CMAB.RequestTimeout) fmt.Printf(" Cache: %+v\n", conf.CMAB.Cache) fmt.Printf(" RetryConfig: %+v\n", conf.CMAB.RetryConfig) @@ -313,7 +308,6 @@ func TestCMABPartialConfig(t *testing.T) { os.Unsetenv("OPTIMIZELY_CMAB_RETRYCONFIG") // Set partial configuration through CMAB_CACHE and CMAB_RETRYCONFIG - _ = os.Setenv("OPTIMIZELY_CMAB", `{"enabled": true, "predictionEndpoint": "https://base-endpoint.example.com"}`) _ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type": "redis", "size": 3000}`) _ = os.Setenv("OPTIMIZELY_CMAB_RETRYCONFIG", `{"maxRetries": 10}`) @@ -322,9 +316,6 @@ func TestCMABPartialConfig(t *testing.T) { assert.NoError(t, initConfig(v)) conf := loadConfig(v) - // Base assertions - assert.Equal(t, "https://base-endpoint.example.com", conf.CMAB.PredictionEndpoint) - // Cache assertions assert.Equal(t, "redis", conf.CMAB.Cache["type"]) assert.Equal(t, float64(3000), conf.CMAB.Cache["size"]) @@ -553,8 +544,6 @@ func TestViperEnv(t *testing.T) { _ = os.Setenv("OPTIMIZELY_WEBHOOK_PROJECTS_20000_SKIPSIGNATURECHECK", "false") _ = os.Setenv("OPTIMIZELY_CMAB", `{ - "enabled": true, - "predictionEndpoint": "https://custom-cmab-endpoint.example.com", "requestTimeout": "15s", "cache": { "type": "redis", diff --git a/config.yaml b/config.yaml index 76de2b40..6dc98b31 100644 --- a/config.yaml +++ b/config.yaml @@ -267,8 +267,6 @@ synchronization: ## cmab: Contextual Multi-Armed Bandit configuration ## cmab: - ## URL for CMAB prediction service - predictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s" ## timeout for CMAB API requests requestTimeout: 10s ## CMAB cache configuration diff --git a/config/config.go b/config/config.go index 4da7e95d..9a90f08a 100644 --- a/config/config.go +++ b/config/config.go @@ -141,8 +141,7 @@ func NewDefaultConfig() *AgentConfig { }, }, CMAB: CMABConfig{ - PredictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s", - RequestTimeout: 10 * time.Second, + RequestTimeout: 10 * time.Second, Cache: map[string]interface{}{ "type": "memory", "size": 1000, @@ -230,6 +229,7 @@ type ClientConfig struct { SdkKeyRegex string `json:"sdkKeyRegex"` UserProfileService UserProfileServiceConfigs `json:"userProfileService"` ODP OdpConfig `json:"odp"` + CMAB CMABConfig `json:"cmab" mapstructure:"cmab"` } // OdpConfig holds the odp configuration @@ -405,9 +405,6 @@ type RuntimeConfig struct { // CMABConfig holds configuration for the Contextual Multi-Armed Bandit functionality type CMABConfig struct { - // PredictionEndpoint is the URL for CMAB predictions - PredictionEndpoint string `json:"predictionEndpoint"` - // RequestTimeout is the timeout for CMAB API requests RequestTimeout time.Duration `json:"requestTimeout"` diff --git a/config/config_test.go b/config/config_test.go index 42de8f74..1f23eed0 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -101,7 +101,6 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(t, 0, conf.Runtime.MutexProfileFraction) // CMAB configuration - assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) // Test cache settings as maps @@ -255,7 +254,6 @@ func TestDefaultCMABConfig(t *testing.T) { conf := NewDefaultConfig() // Test default values - assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) // Test default cache settings as maps diff --git a/pkg/optimizely/cache.go b/pkg/optimizely/cache.go index 093f6585..17763d54 100644 --- a/pkg/optimizely/cache.go +++ b/pkg/optimizely/cache.go @@ -21,9 +21,11 @@ import ( "context" "encoding/json" "errors" + "net/http" "regexp" "strings" "sync" + "time" cmap "github.com/orcaman/concurrent-map" "github.com/rs/zerolog/log" @@ -33,13 +35,14 @@ import ( "github.com/optimizely/agent/pkg/syncer" "github.com/optimizely/agent/plugins/odpcache" "github.com/optimizely/agent/plugins/userprofileservice" + odpCachePkg "github.com/optimizely/go-sdk/v2/pkg/cache" "github.com/optimizely/go-sdk/v2/pkg/client" + cmab "github.com/optimizely/go-sdk/v2/pkg/cmab" sdkconfig "github.com/optimizely/go-sdk/v2/pkg/config" "github.com/optimizely/go-sdk/v2/pkg/decision" "github.com/optimizely/go-sdk/v2/pkg/event" "github.com/optimizely/go-sdk/v2/pkg/logging" "github.com/optimizely/go-sdk/v2/pkg/odp" - odpCachePkg "github.com/optimizely/go-sdk/v2/pkg/odp/cache" odpEventPkg "github.com/optimizely/go-sdk/v2/pkg/odp/event" odpSegmentPkg "github.com/optimizely/go-sdk/v2/pkg/odp/segment" "github.com/optimizely/go-sdk/v2/pkg/tracing" @@ -312,6 +315,67 @@ func defaultLoader( ) clientOptions = append(clientOptions, client.WithOdpManager(odpManager)) + // Parse CMAB cache configuration + cacheSize := 1000 // default + cacheTTL := 30 * time.Minute // default + + if cacheConfig, ok := clientConf.CMAB.Cache["size"].(int); ok { + cacheSize = cacheConfig + } + + if cacheTTLStr, ok := clientConf.CMAB.Cache["ttl"].(string); ok { + if parsedTTL, err := time.ParseDuration(cacheTTLStr); err == nil { + cacheTTL = parsedTTL + } else { + log.Warn().Err(err).Msgf("Failed to parse CMAB cache TTL: %s, using default", cacheTTLStr) + } + } + + // Parse retry configuration + retryConfig := &cmab.RetryConfig{ + MaxRetries: 3, + InitialBackoff: 100 * time.Millisecond, + MaxBackoff: 10 * time.Second, + BackoffMultiplier: 2.0, + } + + if maxRetries, ok := clientConf.CMAB.RetryConfig["maxRetries"].(int); ok { + retryConfig.MaxRetries = maxRetries + } + + if initialBackoffStr, ok := clientConf.CMAB.RetryConfig["initialBackoff"].(string); ok { + if parsedBackoff, err := time.ParseDuration(initialBackoffStr); err == nil { + retryConfig.InitialBackoff = parsedBackoff + } + } + + if maxBackoffStr, ok := clientConf.CMAB.RetryConfig["maxBackoff"].(string); ok { + if parsedBackoff, err := time.ParseDuration(maxBackoffStr); err == nil { + retryConfig.MaxBackoff = parsedBackoff + } + } + + if multiplier, ok := clientConf.CMAB.RetryConfig["backoffMultiplier"].(float64); ok { + retryConfig.BackoffMultiplier = multiplier + } + + // Create CMAB client and service + cmabClient := cmab.NewDefaultCmabClient(cmab.ClientOptions{ + HTTPClient: &http.Client{ + Timeout: clientConf.CMAB.RequestTimeout, + }, + RetryConfig: retryConfig, + Logger: logging.GetLogger(sdkKey, "CmabClient"), + }) + + cmabService := cmab.NewDefaultCmabService(cmab.ServiceOptions{ + Logger: logging.GetLogger(sdkKey, "CmabService"), + CmabCache: odpCachePkg.NewLRUCache(cacheSize, cacheTTL), + CmabClient: cmabClient, + }) + + clientOptions = append(clientOptions, client.WithCmabService(cmabService)) + optimizelyClient, err := optimizelyFactory.Client( clientOptions..., ) diff --git a/pkg/optimizely/cache_test.go b/pkg/optimizely/cache_test.go index ec2b8ddb..62a44b5b 100644 --- a/pkg/optimizely/cache_test.go +++ b/pkg/optimizely/cache_test.go @@ -34,10 +34,10 @@ import ( odpCacheServices "github.com/optimizely/agent/plugins/odpcache/services" "github.com/optimizely/agent/plugins/userprofileservice" "github.com/optimizely/agent/plugins/userprofileservice/services" + "github.com/optimizely/go-sdk/v2/pkg/cache" sdkconfig "github.com/optimizely/go-sdk/v2/pkg/config" "github.com/optimizely/go-sdk/v2/pkg/decision" "github.com/optimizely/go-sdk/v2/pkg/event" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" ) var counter int @@ -795,6 +795,286 @@ func (s *DefaultLoaderTestSuite) TestDefaultRegexValidator() { } } +// Add these tests to your existing cache_test.go file + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationParsing() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": 500, + "ttl": "15m", + }, + RetryConfig: map[string]interface{}{ + "maxRetries": 5, + "initialBackoff": "200ms", + "maxBackoff": "30s", + "backoffMultiplier": 1.5, + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) + // Note: We can't directly test the CMAB service since it's internal to the OptimizelyClient + // But we can verify the loader doesn't error with valid CMAB config +} + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationDefaults() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + // Empty cache and retry config should use defaults + Cache: map[string]interface{}{}, + RetryConfig: map[string]interface{}{}, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABCacheConfigInvalidTTL() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": 1000, + "ttl": "invalid-duration", // This should trigger warning and use default + }, + RetryConfig: map[string]interface{}{}, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) // Should not error, just use defaults + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABCacheConfigWrongTypes() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": "not-an-int", // Wrong type, should use default + "ttl": 123, // Wrong type, should use default + }, + RetryConfig: map[string]interface{}{ + "maxRetries": "not-an-int", // Wrong type, should use default + "backoffMultiplier": "not-a-float", // Wrong type, should use default + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) // Should not error, just use defaults + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABRetryConfigInvalidDurations() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{}, + RetryConfig: map[string]interface{}{ + "maxRetries": 3, + "initialBackoff": "invalid-duration", + "maxBackoff": "also-invalid", + "backoffMultiplier": 2.0, + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) // Should not error, just use defaults for invalid durations + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationAllValidValues() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 10 * time.Second, + Cache: map[string]interface{}{ + "size": 2000, + "ttl": "45m", + }, + RetryConfig: map[string]interface{}{ + "maxRetries": 10, + "initialBackoff": "500ms", + "maxBackoff": "1m", + "backoffMultiplier": 3.0, + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABWithZeroRequestTimeout() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 0, // Zero timeout + Cache: map[string]interface{}{}, + RetryConfig: map[string]interface{}{}, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) +} + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationEdgeCases() { + testCases := []struct { + name string + config config.CMABConfig + }{ + { + name: "Zero cache size", + config: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": 0, + "ttl": "30m", + }, + RetryConfig: map[string]interface{}{}, + }, + }, + { + name: "Zero max retries", + config: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{}, + RetryConfig: map[string]interface{}{ + "maxRetries": 0, + }, + }, + }, + { + name: "Very short TTL", + config: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "ttl": "1ms", + }, + RetryConfig: map[string]interface{}{}, + }, + }, + { + name: "Very long TTL", + config: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "ttl": "24h", + }, + RetryConfig: map[string]interface{}{}, + }, + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: tc.config, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err, "Should not error for case: %s", tc.name) + s.NotNil(client, "Client should not be nil for case: %s", tc.name) + }) + } +} + +func (s *DefaultLoaderTestSuite) TestCMABConfigurationNilMaps() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: nil, // nil map + RetryConfig: nil, // nil map + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) +} + +// Test that CMAB configuration doesn't interfere with existing functionality +func (s *DefaultLoaderTestSuite) TestCMABWithExistingServices() { + conf := config.ClientConfig{ + SdkKeyRegex: "sdkkey", + UserProfileService: map[string]interface{}{ + "default": "in-memory", + "services": map[string]interface{}{ + "in-memory": map[string]interface{}{ + "capacity": 100, + "storageStrategy": "fifo", + }, + }, + }, + ODP: config.OdpConfig{ + SegmentsCache: map[string]interface{}{ + "default": "in-memory", + "services": map[string]interface{}{ + "in-memory": map[string]interface{}{ + "size": 50, + "timeout": "10s", + }, + }, + }, + }, + CMAB: config.CMABConfig{ + RequestTimeout: 5 * time.Second, + Cache: map[string]interface{}{ + "size": 1000, + "ttl": "30m", + }, + RetryConfig: map[string]interface{}{ + "maxRetries": 5, + }, + }, + } + + loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) + client, err := loader("sdkkey") + + s.NoError(err) + s.NotNil(client) + s.NotNil(client.UserProfileService, "UPS should still be configured") + s.NotNil(client.odpCache, "ODP Cache should still be configured") +} + func TestDefaultLoaderTestSuite(t *testing.T) { suite.Run(t, new(DefaultLoaderTestSuite)) } diff --git a/pkg/optimizely/client.go b/pkg/optimizely/client.go index 374171c1..850f413e 100644 --- a/pkg/optimizely/client.go +++ b/pkg/optimizely/client.go @@ -27,7 +27,7 @@ import ( optimizelyclient "github.com/optimizely/go-sdk/v2/pkg/client" "github.com/optimizely/go-sdk/v2/pkg/decision" "github.com/optimizely/go-sdk/v2/pkg/entities" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" ) // ErrEntityNotFound is returned when no entity exists with a given key diff --git a/pkg/optimizely/optimizelytest/config.go b/pkg/optimizely/optimizelytest/config.go index f8bde648..e5cb0a57 100644 --- a/pkg/optimizely/optimizelytest/config.go +++ b/pkg/optimizely/optimizelytest/config.go @@ -518,6 +518,26 @@ func (c *TestProjectConfig) GetFlagVariationsMap() map[string][]entities.Variati return c.flagVariationsMap } +// GetAttributeKeyByID returns the attribute key for the given ID +func (c *TestProjectConfig) GetAttributeKeyByID(id string) (string, error) { + for _, attr := range c.AttributeMap { + if attr.ID == id { + return attr.Key, nil + } + } + return "", fmt.Errorf(`attribute with ID "%s" not found`, id) +} + +// GetExperimentByID returns the experiment with the given ID +func (c *TestProjectConfig) GetExperimentByID(experimentID string) (entities.Experiment, error) { + for _, experiment := range c.ExperimentMap { + if experiment.ID == experimentID { + return experiment, nil + } + } + return entities.Experiment{}, fmt.Errorf(`experiment with ID "%s" not found`, experimentID) +} + // NewConfig initializes a new datafile from a json byte array using the default JSON datafile parser func NewConfig() *TestProjectConfig { config := &TestProjectConfig{ diff --git a/plugins/odpcache/registry.go b/plugins/odpcache/registry.go index 20573067..326bd52d 100644 --- a/plugins/odpcache/registry.go +++ b/plugins/odpcache/registry.go @@ -20,7 +20,7 @@ package odpcache import ( "fmt" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" ) // Creator type defines a function for creating an instance of a Cache diff --git a/plugins/odpcache/registry_test.go b/plugins/odpcache/registry_test.go index 08e8b315..4b58727d 100644 --- a/plugins/odpcache/registry_test.go +++ b/plugins/odpcache/registry_test.go @@ -20,7 +20,7 @@ package odpcache import ( "testing" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" "github.com/stretchr/testify/assert" ) diff --git a/plugins/odpcache/services/in_memory_cache.go b/plugins/odpcache/services/in_memory_cache.go index dad28fda..1e1f4123 100644 --- a/plugins/odpcache/services/in_memory_cache.go +++ b/plugins/odpcache/services/in_memory_cache.go @@ -20,7 +20,7 @@ package services import ( "github.com/optimizely/agent/plugins/odpcache" "github.com/optimizely/agent/plugins/utils" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" ) // InMemoryCache represents the in-memory implementation of Cache interface diff --git a/plugins/odpcache/services/redis_cache.go b/plugins/odpcache/services/redis_cache.go index 73612670..c52f847c 100644 --- a/plugins/odpcache/services/redis_cache.go +++ b/plugins/odpcache/services/redis_cache.go @@ -24,7 +24,7 @@ import ( "github.com/go-redis/redis/v8" "github.com/optimizely/agent/plugins/odpcache" "github.com/optimizely/agent/plugins/utils" - "github.com/optimizely/go-sdk/v2/pkg/odp/cache" + "github.com/optimizely/go-sdk/v2/pkg/cache" "github.com/rs/zerolog/log" ) From 7adae03beba1345eafe746103d1f00ef173f2446 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Thu, 24 Jul 2025 23:11:55 -0700 Subject: [PATCH 09/25] Use go-sdk branch mpirnovar-cmab-gosdk-agent-fssdk-11589 for CMAB support --- go.mod | 2 +- go.sum | 4 ++-- pkg/optimizely/cache_test.go | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 51eaf1b6..c1363816 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.2 github.com/google/uuid v1.3.1 github.com/lestrrat-go/jwx/v2 v2.0.20 - github.com/optimizely/go-sdk/v2 v2.0.0 + github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725054523-d5d321c113b3 github.com/orcaman/concurrent-map v1.0.0 github.com/prometheus/client_golang v1.18.0 github.com/rakyll/statik v0.1.7 diff --git a/go.sum b/go.sum index bf656eec..c2b12b4b 100644 --- a/go.sum +++ b/go.sum @@ -229,8 +229,8 @@ github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE= github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs= -github.com/optimizely/go-sdk/v2 v2.0.0 h1:GUwblFAgWc6FbRujtNZ4cz6+VlfaVqWXkq8PGs7Hgrw= -github.com/optimizely/go-sdk/v2 v2.0.0/go.mod h1:MusRCFsU7+XzJCoCTgheLoENJSf1iiFYm4KbJqz6BYA= +github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725054523-d5d321c113b3 h1:5uif06fihWaXZhr/Ic11mQ4XB2dUs04FXb+E6G0j4Tw= +github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725054523-d5d321c113b3/go.mod h1:MusRCFsU7+XzJCoCTgheLoENJSf1iiFYm4KbJqz6BYA= github.com/orcaman/concurrent-map v1.0.0 h1:I/2A2XPCb4IuQWcQhBhSwGfiuybl/J0ev9HDbW65HOY= github.com/orcaman/concurrent-map v1.0.0/go.mod h1:Lu3tH6HLW3feq74c2GC+jIMS/K2CFcDWnWD9XkenwhI= github.com/pelletier/go-toml/v2 v2.0.6 h1:nrzqCb7j9cDFj2coyLNLaZuJTLjWjlaz6nvTvIwycIU= diff --git a/pkg/optimizely/cache_test.go b/pkg/optimizely/cache_test.go index 62a44b5b..641f6d8a 100644 --- a/pkg/optimizely/cache_test.go +++ b/pkg/optimizely/cache_test.go @@ -795,8 +795,6 @@ func (s *DefaultLoaderTestSuite) TestDefaultRegexValidator() { } } -// Add these tests to your existing cache_test.go file - func (s *DefaultLoaderTestSuite) TestCMABConfigurationParsing() { conf := config.ClientConfig{ SdkKeyRegex: "sdkkey", From 789914e43312447fc710f8b0deb55ee6ca60b745 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 25 Jul 2025 10:51:35 -0700 Subject: [PATCH 10/25] fix formatting --- pkg/optimizely/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/optimizely/client.go b/pkg/optimizely/client.go index 850f413e..248aae53 100644 --- a/pkg/optimizely/client.go +++ b/pkg/optimizely/client.go @@ -24,10 +24,10 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + "github.com/optimizely/go-sdk/v2/pkg/cache" optimizelyclient "github.com/optimizely/go-sdk/v2/pkg/client" "github.com/optimizely/go-sdk/v2/pkg/decision" "github.com/optimizely/go-sdk/v2/pkg/entities" - "github.com/optimizely/go-sdk/v2/pkg/cache" ) // ErrEntityNotFound is returned when no entity exists with a given key From fbea582595330f0ecae8dafc1fbd33b73474ebda Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 25 Jul 2025 14:26:37 -0700 Subject: [PATCH 11/25] surface cmabUUID in Decide API response --- pkg/handlers/decide.go | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/pkg/handlers/decide.go b/pkg/handlers/decide.go index 9607f6e5..0181fc2b 100644 --- a/pkg/handlers/decide.go +++ b/pkg/handlers/decide.go @@ -56,6 +56,7 @@ type DecideOut struct { client.OptimizelyDecision Variables map[string]interface{} `json:"variables,omitempty"` IsEveryoneElseVariation bool `json:"isEveryoneElseVariation"` + CmabUUID *string `json:"cmabUUID,omitempty"` } // Decide makes feature decisions for the selected query parameters @@ -118,35 +119,39 @@ func Decide(w http.ResponseWriter, r *http.Request) { key := keys[0] logger.Debug().Str("featureKey", key).Msg("fetching feature decision") d := optimizelyUserContext.Decide(key, decideOptions) - decideOut := DecideOut{d, d.Variables.ToMap(), isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey)} + var cmabUUID *string + if d.CmabUUID != "" { + cmabUUID = &d.CmabUUID + } + decideOut := DecideOut{ + OptimizelyDecision: d, + Variables: d.Variables.ToMap(), + IsEveryoneElseVariation: isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey), + CmabUUID: cmabUUID, + } render.JSON(w, r, decideOut) return - default: // Decide for Keys decides = optimizelyUserContext.DecideForKeys(keys, decideOptions) } decideOuts := []DecideOut{} for _, d := range decides { - decideOut := DecideOut{d, d.Variables.ToMap(), isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey)} + var cmabUUID *string + if d.CmabUUID != "" { + cmabUUID = &d.CmabUUID + } + decideOut := DecideOut{ + OptimizelyDecision: d, + Variables: d.Variables.ToMap(), + IsEveryoneElseVariation: isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey), + CmabUUID: cmabUUID, + } decideOuts = append(decideOuts, decideOut) logger.Debug().Msgf("Feature %q is enabled for user %s? %t", d.FlagKey, d.UserContext.UserID, d.Enabled) } render.JSON(w, r, decideOuts) -} - -func getUserContextWithOptions(r *http.Request) (DecideBody, error) { - var body DecideBody - err := ParseRequestBody(r, &body) - if err != nil { - return DecideBody{}, err - } - - if body.UserID == "" { - return DecideBody{}, ErrEmptyUserID - } - - return body, nil + return } func isEveryoneElseVariation(rules []config.OptimizelyExperiment, ruleKey string) bool { From f2be578d4d2e31e375b106f5082a1352ffeb200a Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 25 Jul 2025 17:52:58 -0700 Subject: [PATCH 12/25] Add support for returning the cmabUUID field in Decide API responses and tests. --- go.mod | 2 +- go.sum | 4 +- pkg/handlers/decide.go | 76 ++++++++++++++++++++++++++----------- pkg/handlers/decide_test.go | 13 +++++++ 4 files changed, 69 insertions(+), 26 deletions(-) diff --git a/go.mod b/go.mod index c1363816..3f4b49c8 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.2 github.com/google/uuid v1.3.1 github.com/lestrrat-go/jwx/v2 v2.0.20 - github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725054523-d5d321c113b3 + github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725211427-2baee7ec755d github.com/orcaman/concurrent-map v1.0.0 github.com/prometheus/client_golang v1.18.0 github.com/rakyll/statik v0.1.7 diff --git a/go.sum b/go.sum index c2b12b4b..33e3fa1b 100644 --- a/go.sum +++ b/go.sum @@ -229,8 +229,8 @@ github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE= github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs= -github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725054523-d5d321c113b3 h1:5uif06fihWaXZhr/Ic11mQ4XB2dUs04FXb+E6G0j4Tw= -github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725054523-d5d321c113b3/go.mod h1:MusRCFsU7+XzJCoCTgheLoENJSf1iiFYm4KbJqz6BYA= +github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725211427-2baee7ec755d h1:50UbypBJW9larBSnkaePbLe5Kkm6cg1/hAZDpvkRvVg= +github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725211427-2baee7ec755d/go.mod h1:MusRCFsU7+XzJCoCTgheLoENJSf1iiFYm4KbJqz6BYA= github.com/orcaman/concurrent-map v1.0.0 h1:I/2A2XPCb4IuQWcQhBhSwGfiuybl/J0ev9HDbW65HOY= github.com/orcaman/concurrent-map v1.0.0/go.mod h1:Lu3tH6HLW3feq74c2GC+jIMS/K2CFcDWnWD9XkenwhI= github.com/pelletier/go-toml/v2 v2.0.6 h1:nrzqCb7j9cDFj2coyLNLaZuJTLjWjlaz6nvTvIwycIU= diff --git a/pkg/handlers/decide.go b/pkg/handlers/decide.go index 0181fc2b..ade8e617 100644 --- a/pkg/handlers/decide.go +++ b/pkg/handlers/decide.go @@ -109,19 +109,35 @@ func Decide(w http.ResponseWriter, r *http.Request) { featureMap = cfg.FeaturesMap } - var decides map[string]client.OptimizelyDecision switch len(keys) { case 0: // Decide All - decides = optimizelyUserContext.DecideAll(decideOptions) + decides := optimizelyUserContext.DecideAll(decideOptions) + decideOuts := []DecideOut{} + for _, d := range decides { + var cmabUUID *string + if d.CmabUUID != nil && *d.CmabUUID != "" { + cmabUUID = d.CmabUUID + } + decideOut := DecideOut{ + OptimizelyDecision: d, + Variables: d.Variables.ToMap(), + IsEveryoneElseVariation: isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey), + CmabUUID: cmabUUID, + } + decideOuts = append(decideOuts, decideOut) + logger.Debug().Msgf("Feature %q is enabled for user %s? %t", d.FlagKey, d.UserContext.UserID, d.Enabled) + } + render.JSON(w, r, decideOuts) + return case 1: - // Decide + // Decide single key key := keys[0] logger.Debug().Str("featureKey", key).Msg("fetching feature decision") d := optimizelyUserContext.Decide(key, decideOptions) var cmabUUID *string - if d.CmabUUID != "" { - cmabUUID = &d.CmabUUID + if d.CmabUUID != nil && *d.CmabUUID != "" { + cmabUUID = d.CmabUUID } decideOut := DecideOut{ OptimizelyDecision: d, @@ -131,27 +147,41 @@ func Decide(w http.ResponseWriter, r *http.Request) { } render.JSON(w, r, decideOut) return - // Decide for Keys - decides = optimizelyUserContext.DecideForKeys(keys, decideOptions) + default: + // Decide for multiple keys + decides := optimizelyUserContext.DecideForKeys(keys, decideOptions) + decideOuts := []DecideOut{} + for _, d := range decides { + var cmabUUID *string + if d.CmabUUID != nil && *d.CmabUUID != "" { + cmabUUID = d.CmabUUID + } + decideOut := DecideOut{ + OptimizelyDecision: d, + Variables: d.Variables.ToMap(), + IsEveryoneElseVariation: isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey), + CmabUUID: cmabUUID, + } + decideOuts = append(decideOuts, decideOut) + logger.Debug().Msgf("Feature %q is enabled for user %s? %t", d.FlagKey, d.UserContext.UserID, d.Enabled) + } + render.JSON(w, r, decideOuts) + return } +} - decideOuts := []DecideOut{} - for _, d := range decides { - var cmabUUID *string - if d.CmabUUID != "" { - cmabUUID = &d.CmabUUID - } - decideOut := DecideOut{ - OptimizelyDecision: d, - Variables: d.Variables.ToMap(), - IsEveryoneElseVariation: isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey), - CmabUUID: cmabUUID, - } - decideOuts = append(decideOuts, decideOut) - logger.Debug().Msgf("Feature %q is enabled for user %s? %t", d.FlagKey, d.UserContext.UserID, d.Enabled) +func getUserContextWithOptions(r *http.Request) (DecideBody, error) { + var body DecideBody + err := ParseRequestBody(r, &body) + if err != nil { + return DecideBody{}, err + } + + if body.UserID == "" { + return DecideBody{}, ErrEmptyUserID } - render.JSON(w, r, decideOuts) - return + + return body, nil } func isEveryoneElseVariation(rules []config.OptimizelyExperiment, ruleKey string) bool { diff --git a/pkg/handlers/decide_test.go b/pkg/handlers/decide_test.go index 7e47e791..4bf8d34f 100644 --- a/pkg/handlers/decide_test.go +++ b/pkg/handlers/decide_test.go @@ -550,9 +550,11 @@ func (suite *DecideTestSuite) TestDecideMultipleFlags() { Enabled: true, VariationKey: "3", Reasons: []string{}, + CmabUUID: nil, }, Variables: nil, IsEveryoneElseVariation: false, + CmabUUID: nil, }, { OptimizelyDecision: client.OptimizelyDecision{ @@ -562,15 +564,19 @@ func (suite *DecideTestSuite) TestDecideMultipleFlags() { Enabled: true, VariationKey: "6", Reasons: []string{}, + CmabUUID: nil, }, Variables: nil, IsEveryoneElseVariation: false, + CmabUUID: nil, }, } // Toggle between tracking and no tracking. for _, body := range [][]byte{suite.body, suite.bodyEvent} { req := httptest.NewRequest("POST", "/decide?keys=featureA&keys=featureB", bytes.NewBuffer(body)) + fmt.Printf("Request URL: %s\n", req.URL.String()) + fmt.Printf("Request Body: %s\n", string(body)) rec := httptest.NewRecorder() suite.mux.ServeHTTP(rec, req) @@ -580,6 +586,9 @@ func (suite *DecideTestSuite) TestDecideMultipleFlags() { var actual []DecideOut err := json.Unmarshal(rec.Body.Bytes(), &actual) + fmt.Printf("Response Body: %s\n", rec.Body.String()) + fmt.Printf("Unmarshalled actual: %+v\n", actual) + suite.NoError(err) suite.ElementsMatch(expected, actual) } @@ -611,9 +620,11 @@ func (suite *DecideTestSuite) TestDecideAllFlags() { Enabled: true, VariationKey: "3", Reasons: []string{}, + CmabUUID: nil, }, Variables: nil, IsEveryoneElseVariation: false, + CmabUUID: nil, }, { OptimizelyDecision: client.OptimizelyDecision{ @@ -623,9 +634,11 @@ func (suite *DecideTestSuite) TestDecideAllFlags() { Enabled: true, VariationKey: "6", Reasons: []string{}, + CmabUUID: nil, }, Variables: nil, IsEveryoneElseVariation: false, + CmabUUID: nil, }, { OptimizelyDecision: client.OptimizelyDecision{ From 95249f009734e3fa311ce3f11de4e5088e010d91 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Sat, 26 Jul 2025 15:17:21 -0700 Subject: [PATCH 13/25] remove duplicate CmabUUID --- pkg/handlers/decide.go | 16 ---------------- pkg/handlers/decide_test.go | 4 ---- 2 files changed, 20 deletions(-) diff --git a/pkg/handlers/decide.go b/pkg/handlers/decide.go index ade8e617..0dae77eb 100644 --- a/pkg/handlers/decide.go +++ b/pkg/handlers/decide.go @@ -56,7 +56,6 @@ type DecideOut struct { client.OptimizelyDecision Variables map[string]interface{} `json:"variables,omitempty"` IsEveryoneElseVariation bool `json:"isEveryoneElseVariation"` - CmabUUID *string `json:"cmabUUID,omitempty"` } // Decide makes feature decisions for the selected query parameters @@ -115,15 +114,10 @@ func Decide(w http.ResponseWriter, r *http.Request) { decides := optimizelyUserContext.DecideAll(decideOptions) decideOuts := []DecideOut{} for _, d := range decides { - var cmabUUID *string - if d.CmabUUID != nil && *d.CmabUUID != "" { - cmabUUID = d.CmabUUID - } decideOut := DecideOut{ OptimizelyDecision: d, Variables: d.Variables.ToMap(), IsEveryoneElseVariation: isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey), - CmabUUID: cmabUUID, } decideOuts = append(decideOuts, decideOut) logger.Debug().Msgf("Feature %q is enabled for user %s? %t", d.FlagKey, d.UserContext.UserID, d.Enabled) @@ -135,15 +129,10 @@ func Decide(w http.ResponseWriter, r *http.Request) { key := keys[0] logger.Debug().Str("featureKey", key).Msg("fetching feature decision") d := optimizelyUserContext.Decide(key, decideOptions) - var cmabUUID *string - if d.CmabUUID != nil && *d.CmabUUID != "" { - cmabUUID = d.CmabUUID - } decideOut := DecideOut{ OptimizelyDecision: d, Variables: d.Variables.ToMap(), IsEveryoneElseVariation: isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey), - CmabUUID: cmabUUID, } render.JSON(w, r, decideOut) return @@ -152,15 +141,10 @@ func Decide(w http.ResponseWriter, r *http.Request) { decides := optimizelyUserContext.DecideForKeys(keys, decideOptions) decideOuts := []DecideOut{} for _, d := range decides { - var cmabUUID *string - if d.CmabUUID != nil && *d.CmabUUID != "" { - cmabUUID = d.CmabUUID - } decideOut := DecideOut{ OptimizelyDecision: d, Variables: d.Variables.ToMap(), IsEveryoneElseVariation: isEveryoneElseVariation(featureMap[d.FlagKey].DeliveryRules, d.RuleKey), - CmabUUID: cmabUUID, } decideOuts = append(decideOuts, decideOut) logger.Debug().Msgf("Feature %q is enabled for user %s? %t", d.FlagKey, d.UserContext.UserID, d.Enabled) diff --git a/pkg/handlers/decide_test.go b/pkg/handlers/decide_test.go index 4bf8d34f..e03bfe79 100644 --- a/pkg/handlers/decide_test.go +++ b/pkg/handlers/decide_test.go @@ -554,7 +554,6 @@ func (suite *DecideTestSuite) TestDecideMultipleFlags() { }, Variables: nil, IsEveryoneElseVariation: false, - CmabUUID: nil, }, { OptimizelyDecision: client.OptimizelyDecision{ @@ -568,7 +567,6 @@ func (suite *DecideTestSuite) TestDecideMultipleFlags() { }, Variables: nil, IsEveryoneElseVariation: false, - CmabUUID: nil, }, } @@ -624,7 +622,6 @@ func (suite *DecideTestSuite) TestDecideAllFlags() { }, Variables: nil, IsEveryoneElseVariation: false, - CmabUUID: nil, }, { OptimizelyDecision: client.OptimizelyDecision{ @@ -638,7 +635,6 @@ func (suite *DecideTestSuite) TestDecideAllFlags() { }, Variables: nil, IsEveryoneElseVariation: false, - CmabUUID: nil, }, { OptimizelyDecision: client.OptimizelyDecision{ From 70f98d86fdeb3b9fe02cfb75a4595cc9e204cc03 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Sat, 26 Jul 2025 15:43:59 -0700 Subject: [PATCH 14/25] Add configurable CMAB prediction endpoint for FSC testing --- config/config.go | 6 +++++- config/config_test.go | 2 ++ pkg/optimizely/cache.go | 5 +++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 9a90f08a..8735ab0d 100644 --- a/config/config.go +++ b/config/config.go @@ -141,7 +141,8 @@ func NewDefaultConfig() *AgentConfig { }, }, CMAB: CMABConfig{ - RequestTimeout: 10 * time.Second, + RequestTimeout: 10 * time.Second, + PredictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s", Cache: map[string]interface{}{ "type": "memory", "size": 1000, @@ -408,6 +409,9 @@ type CMABConfig struct { // RequestTimeout is the timeout for CMAB API requests RequestTimeout time.Duration `json:"requestTimeout"` + // PredictionEndpoint is the URL template for CMAB prediction API + PredictionEndpoint string `json:"predictionEndpoint"` + // Cache configuration Cache map[string]interface{} `json:"cache"` diff --git a/config/config_test.go b/config/config_test.go index 1f23eed0..a37f0c54 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -102,6 +102,7 @@ func TestDefaultConfig(t *testing.T) { // CMAB configuration assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) + assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) // Test cache settings as maps cacheMap := conf.CMAB.Cache @@ -255,6 +256,7 @@ func TestDefaultCMABConfig(t *testing.T) { // Test default values assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) + assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) // Test default cache settings as maps cacheMap := conf.CMAB.Cache diff --git a/pkg/optimizely/cache.go b/pkg/optimizely/cache.go index 17763d54..1bba1971 100644 --- a/pkg/optimizely/cache.go +++ b/pkg/optimizely/cache.go @@ -359,6 +359,11 @@ func defaultLoader( retryConfig.BackoffMultiplier = multiplier } + // Set CMAB prediction endpoint if configured + if clientConf.CMAB.PredictionEndpoint != "" { + cmab.CMABPredictionEndpoint = clientConf.CMAB.PredictionEndpoint + } + // Create CMAB client and service cmabClient := cmab.NewDefaultCmabClient(cmab.ClientOptions{ HTTPClient: &http.Client{ From 9988eeea38ca16ec567bebc233e097b5d8bae6f9 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Sun, 27 Jul 2025 10:49:35 -0700 Subject: [PATCH 15/25] Force GitHub refresh From 79b5da0280d54d9b50248f2a37f9363e50ba68ff Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Sun, 27 Jul 2025 11:48:26 -0700 Subject: [PATCH 16/25] add prediction endpoint handling to main.go --- cmd/optimizely/main.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index 29dfaadf..d52884a1 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -122,6 +122,9 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { if retryConfig, ok := cmab["retryConfig"].(map[string]interface{}); ok { conf.CMAB.RetryConfig = retryConfig } + if predictionEndpoint, ok := cmab["predictionEndpoint"].(string); ok { + conf.CMAB.PredictionEndpoint = predictionEndpoint + } } // Check for individual map sections @@ -133,6 +136,11 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { conf.CMAB.RetryConfig = cmabRetryConfig } + // Check for individual predictionEndpoint field + if predictionEndpoint := v.GetString("cmab.predictionEndpoint"); predictionEndpoint != "" { + conf.CMAB.PredictionEndpoint = predictionEndpoint + } + return conf } From 7151c5b117064dd6c863dc42875ed02ff0b2021e Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Mon, 28 Jul 2025 13:11:24 -0700 Subject: [PATCH 17/25] Update agent to use CMAB configuration approach --- cmd/optimizely/main.go | 7 ------ config/config.go | 6 +---- go.mod | 2 +- go.sum | 2 ++ pkg/optimizely/cache.go | 56 ++++++++++++++++------------------------- 5 files changed, 25 insertions(+), 48 deletions(-) diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index d52884a1..7be73385 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -122,9 +122,6 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { if retryConfig, ok := cmab["retryConfig"].(map[string]interface{}); ok { conf.CMAB.RetryConfig = retryConfig } - if predictionEndpoint, ok := cmab["predictionEndpoint"].(string); ok { - conf.CMAB.PredictionEndpoint = predictionEndpoint - } } // Check for individual map sections @@ -136,10 +133,6 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { conf.CMAB.RetryConfig = cmabRetryConfig } - // Check for individual predictionEndpoint field - if predictionEndpoint := v.GetString("cmab.predictionEndpoint"); predictionEndpoint != "" { - conf.CMAB.PredictionEndpoint = predictionEndpoint - } return conf } diff --git a/config/config.go b/config/config.go index 8735ab0d..9a90f08a 100644 --- a/config/config.go +++ b/config/config.go @@ -141,8 +141,7 @@ func NewDefaultConfig() *AgentConfig { }, }, CMAB: CMABConfig{ - RequestTimeout: 10 * time.Second, - PredictionEndpoint: "https://prediction.cmab.optimizely.com/predict/%s", + RequestTimeout: 10 * time.Second, Cache: map[string]interface{}{ "type": "memory", "size": 1000, @@ -409,9 +408,6 @@ type CMABConfig struct { // RequestTimeout is the timeout for CMAB API requests RequestTimeout time.Duration `json:"requestTimeout"` - // PredictionEndpoint is the URL template for CMAB prediction API - PredictionEndpoint string `json:"predictionEndpoint"` - // Cache configuration Cache map[string]interface{} `json:"cache"` diff --git a/go.mod b/go.mod index 3f4b49c8..1df25d47 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.2 github.com/google/uuid v1.3.1 github.com/lestrrat-go/jwx/v2 v2.0.20 - github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725211427-2baee7ec755d + github.com/optimizely/go-sdk/v2 v2.1.1-0.20250728200455-37780395c725 github.com/orcaman/concurrent-map v1.0.0 github.com/prometheus/client_golang v1.18.0 github.com/rakyll/statik v0.1.7 diff --git a/go.sum b/go.sum index 33e3fa1b..41ea334c 100644 --- a/go.sum +++ b/go.sum @@ -231,6 +231,8 @@ github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE= github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs= github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725211427-2baee7ec755d h1:50UbypBJW9larBSnkaePbLe5Kkm6cg1/hAZDpvkRvVg= github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725211427-2baee7ec755d/go.mod h1:MusRCFsU7+XzJCoCTgheLoENJSf1iiFYm4KbJqz6BYA= +github.com/optimizely/go-sdk/v2 v2.1.1-0.20250728200455-37780395c725 h1:CsafFEQ3xBhKiMyKho47oCyHYuWy9c8w20D5Oc2b4dw= +github.com/optimizely/go-sdk/v2 v2.1.1-0.20250728200455-37780395c725/go.mod h1:MusRCFsU7+XzJCoCTgheLoENJSf1iiFYm4KbJqz6BYA= github.com/orcaman/concurrent-map v1.0.0 h1:I/2A2XPCb4IuQWcQhBhSwGfiuybl/J0ev9HDbW65HOY= github.com/orcaman/concurrent-map v1.0.0/go.mod h1:Lu3tH6HLW3feq74c2GC+jIMS/K2CFcDWnWD9XkenwhI= github.com/pelletier/go-toml/v2 v2.0.6 h1:nrzqCb7j9cDFj2coyLNLaZuJTLjWjlaz6nvTvIwycIU= diff --git a/pkg/optimizely/cache.go b/pkg/optimizely/cache.go index 1bba1971..f62114d2 100644 --- a/pkg/optimizely/cache.go +++ b/pkg/optimizely/cache.go @@ -21,7 +21,6 @@ import ( "context" "encoding/json" "errors" - "net/http" "regexp" "strings" "sync" @@ -37,7 +36,7 @@ import ( "github.com/optimizely/agent/plugins/userprofileservice" odpCachePkg "github.com/optimizely/go-sdk/v2/pkg/cache" "github.com/optimizely/go-sdk/v2/pkg/client" - cmab "github.com/optimizely/go-sdk/v2/pkg/cmab" + "github.com/optimizely/go-sdk/v2/pkg/cmab" sdkconfig "github.com/optimizely/go-sdk/v2/pkg/config" "github.com/optimizely/go-sdk/v2/pkg/decision" "github.com/optimizely/go-sdk/v2/pkg/event" @@ -316,8 +315,8 @@ func defaultLoader( clientOptions = append(clientOptions, client.WithOdpManager(odpManager)) // Parse CMAB cache configuration - cacheSize := 1000 // default - cacheTTL := 30 * time.Minute // default + cacheSize := cmab.DefaultCacheSize + cacheTTL := cmab.DefaultCacheTTL if cacheConfig, ok := clientConf.CMAB.Cache["size"].(int); ok { cacheSize = cacheConfig @@ -331,55 +330,42 @@ func defaultLoader( } } - // Parse retry configuration + // Create retry config retryConfig := &cmab.RetryConfig{ - MaxRetries: 3, - InitialBackoff: 100 * time.Millisecond, - MaxBackoff: 10 * time.Second, - BackoffMultiplier: 2.0, + MaxRetries: cmab.DefaultMaxRetries, + InitialBackoff: cmab.DefaultInitialBackoff, + MaxBackoff: cmab.DefaultMaxBackoff, + BackoffMultiplier: cmab.DefaultBackoffMultiplier, } + // Parse retry configuration if maxRetries, ok := clientConf.CMAB.RetryConfig["maxRetries"].(int); ok { retryConfig.MaxRetries = maxRetries } - if initialBackoffStr, ok := clientConf.CMAB.RetryConfig["initialBackoff"].(string); ok { - if parsedBackoff, err := time.ParseDuration(initialBackoffStr); err == nil { - retryConfig.InitialBackoff = parsedBackoff + if backoff, err := time.ParseDuration(initialBackoffStr); err == nil { + retryConfig.InitialBackoff = backoff } } - if maxBackoffStr, ok := clientConf.CMAB.RetryConfig["maxBackoff"].(string); ok { - if parsedBackoff, err := time.ParseDuration(maxBackoffStr); err == nil { - retryConfig.MaxBackoff = parsedBackoff + if backoff, err := time.ParseDuration(maxBackoffStr); err == nil { + retryConfig.MaxBackoff = backoff } } - if multiplier, ok := clientConf.CMAB.RetryConfig["backoffMultiplier"].(float64); ok { retryConfig.BackoffMultiplier = multiplier } - // Set CMAB prediction endpoint if configured - if clientConf.CMAB.PredictionEndpoint != "" { - cmab.CMABPredictionEndpoint = clientConf.CMAB.PredictionEndpoint - } - - // Create CMAB client and service - cmabClient := cmab.NewDefaultCmabClient(cmab.ClientOptions{ - HTTPClient: &http.Client{ - Timeout: clientConf.CMAB.RequestTimeout, - }, + // Create CMAB config (NO endpoint configuration - not configurable) + cmabConfig := cmab.Config{ + CacheSize: cacheSize, + CacheTTL: cacheTTL, + HTTPTimeout: clientConf.CMAB.RequestTimeout, RetryConfig: retryConfig, - Logger: logging.GetLogger(sdkKey, "CmabClient"), - }) - - cmabService := cmab.NewDefaultCmabService(cmab.ServiceOptions{ - Logger: logging.GetLogger(sdkKey, "CmabService"), - CmabCache: odpCachePkg.NewLRUCache(cacheSize, cacheTTL), - CmabClient: cmabClient, - }) + } - clientOptions = append(clientOptions, client.WithCmabService(cmabService)) + // Add to client options + clientOptions = append(clientOptions, client.WithCmabConfig(cmabConfig)) optimizelyClient, err := optimizelyFactory.Client( clientOptions..., From bd5d113095db44860fa810be6ce1619a539ebbe0 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Mon, 28 Jul 2025 13:13:57 -0700 Subject: [PATCH 18/25] fix tests --- config/config_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index a37f0c54..1f23eed0 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -102,7 +102,6 @@ func TestDefaultConfig(t *testing.T) { // CMAB configuration assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) - assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) // Test cache settings as maps cacheMap := conf.CMAB.Cache @@ -256,7 +255,6 @@ func TestDefaultCMABConfig(t *testing.T) { // Test default values assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) - assert.Equal(t, "https://prediction.cmab.optimizely.com/predict/%s", conf.CMAB.PredictionEndpoint) // Test default cache settings as maps cacheMap := conf.CMAB.Cache From 7f34019254336849c08c6d6e39d0d51b56c449eb Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Mon, 28 Jul 2025 14:50:54 -0700 Subject: [PATCH 19/25] Force GitHub refresh From f508b7389a2c22b4d6abd061524499afe6c4adc9 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 29 Jul 2025 09:36:33 -0700 Subject: [PATCH 20/25] configure ENV var OPTIMIZELY_CMAB_PREDICTIONENDPOINT to allow fsc tests to run --- pkg/optimizely/cache.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/optimizely/cache.go b/pkg/optimizely/cache.go index f62114d2..725b6467 100644 --- a/pkg/optimizely/cache.go +++ b/pkg/optimizely/cache.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "errors" + "os" "regexp" "strings" "sync" @@ -314,6 +315,14 @@ func defaultLoader( ) clientOptions = append(clientOptions, client.WithOdpManager(odpManager)) + // Configure CMAB prediction endpoint from environment variable + // This allows FSC tests to override the endpoint by setting OPTIMIZELY_CMAB_PREDICTIONENDPOINT + if cmabEndpoint := os.Getenv("OPTIMIZELY_CMAB_PREDICTIONENDPOINT"); cmabEndpoint != "" { + // Set the global variable that go-sdk uses (same pattern as go-sdk FSC tests) + cmab.CMABPredictionEndpoint = cmabEndpoint + "/%s" + log.Info().Str("endpoint", cmabEndpoint).Msg("Using custom CMAB prediction endpoint") + } + // Parse CMAB cache configuration cacheSize := cmab.DefaultCacheSize cacheTTL := cmab.DefaultCacheTTL From a6db9b5adec4b1d2e4e998da379832ba4652d6d6 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 29 Jul 2025 09:59:51 -0700 Subject: [PATCH 21/25] remove %s, already in the endpoint in fsc --- pkg/optimizely/cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/optimizely/cache.go b/pkg/optimizely/cache.go index 725b6467..a875bf23 100644 --- a/pkg/optimizely/cache.go +++ b/pkg/optimizely/cache.go @@ -318,8 +318,8 @@ func defaultLoader( // Configure CMAB prediction endpoint from environment variable // This allows FSC tests to override the endpoint by setting OPTIMIZELY_CMAB_PREDICTIONENDPOINT if cmabEndpoint := os.Getenv("OPTIMIZELY_CMAB_PREDICTIONENDPOINT"); cmabEndpoint != "" { - // Set the global variable that go-sdk uses (same pattern as go-sdk FSC tests) - cmab.CMABPredictionEndpoint = cmabEndpoint + "/%s" + // Set the global variable that go-sdk uses (FSC already includes the /%s format) + cmab.CMABPredictionEndpoint = cmabEndpoint log.Info().Str("endpoint", cmabEndpoint).Msg("Using custom CMAB prediction endpoint") } From 9cc1e9d944318d2404a15bd5e1e1312a1ee9bb74 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 29 Jul 2025 15:14:18 -0700 Subject: [PATCH 22/25] Add client reset functionality for FSC CMAB test isolation --- pkg/handlers/reset.go | 61 +++++++++++++++ pkg/handlers/reset_test.go | 140 +++++++++++++++++++++++++++++++++++ pkg/middleware/cached.go | 4 + pkg/middleware/utils.go | 10 +++ pkg/optimizely/cache.go | 21 ++++++ pkg/optimizely/cache_test.go | 27 +++++++ pkg/routers/api.go | 6 ++ 7 files changed, 269 insertions(+) create mode 100644 pkg/handlers/reset.go create mode 100644 pkg/handlers/reset_test.go diff --git a/pkg/handlers/reset.go b/pkg/handlers/reset.go new file mode 100644 index 00000000..acb58ee8 --- /dev/null +++ b/pkg/handlers/reset.go @@ -0,0 +1,61 @@ +/**************************************************************************** + * Copyright 2025, 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 handlers // +package handlers + +import ( + "errors" + "net/http" + + "github.com/go-chi/render" + + "github.com/optimizely/agent/pkg/middleware" +) + +// ResetClient handles the /v1/reset endpoint from FSC tests +// This clears the client cache to ensure clean state between test scenarios, +// particularly important for CMAB cache testing +func ResetClient(w http.ResponseWriter, r *http.Request) { + // Get SDK key from header + sdkKey := r.Header.Get("X-Optimizely-SDK-Key") + if sdkKey == "" { + RenderError(errors.New("SDK key required for reset"), http.StatusBadRequest, w, r) + return + } + + // Get the cache from context + cache, err := middleware.GetOptlyCache(r) + if err != nil { + RenderError(errors.New("cache not available"), http.StatusInternalServerError, w, r) + return + } + + // Get logger for debugging + logger := middleware.GetLogger(r) + logger.Debug().Str("sdkKey", sdkKey).Msg("Resetting client for FSC test") + + // Reset the client using the cache interface + if optlyCache, ok := cache.(interface{ ResetClient(string) }); ok { + optlyCache.ResetClient(sdkKey) + } else { + RenderError(errors.New("cache reset not supported"), http.StatusInternalServerError, w, r) + return + } + + // Return success + render.JSON(w, r, map[string]interface{}{"result": true}) +} \ No newline at end of file diff --git a/pkg/handlers/reset_test.go b/pkg/handlers/reset_test.go new file mode 100644 index 00000000..0cbaa1ae --- /dev/null +++ b/pkg/handlers/reset_test.go @@ -0,0 +1,140 @@ +/**************************************************************************** + * Copyright 2025, 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 handlers // +package handlers + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/optimizely/agent/pkg/middleware" + "github.com/optimizely/agent/pkg/optimizely" + "github.com/optimizely/agent/pkg/optimizely/optimizelytest" + + "github.com/go-chi/chi/v5" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" +) + +type MockCache struct { + mock.Mock +} + +func (m *MockCache) GetClient(key string) (*optimizely.OptlyClient, error) { + args := m.Called(key) + return args.Get(0).(*optimizely.OptlyClient), args.Error(1) +} + +func (m *MockCache) UpdateConfigs(_ string) { +} + +func (m *MockCache) SetUserProfileService(sdkKey, userProfileService string) { + m.Called(sdkKey, userProfileService) +} + +func (m *MockCache) SetODPCache(sdkKey, odpCache string) { + m.Called(sdkKey, odpCache) +} + +func (m *MockCache) ResetClient(sdkKey string) { + m.Called(sdkKey) +} + +type ResetTestSuite struct { + suite.Suite + oc *optimizely.OptlyClient + tc *optimizelytest.TestClient + mux *chi.Mux + cache *MockCache +} + +func (suite *ResetTestSuite) ClientCtx(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), middleware.OptlyClientKey, suite.oc) + ctx = context.WithValue(ctx, middleware.OptlyCacheKey, suite.cache) + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + +func (suite *ResetTestSuite) SetupTest() { + testClient := optimizelytest.NewClient() + suite.tc = testClient + suite.oc = &optimizely.OptlyClient{OptimizelyClient: testClient.OptimizelyClient} + + mockCache := new(MockCache) + mockCache.On("ResetClient", "test-sdk-key").Return() + suite.cache = mockCache + + mux := chi.NewMux() + mux.Use(suite.ClientCtx) + mux.Post("/reset", ResetClient) + suite.mux = mux +} + +func (suite *ResetTestSuite) TestResetClient() { + req := httptest.NewRequest("POST", "/reset", nil) + req.Header.Set("X-Optimizely-SDK-Key", "test-sdk-key") + recorder := httptest.NewRecorder() + + suite.mux.ServeHTTP(recorder, req) + + suite.Equal(http.StatusOK, recorder.Code) + suite.Contains(recorder.Header().Get("content-type"), "application/json") + suite.Contains(recorder.Body.String(), `"result":true`) + + // Verify ResetClient was called with correct SDK key + suite.cache.AssertCalled(suite.T(), "ResetClient", "test-sdk-key") +} + +func (suite *ResetTestSuite) TestResetClientMissingSDKKey() { + req := httptest.NewRequest("POST", "/reset", nil) + recorder := httptest.NewRecorder() + + suite.mux.ServeHTTP(recorder, req) + + suite.Equal(http.StatusBadRequest, recorder.Code) + suite.Contains(recorder.Body.String(), "SDK key required for reset") +} + +func (suite *ResetTestSuite) TestResetClientCacheNotAvailable() { + // Create a context without cache + req := httptest.NewRequest("POST", "/reset", nil) + req.Header.Set("X-Optimizely-SDK-Key", "test-sdk-key") + recorder := httptest.NewRecorder() + + // Use middleware that doesn't include cache + mux := chi.NewMux() + mux.Use(func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), middleware.OptlyClientKey, suite.oc) + // Note: no cache in context + next.ServeHTTP(w, r.WithContext(ctx)) + }) + }) + mux.Post("/reset", ResetClient) + + mux.ServeHTTP(recorder, req) + + suite.Equal(http.StatusInternalServerError, recorder.Code) + suite.Contains(recorder.Body.String(), "cache not available") +} + +func TestResetTestSuite(t *testing.T) { + suite.Run(t, new(ResetTestSuite)) +} \ No newline at end of file diff --git a/pkg/middleware/cached.go b/pkg/middleware/cached.go index ca50621d..feebcd19 100644 --- a/pkg/middleware/cached.go +++ b/pkg/middleware/cached.go @@ -41,6 +41,9 @@ const OptlyFeatureKey = contextKey("featureKey") // OptlyExperimentKey is the context key used by ExperimentCtx for setting an Experiment const OptlyExperimentKey = contextKey("experimentKey") +// OptlyCacheKey is the context key for the OptlyCache +const OptlyCacheKey = contextKey("optlyCache") + // OptlySDKHeader is the header key for an ad-hoc SDK key const OptlySDKHeader = "X-Optimizely-SDK-Key" @@ -98,6 +101,7 @@ func (mw *CachedOptlyMiddleware) ClientCtx(next http.Handler) http.Handler { } ctx := context.WithValue(r.Context(), OptlyClientKey, optlyClient) + ctx = context.WithValue(ctx, OptlyCacheKey, mw.Cache) next.ServeHTTP(w, r.WithContext(ctx)) }) } diff --git a/pkg/middleware/utils.go b/pkg/middleware/utils.go index 4c5ad136..71e55bf3 100644 --- a/pkg/middleware/utils.go +++ b/pkg/middleware/utils.go @@ -47,6 +47,16 @@ func GetOptlyClient(r *http.Request) (*optimizely.OptlyClient, error) { return optlyClient, nil } +// GetOptlyCache is a utility to extract the OptlyCache from the http request context. +func GetOptlyCache(r *http.Request) (optimizely.Cache, error) { + cache, ok := r.Context().Value(OptlyCacheKey).(optimizely.Cache) + if !ok || cache == nil { + return nil, fmt.Errorf("optlyCache not available") + } + + return cache, nil +} + // GetLogger gets the logger with some info coming from http request func GetLogger(r *http.Request) *zerolog.Logger { reqID := r.Header.Get(OptlyRequestHeader) diff --git a/pkg/optimizely/cache.go b/pkg/optimizely/cache.go index a875bf23..5b598d52 100644 --- a/pkg/optimizely/cache.go +++ b/pkg/optimizely/cache.go @@ -432,3 +432,24 @@ func getServiceWithType(serviceType, sdkKey string, serviceMap cmap.ConcurrentMa } return nil } + +// ResetClient removes the optimizely client from cache to ensure clean state for testing +// This is primarily used by FSC tests to clear CMAB cache between test scenarios +func (c *OptlyCache) ResetClient(sdkKey string) { + // Remove the client from the cache + if val, exists := c.optlyMap.Get(sdkKey); exists { + c.optlyMap.Remove(sdkKey) + + // Close the client to clean up resources + if client, ok := val.(*OptlyClient); ok { + client.Close() + } + + message := "Reset Optimizely client for testing" + if ShouldIncludeSDKKey { + log.Info().Str("sdkKey", sdkKey).Msg(message) + } else { + log.Info().Msg(message) + } + } +} diff --git a/pkg/optimizely/cache_test.go b/pkg/optimizely/cache_test.go index 641f6d8a..785915f7 100644 --- a/pkg/optimizely/cache_test.go +++ b/pkg/optimizely/cache_test.go @@ -310,6 +310,33 @@ func (suite *CacheTestSuite) TestNilCreatorAddedforODPCache() { suite.Nil(odpCache) } +func (suite *CacheTestSuite) TestResetClient() { + // First, get a client to put it in the cache + client, err := suite.cache.GetClient("test-sdk-key") + suite.NoError(err) + suite.NotNil(client) + + // Verify client is in cache + cachedClient, exists := suite.cache.optlyMap.Get("test-sdk-key") + suite.True(exists) + suite.NotNil(cachedClient) + + // Reset the client + suite.cache.ResetClient("test-sdk-key") + + // Verify client is removed from cache + _, exists = suite.cache.optlyMap.Get("test-sdk-key") + suite.False(exists) +} + +func (suite *CacheTestSuite) TestResetClientNonExistent() { + // Reset a client that doesn't exist - should not panic + suite.cache.ResetClient("non-existent-key") + + // Verify no clients are in cache + suite.Equal(0, suite.cache.optlyMap.Count()) +} + // In order for 'go test' to run this suite, we need to create // a normal test function and pass our suite to suite.Run func TestCacheTestSuite(t *testing.T) { diff --git a/pkg/routers/api.go b/pkg/routers/api.go index 79df6103..2d9e48ca 100644 --- a/pkg/routers/api.go +++ b/pkg/routers/api.go @@ -49,6 +49,7 @@ type APIOptions struct { overrideHandler http.HandlerFunc lookupHandler http.HandlerFunc saveHandler http.HandlerFunc + resetHandler http.HandlerFunc sendOdpEventHandler http.HandlerFunc nStreamHandler http.HandlerFunc oAuthHandler http.HandlerFunc @@ -80,6 +81,7 @@ func NewDefaultAPIRouter(optlyCache optimizely.Cache, conf config.AgentConfig, m if !conf.API.EnableOverrides { overrideHandler = forbiddenHandler("Overrides not enabled") } + resetHandler := handlers.ResetClient nStreamHandler := forbiddenHandler("Notification stream not enabled") if conf.API.EnableNotifications { @@ -102,6 +104,7 @@ func NewDefaultAPIRouter(optlyCache optimizely.Cache, conf config.AgentConfig, m overrideHandler: overrideHandler, lookupHandler: handlers.Lookup, saveHandler: handlers.Save, + resetHandler: resetHandler, trackHandler: handlers.TrackEvent, sendOdpEventHandler: handlers.SendOdpEvent, sdkMiddleware: mw.ClientCtx, @@ -131,6 +134,7 @@ func WithAPIRouter(opt *APIOptions, r chi.Router) { overrideTimer := middleware.Metricize("override", opt.metricsRegistry) lookupTimer := middleware.Metricize("lookup", opt.metricsRegistry) saveTimer := middleware.Metricize("save", opt.metricsRegistry) + resetTimer := middleware.Metricize("reset", opt.metricsRegistry) trackTimer := middleware.Metricize("track-event", opt.metricsRegistry) sendOdpEventTimer := middleware.Metricize("send-odp-event", opt.metricsRegistry) createAccesstokenTimer := middleware.Metricize("create-api-access-token", opt.metricsRegistry) @@ -144,6 +148,7 @@ func WithAPIRouter(opt *APIOptions, r chi.Router) { overrideTracer := middleware.AddTracing("overrideHandler", "Override") lookupTracer := middleware.AddTracing("lookupHandler", "Lookup") saveTracer := middleware.AddTracing("saveHandler", "Save") + resetTracer := middleware.AddTracing("resetHandler", "Reset") sendOdpEventTracer := middleware.AddTracing("sendOdpEventHandler", "SendOdpEvent") nStreamTracer := middleware.AddTracing("notificationHandler", "SendNotificationEvent") authTracer := middleware.AddTracing("authHandler", "AuthToken") @@ -164,6 +169,7 @@ func WithAPIRouter(opt *APIOptions, r chi.Router) { r.With(decideTimer, opt.oAuthMiddleware, contentTypeMiddleware, decideTracer).Post("/decide", opt.decideHandler) r.With(trackTimer, opt.oAuthMiddleware, contentTypeMiddleware, trackTracer).Post("/track", opt.trackHandler) r.With(overrideTimer, opt.oAuthMiddleware, contentTypeMiddleware, overrideTracer).Post("/override", opt.overrideHandler) + r.With(resetTimer, opt.oAuthMiddleware, contentTypeMiddleware, resetTracer).Post("/reset", opt.resetHandler) r.With(lookupTimer, opt.oAuthMiddleware, contentTypeMiddleware, lookupTracer).Post("/lookup", opt.lookupHandler) r.With(saveTimer, opt.oAuthMiddleware, contentTypeMiddleware, saveTracer).Post("/save", opt.saveHandler) r.With(sendOdpEventTimer, opt.oAuthMiddleware, contentTypeMiddleware, sendOdpEventTracer).Post("/send-odp-event", opt.sendOdpEventHandler) From ec06047d4f38ded9bf8c0244a31c7192a98a2e20 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Wed, 30 Jul 2025 12:20:28 -0700 Subject: [PATCH 23/25] Trigger PR check From 8a3e27bb32fedc2c53e1239b1ebe59e4a3ebd750 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Wed, 30 Jul 2025 12:40:55 -0700 Subject: [PATCH 24/25] fix formatting issues --- cmd/optimizely/main.go | 1 - pkg/handlers/reset.go | 2 +- pkg/handlers/reset_test.go | 6 +++--- pkg/optimizely/cache.go | 4 ++-- pkg/optimizely/cache_test.go | 8 ++++---- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index 7be73385..29dfaadf 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -133,7 +133,6 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { conf.CMAB.RetryConfig = cmabRetryConfig } - return conf } diff --git a/pkg/handlers/reset.go b/pkg/handlers/reset.go index acb58ee8..5f7420d8 100644 --- a/pkg/handlers/reset.go +++ b/pkg/handlers/reset.go @@ -58,4 +58,4 @@ func ResetClient(w http.ResponseWriter, r *http.Request) { // Return success render.JSON(w, r, map[string]interface{}{"result": true}) -} \ No newline at end of file +} diff --git a/pkg/handlers/reset_test.go b/pkg/handlers/reset_test.go index 0cbaa1ae..479403d6 100644 --- a/pkg/handlers/reset_test.go +++ b/pkg/handlers/reset_test.go @@ -76,7 +76,7 @@ func (suite *ResetTestSuite) SetupTest() { testClient := optimizelytest.NewClient() suite.tc = testClient suite.oc = &optimizely.OptlyClient{OptimizelyClient: testClient.OptimizelyClient} - + mockCache := new(MockCache) mockCache.On("ResetClient", "test-sdk-key").Return() suite.cache = mockCache @@ -97,7 +97,7 @@ func (suite *ResetTestSuite) TestResetClient() { suite.Equal(http.StatusOK, recorder.Code) suite.Contains(recorder.Header().Get("content-type"), "application/json") suite.Contains(recorder.Body.String(), `"result":true`) - + // Verify ResetClient was called with correct SDK key suite.cache.AssertCalled(suite.T(), "ResetClient", "test-sdk-key") } @@ -137,4 +137,4 @@ func (suite *ResetTestSuite) TestResetClientCacheNotAvailable() { func TestResetTestSuite(t *testing.T) { suite.Run(t, new(ResetTestSuite)) -} \ No newline at end of file +} diff --git a/pkg/optimizely/cache.go b/pkg/optimizely/cache.go index 5b598d52..c82f6cd2 100644 --- a/pkg/optimizely/cache.go +++ b/pkg/optimizely/cache.go @@ -439,12 +439,12 @@ func (c *OptlyCache) ResetClient(sdkKey string) { // Remove the client from the cache if val, exists := c.optlyMap.Get(sdkKey); exists { c.optlyMap.Remove(sdkKey) - + // Close the client to clean up resources if client, ok := val.(*OptlyClient); ok { client.Close() } - + message := "Reset Optimizely client for testing" if ShouldIncludeSDKKey { log.Info().Str("sdkKey", sdkKey).Msg(message) diff --git a/pkg/optimizely/cache_test.go b/pkg/optimizely/cache_test.go index 785915f7..820da655 100644 --- a/pkg/optimizely/cache_test.go +++ b/pkg/optimizely/cache_test.go @@ -315,15 +315,15 @@ func (suite *CacheTestSuite) TestResetClient() { client, err := suite.cache.GetClient("test-sdk-key") suite.NoError(err) suite.NotNil(client) - + // Verify client is in cache cachedClient, exists := suite.cache.optlyMap.Get("test-sdk-key") suite.True(exists) suite.NotNil(cachedClient) - + // Reset the client suite.cache.ResetClient("test-sdk-key") - + // Verify client is removed from cache _, exists = suite.cache.optlyMap.Get("test-sdk-key") suite.False(exists) @@ -332,7 +332,7 @@ func (suite *CacheTestSuite) TestResetClient() { func (suite *CacheTestSuite) TestResetClientNonExistent() { // Reset a client that doesn't exist - should not panic suite.cache.ResetClient("non-existent-key") - + // Verify no clients are in cache suite.Equal(0, suite.cache.optlyMap.Count()) } From 7efc05eb872b2cfe1a4655cf0203b3dda0359553 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Thu, 31 Jul 2025 11:05:47 -0700 Subject: [PATCH 25/25] Refactored CMAB configuration from unstructured map[string]interface{} to structured types for better type safety and maintainability. --- cmd/optimizely/main.go | 62 +++++++++++++++-- cmd/optimizely/main_test.go | 125 ++++++--------------------------- config/config.go | 44 +++++++++--- config/config_test.go | 48 ++++++------- go.mod | 2 +- go.sum | 6 +- pkg/optimizely/cache.go | 48 ++++++------- pkg/optimizely/cache_test.go | 129 ++++++++++++++++++----------------- 8 files changed, 227 insertions(+), 237 deletions(-) diff --git a/cmd/optimizely/main.go b/cmd/optimizely/main.go index 29dfaadf..a3aa53cc 100644 --- a/cmd/optimizely/main.go +++ b/cmd/optimizely/main.go @@ -117,20 +117,74 @@ func loadConfig(v *viper.Viper) *config.AgentConfig { } } if cache, ok := cmab["cache"].(map[string]interface{}); ok { - conf.CMAB.Cache = cache + if cacheType, ok := cache["type"].(string); ok { + conf.CMAB.Cache.Type = cacheType + } + if cacheSize, ok := cache["size"].(float64); ok { + conf.CMAB.Cache.Size = int(cacheSize) + } + if cacheTTL, ok := cache["ttl"].(string); ok { + if duration, err := time.ParseDuration(cacheTTL); err == nil { + conf.CMAB.Cache.TTL = duration + } + } } if retryConfig, ok := cmab["retryConfig"].(map[string]interface{}); ok { - conf.CMAB.RetryConfig = retryConfig + if maxRetries, ok := retryConfig["maxRetries"].(float64); ok { + conf.CMAB.RetryConfig.MaxRetries = int(maxRetries) + } + if initialBackoff, ok := retryConfig["initialBackoff"].(string); ok { + if duration, err := time.ParseDuration(initialBackoff); err == nil { + conf.CMAB.RetryConfig.InitialBackoff = duration + } + } + if maxBackoff, ok := retryConfig["maxBackoff"].(string); ok { + if duration, err := time.ParseDuration(maxBackoff); err == nil { + conf.CMAB.RetryConfig.MaxBackoff = duration + } + } + if backoffMultiplier, ok := retryConfig["backoffMultiplier"].(float64); ok { + conf.CMAB.RetryConfig.BackoffMultiplier = backoffMultiplier + } } } // Check for individual map sections if cmabCache := v.GetStringMap("cmab.cache"); len(cmabCache) > 0 { - conf.CMAB.Cache = cmabCache + if cacheType, ok := cmabCache["type"].(string); ok { + conf.CMAB.Cache.Type = cacheType + } + if cacheSize, ok := cmabCache["size"].(int); ok { + conf.CMAB.Cache.Size = cacheSize + } else if cacheSize, ok := cmabCache["size"].(float64); ok { + conf.CMAB.Cache.Size = int(cacheSize) + } + if cacheTTL, ok := cmabCache["ttl"].(string); ok { + if duration, err := time.ParseDuration(cacheTTL); err == nil { + conf.CMAB.Cache.TTL = duration + } + } } if cmabRetryConfig := v.GetStringMap("cmab.retryConfig"); len(cmabRetryConfig) > 0 { - conf.CMAB.RetryConfig = cmabRetryConfig + if maxRetries, ok := cmabRetryConfig["maxRetries"].(int); ok { + conf.CMAB.RetryConfig.MaxRetries = maxRetries + } else if maxRetries, ok := cmabRetryConfig["maxRetries"].(float64); ok { + conf.CMAB.RetryConfig.MaxRetries = int(maxRetries) + } + if initialBackoff, ok := cmabRetryConfig["initialBackoff"].(string); ok { + if duration, err := time.ParseDuration(initialBackoff); err == nil { + conf.CMAB.RetryConfig.InitialBackoff = duration + } + } + if maxBackoff, ok := cmabRetryConfig["maxBackoff"].(string); ok { + if duration, err := time.ParseDuration(maxBackoff); err == nil { + conf.CMAB.RetryConfig.MaxBackoff = duration + } + } + if backoffMultiplier, ok := cmabRetryConfig["backoffMultiplier"].(float64); ok { + conf.CMAB.RetryConfig.BackoffMultiplier = backoffMultiplier + } } return conf diff --git a/cmd/optimizely/main_test.go b/cmd/optimizely/main_test.go index 48336a6f..d5f929a9 100644 --- a/cmd/optimizely/main_test.go +++ b/cmd/optimizely/main_test.go @@ -189,75 +189,18 @@ func assertCMAB(t *testing.T, cmab config.CMABConfig) { // Base assertions assert.Equal(t, 15*time.Second, cmab.RequestTimeout) - // Check if cache map is initialized - cacheMap := cmab.Cache - if cacheMap == nil { - t.Fatal("Cache map is nil") - } - - // Debug cache type - cacheTypeValue := cacheMap["type"] - fmt.Printf("Cache type: %v (%T)\n", cacheTypeValue, cacheTypeValue) - assert.Equal(t, "redis", cacheTypeValue) - - // Debug cache size - cacheSizeValue := cacheMap["size"] - fmt.Printf("Cache size: %v (%T)\n", cacheSizeValue, cacheSizeValue) - sizeValue, ok := cacheSizeValue.(float64) - assert.True(t, ok, "Cache size should be float64") - assert.Equal(t, float64(2000), sizeValue) - - // Cache TTL - cacheTTLValue := cacheMap["ttl"] - fmt.Printf("Cache TTL: %v (%T)\n", cacheTTLValue, cacheTTLValue) - assert.Equal(t, "45m", cacheTTLValue) - - // Redis settings - redisValue := cacheMap["redis"] - fmt.Printf("Redis: %v (%T)\n", redisValue, redisValue) - redisMap, ok := redisValue.(map[string]interface{}) - assert.True(t, ok, "Redis should be a map") - - if !ok { - t.Fatal("Redis is not a map") - } - - redisHost := redisMap["host"] - fmt.Printf("Redis host: %v (%T)\n", redisHost, redisHost) - assert.Equal(t, "redis.example.com:6379", redisHost) - - redisPassword := redisMap["password"] - fmt.Printf("Redis password: %v (%T)\n", redisPassword, redisPassword) - assert.Equal(t, "password123", redisPassword) - - redisDBValue := redisMap["database"] - fmt.Printf("Redis DB: %v (%T)\n", redisDBValue, redisDBValue) - dbValue, ok := redisDBValue.(float64) - assert.True(t, ok, "Redis DB should be float64") - assert.Equal(t, float64(2), dbValue) - - // Retry settings - retryMap := cmab.RetryConfig - if retryMap == nil { - t.Fatal("RetryConfig map is nil") - } - - // Max retries - maxRetriesValue := retryMap["maxRetries"] - fmt.Printf("maxRetries: %v (%T)\n", maxRetriesValue, maxRetriesValue) - maxRetries, ok := maxRetriesValue.(float64) - assert.True(t, ok, "maxRetries should be float64") - assert.Equal(t, float64(5), maxRetries) - - // Check other retry settings - fmt.Printf("initialBackoff: %v (%T)\n", retryMap["initialBackoff"], retryMap["initialBackoff"]) - assert.Equal(t, "200ms", retryMap["initialBackoff"]) - - fmt.Printf("maxBackoff: %v (%T)\n", retryMap["maxBackoff"], retryMap["maxBackoff"]) - assert.Equal(t, "30s", retryMap["maxBackoff"]) - - fmt.Printf("backoffMultiplier: %v (%T)\n", retryMap["backoffMultiplier"], retryMap["backoffMultiplier"]) - assert.Equal(t, 3.0, retryMap["backoffMultiplier"]) + // Check cache configuration + cache := cmab.Cache + assert.Equal(t, "redis", cache.Type) + assert.Equal(t, 2000, cache.Size) + assert.Equal(t, 45*time.Minute, cache.TTL) + + // Check retry configuration + retry := cmab.RetryConfig + assert.Equal(t, 5, retry.MaxRetries) + assert.Equal(t, 200*time.Millisecond, retry.InitialBackoff) + assert.Equal(t, 30*time.Second, retry.MaxBackoff) + assert.Equal(t, 3.0, retry.BackoffMultiplier) } func TestCMABEnvDebug(t *testing.T) { @@ -266,12 +209,7 @@ func TestCMABEnvDebug(t *testing.T) { "cache": { "type": "redis", "size": 2000, - "ttl": "45m", - "redis": { - "host": "redis.example.com:6379", - "password": "password123", - "database": 2 - } + "ttl": "45m" }, "retryConfig": { "maxRetries": 5, @@ -317,11 +255,11 @@ func TestCMABPartialConfig(t *testing.T) { conf := loadConfig(v) // Cache assertions - assert.Equal(t, "redis", conf.CMAB.Cache["type"]) - assert.Equal(t, float64(3000), conf.CMAB.Cache["size"]) + assert.Equal(t, "redis", conf.CMAB.Cache.Type) + assert.Equal(t, 3000, conf.CMAB.Cache.Size) // RetryConfig assertions - assert.Equal(t, float64(10), conf.CMAB.RetryConfig["maxRetries"]) + assert.Equal(t, 10, conf.CMAB.RetryConfig.MaxRetries) // Clean up os.Unsetenv("OPTIMIZELY_CMAB") @@ -548,12 +486,7 @@ func TestViperEnv(t *testing.T) { "cache": { "type": "redis", "size": 2000, - "ttl": "45m", - "redis": { - "host": "redis.example.com:6379", - "password": "password123", - "database": 2 - } + "ttl": "45m" }, "retryConfig": { "maxRetries": 5, @@ -690,7 +623,7 @@ func TestCMABComplexJSON(t *testing.T) { os.Unsetenv("OPTIMIZELY_CMAB_CACHE_REDIS_DATABASE") // Set complex JSON environment variable for CMAB cache - _ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type":"redis","size":5000,"ttl":"3h","redis":{"host":"json-redis.example.com:6379","password":"json-password","database":4}}`) + _ = os.Setenv("OPTIMIZELY_CMAB_CACHE", `{"type":"redis","size":5000,"ttl":"3h"}`) defer func() { // Clean up @@ -702,22 +635,8 @@ func TestCMABComplexJSON(t *testing.T) { actual := loadConfig(v) // Test cache settings from JSON environment variable - cacheMap := actual.CMAB.Cache - assert.Equal(t, "redis", cacheMap["type"]) - - // Account for JSON unmarshaling to float64 - size, ok := cacheMap["size"].(float64) - assert.True(t, ok, "Size should be a float64") - assert.Equal(t, float64(5000), size) - - assert.Equal(t, "3h", cacheMap["ttl"]) - - redisMap, ok := cacheMap["redis"].(map[string]interface{}) - assert.True(t, ok, "Redis config should be a map") - assert.Equal(t, "json-redis.example.com:6379", redisMap["host"]) - assert.Equal(t, "json-password", redisMap["password"]) - - db, ok := redisMap["database"].(float64) - assert.True(t, ok, "Database should be a float64") - assert.Equal(t, float64(4), db) + cache := actual.CMAB.Cache + assert.Equal(t, "redis", cache.Type) + assert.Equal(t, 5000, cache.Size) + assert.Equal(t, 3*time.Hour, cache.TTL) } diff --git a/config/config.go b/config/config.go index 9a90f08a..ed9eb646 100644 --- a/config/config.go +++ b/config/config.go @@ -142,16 +142,16 @@ func NewDefaultConfig() *AgentConfig { }, CMAB: CMABConfig{ RequestTimeout: 10 * time.Second, - Cache: map[string]interface{}{ - "type": "memory", - "size": 1000, - "ttl": "30m", + Cache: CMABCacheConfig{ + Type: "memory", + Size: 1000, + TTL: 30 * time.Minute, }, - RetryConfig: map[string]interface{}{ - "maxRetries": 3, - "initialBackoff": "100ms", - "maxBackoff": "10s", - "backoffMultiplier": 2.0, + RetryConfig: CMABRetryConfig{ + MaxRetries: 3, + InitialBackoff: 100 * time.Millisecond, + MaxBackoff: 10 * time.Second, + BackoffMultiplier: 2.0, }, }, } @@ -409,8 +409,30 @@ type CMABConfig struct { RequestTimeout time.Duration `json:"requestTimeout"` // Cache configuration - Cache map[string]interface{} `json:"cache"` + Cache CMABCacheConfig `json:"cache"` // RetryConfig for CMAB API requests - RetryConfig map[string]interface{} `json:"retryConfig"` + RetryConfig CMABRetryConfig `json:"retryConfig"` +} + +// CMABCacheConfig holds the CMAB cache configuration +type CMABCacheConfig struct { + // Type of cache (currently only "memory" is supported) + Type string `json:"type"` + // Size is the maximum number of entries for in-memory cache + Size int `json:"size"` + // TTL is the time-to-live for cached decisions + TTL time.Duration `json:"ttl"` +} + +// CMABRetryConfig holds the CMAB retry configuration +type CMABRetryConfig struct { + // MaxRetries is the maximum number of retry attempts + MaxRetries int `json:"maxRetries"` + // InitialBackoff is the initial backoff duration + InitialBackoff time.Duration `json:"initialBackoff"` + // MaxBackoff is the maximum backoff duration + MaxBackoff time.Duration `json:"maxBackoff"` + // BackoffMultiplier is the multiplier for exponential backoff + BackoffMultiplier float64 `json:"backoffMultiplier"` } diff --git a/config/config_test.go b/config/config_test.go index 1f23eed0..eb0df6fb 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -103,18 +103,18 @@ func TestDefaultConfig(t *testing.T) { // CMAB configuration assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) - // Test cache settings as maps - cacheMap := conf.CMAB.Cache - assert.Equal(t, "memory", cacheMap["type"]) - assert.Equal(t, 1000, cacheMap["size"]) - assert.Equal(t, "30m", cacheMap["ttl"]) - - // Test retry settings as maps - retryMap := conf.CMAB.RetryConfig - assert.Equal(t, 3, retryMap["maxRetries"]) - assert.Equal(t, "100ms", retryMap["initialBackoff"]) - assert.Equal(t, "10s", retryMap["maxBackoff"]) - assert.Equal(t, 2.0, retryMap["backoffMultiplier"]) + // Test cache settings + cache := conf.CMAB.Cache + assert.Equal(t, "memory", cache.Type) + assert.Equal(t, 1000, cache.Size) + assert.Equal(t, 30*time.Minute, cache.TTL) + + // Test retry settings + retry := conf.CMAB.RetryConfig + assert.Equal(t, 3, retry.MaxRetries) + assert.Equal(t, 100*time.Millisecond, retry.InitialBackoff) + assert.Equal(t, 10*time.Second, retry.MaxBackoff) + assert.Equal(t, 2.0, retry.BackoffMultiplier) } type logObservation struct { @@ -256,16 +256,16 @@ func TestDefaultCMABConfig(t *testing.T) { // Test default values assert.Equal(t, 10*time.Second, conf.CMAB.RequestTimeout) - // Test default cache settings as maps - cacheMap := conf.CMAB.Cache - assert.Equal(t, "memory", cacheMap["type"]) - assert.Equal(t, 1000, cacheMap["size"]) - assert.Equal(t, "30m", cacheMap["ttl"]) - - // Test default retry settings as maps - retryMap := conf.CMAB.RetryConfig - assert.Equal(t, 3, retryMap["maxRetries"]) - assert.Equal(t, "100ms", retryMap["initialBackoff"]) - assert.Equal(t, "10s", retryMap["maxBackoff"]) - assert.Equal(t, 2.0, retryMap["backoffMultiplier"]) + // Test default cache settings + cache := conf.CMAB.Cache + assert.Equal(t, "memory", cache.Type) + assert.Equal(t, 1000, cache.Size) + assert.Equal(t, 30*time.Minute, cache.TTL) + + // Test default retry settings + retry := conf.CMAB.RetryConfig + assert.Equal(t, 3, retry.MaxRetries) + assert.Equal(t, 100*time.Millisecond, retry.InitialBackoff) + assert.Equal(t, 10*time.Second, retry.MaxBackoff) + assert.Equal(t, 2.0, retry.BackoffMultiplier) } diff --git a/go.mod b/go.mod index 1df25d47..21e38504 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.2 github.com/google/uuid v1.3.1 github.com/lestrrat-go/jwx/v2 v2.0.20 - github.com/optimizely/go-sdk/v2 v2.1.1-0.20250728200455-37780395c725 + github.com/optimizely/go-sdk/v2 v2.1.1-0.20250730195201-16820334c705 github.com/orcaman/concurrent-map v1.0.0 github.com/prometheus/client_golang v1.18.0 github.com/rakyll/statik v0.1.7 diff --git a/go.sum b/go.sum index 41ea334c..a7cfd4af 100644 --- a/go.sum +++ b/go.sum @@ -229,10 +229,8 @@ github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE= github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs= -github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725211427-2baee7ec755d h1:50UbypBJW9larBSnkaePbLe5Kkm6cg1/hAZDpvkRvVg= -github.com/optimizely/go-sdk/v2 v2.1.1-0.20250725211427-2baee7ec755d/go.mod h1:MusRCFsU7+XzJCoCTgheLoENJSf1iiFYm4KbJqz6BYA= -github.com/optimizely/go-sdk/v2 v2.1.1-0.20250728200455-37780395c725 h1:CsafFEQ3xBhKiMyKho47oCyHYuWy9c8w20D5Oc2b4dw= -github.com/optimizely/go-sdk/v2 v2.1.1-0.20250728200455-37780395c725/go.mod h1:MusRCFsU7+XzJCoCTgheLoENJSf1iiFYm4KbJqz6BYA= +github.com/optimizely/go-sdk/v2 v2.1.1-0.20250730195201-16820334c705 h1:W9Y7TonTlWOy68rDSu2ci5G7zsAuXUPdq69S96q6Cew= +github.com/optimizely/go-sdk/v2 v2.1.1-0.20250730195201-16820334c705/go.mod h1:MusRCFsU7+XzJCoCTgheLoENJSf1iiFYm4KbJqz6BYA= github.com/orcaman/concurrent-map v1.0.0 h1:I/2A2XPCb4IuQWcQhBhSwGfiuybl/J0ev9HDbW65HOY= github.com/orcaman/concurrent-map v1.0.0/go.mod h1:Lu3tH6HLW3feq74c2GC+jIMS/K2CFcDWnWD9XkenwhI= github.com/pelletier/go-toml/v2 v2.0.6 h1:nrzqCb7j9cDFj2coyLNLaZuJTLjWjlaz6nvTvIwycIU= diff --git a/pkg/optimizely/cache.go b/pkg/optimizely/cache.go index c82f6cd2..711b53da 100644 --- a/pkg/optimizely/cache.go +++ b/pkg/optimizely/cache.go @@ -25,7 +25,6 @@ import ( "regexp" "strings" "sync" - "time" cmap "github.com/orcaman/concurrent-map" "github.com/rs/zerolog/log" @@ -324,45 +323,36 @@ func defaultLoader( } // Parse CMAB cache configuration - cacheSize := cmab.DefaultCacheSize - cacheTTL := cmab.DefaultCacheTTL - - if cacheConfig, ok := clientConf.CMAB.Cache["size"].(int); ok { - cacheSize = cacheConfig + cacheSize := clientConf.CMAB.Cache.Size + if cacheSize == 0 { + cacheSize = cmab.DefaultCacheSize } - if cacheTTLStr, ok := clientConf.CMAB.Cache["ttl"].(string); ok { - if parsedTTL, err := time.ParseDuration(cacheTTLStr); err == nil { - cacheTTL = parsedTTL - } else { - log.Warn().Err(err).Msgf("Failed to parse CMAB cache TTL: %s, using default", cacheTTLStr) - } + cacheTTL := clientConf.CMAB.Cache.TTL + if cacheTTL == 0 { + cacheTTL = cmab.DefaultCacheTTL } // Create retry config retryConfig := &cmab.RetryConfig{ - MaxRetries: cmab.DefaultMaxRetries, - InitialBackoff: cmab.DefaultInitialBackoff, - MaxBackoff: cmab.DefaultMaxBackoff, - BackoffMultiplier: cmab.DefaultBackoffMultiplier, + MaxRetries: clientConf.CMAB.RetryConfig.MaxRetries, + InitialBackoff: clientConf.CMAB.RetryConfig.InitialBackoff, + MaxBackoff: clientConf.CMAB.RetryConfig.MaxBackoff, + BackoffMultiplier: clientConf.CMAB.RetryConfig.BackoffMultiplier, } - // Parse retry configuration - if maxRetries, ok := clientConf.CMAB.RetryConfig["maxRetries"].(int); ok { - retryConfig.MaxRetries = maxRetries + // Apply defaults for retry config if not set + if retryConfig.MaxRetries == 0 { + retryConfig.MaxRetries = cmab.DefaultMaxRetries } - if initialBackoffStr, ok := clientConf.CMAB.RetryConfig["initialBackoff"].(string); ok { - if backoff, err := time.ParseDuration(initialBackoffStr); err == nil { - retryConfig.InitialBackoff = backoff - } + if retryConfig.InitialBackoff == 0 { + retryConfig.InitialBackoff = cmab.DefaultInitialBackoff } - if maxBackoffStr, ok := clientConf.CMAB.RetryConfig["maxBackoff"].(string); ok { - if backoff, err := time.ParseDuration(maxBackoffStr); err == nil { - retryConfig.MaxBackoff = backoff - } + if retryConfig.MaxBackoff == 0 { + retryConfig.MaxBackoff = cmab.DefaultMaxBackoff } - if multiplier, ok := clientConf.CMAB.RetryConfig["backoffMultiplier"].(float64); ok { - retryConfig.BackoffMultiplier = multiplier + if retryConfig.BackoffMultiplier == 0 { + retryConfig.BackoffMultiplier = cmab.DefaultBackoffMultiplier } // Create CMAB config (NO endpoint configuration - not configurable) diff --git a/pkg/optimizely/cache_test.go b/pkg/optimizely/cache_test.go index 820da655..23292b4d 100644 --- a/pkg/optimizely/cache_test.go +++ b/pkg/optimizely/cache_test.go @@ -827,15 +827,16 @@ func (s *DefaultLoaderTestSuite) TestCMABConfigurationParsing() { SdkKeyRegex: "sdkkey", CMAB: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: map[string]interface{}{ - "size": 500, - "ttl": "15m", + Cache: config.CMABCacheConfig{ + Type: "memory", + Size: 500, + TTL: 15 * time.Minute, }, - RetryConfig: map[string]interface{}{ - "maxRetries": 5, - "initialBackoff": "200ms", - "maxBackoff": "30s", - "backoffMultiplier": 1.5, + RetryConfig: config.CMABRetryConfig{ + MaxRetries: 5, + InitialBackoff: 200 * time.Millisecond, + MaxBackoff: 30 * time.Second, + BackoffMultiplier: 1.5, }, }, } @@ -855,8 +856,8 @@ func (s *DefaultLoaderTestSuite) TestCMABConfigurationDefaults() { CMAB: config.CMABConfig{ RequestTimeout: 5 * time.Second, // Empty cache and retry config should use defaults - Cache: map[string]interface{}{}, - RetryConfig: map[string]interface{}{}, + Cache: config.CMABCacheConfig{}, + RetryConfig: config.CMABRetryConfig{}, }, } @@ -872,11 +873,12 @@ func (s *DefaultLoaderTestSuite) TestCMABCacheConfigInvalidTTL() { SdkKeyRegex: "sdkkey", CMAB: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: map[string]interface{}{ - "size": 1000, - "ttl": "invalid-duration", // This should trigger warning and use default + // Test with valid values since structured types prevent invalid input + Cache: config.CMABCacheConfig{ + Size: 1000, + TTL: 10 * time.Minute, }, - RetryConfig: map[string]interface{}{}, + RetryConfig: config.CMABRetryConfig{}, }, } @@ -887,18 +889,21 @@ func (s *DefaultLoaderTestSuite) TestCMABCacheConfigInvalidTTL() { s.NotNil(client) } -func (s *DefaultLoaderTestSuite) TestCMABCacheConfigWrongTypes() { +func (s *DefaultLoaderTestSuite) TestCMABCacheConfigWithValidStructuredTypes() { conf := config.ClientConfig{ SdkKeyRegex: "sdkkey", CMAB: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: map[string]interface{}{ - "size": "not-an-int", // Wrong type, should use default - "ttl": 123, // Wrong type, should use default + Cache: config.CMABCacheConfig{ + Type: "memory", + Size: 1000, + TTL: 15 * time.Minute, }, - RetryConfig: map[string]interface{}{ - "maxRetries": "not-an-int", // Wrong type, should use default - "backoffMultiplier": "not-a-float", // Wrong type, should use default + RetryConfig: config.CMABRetryConfig{ + MaxRetries: 3, + InitialBackoff: 100 * time.Millisecond, + MaxBackoff: 10 * time.Second, + BackoffMultiplier: 2.0, }, }, } @@ -906,21 +911,21 @@ func (s *DefaultLoaderTestSuite) TestCMABCacheConfigWrongTypes() { loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) client, err := loader("sdkkey") - s.NoError(err) // Should not error, just use defaults + s.NoError(err) s.NotNil(client) } -func (s *DefaultLoaderTestSuite) TestCMABRetryConfigInvalidDurations() { +func (s *DefaultLoaderTestSuite) TestCMABRetryConfigWithValidDurations() { conf := config.ClientConfig{ SdkKeyRegex: "sdkkey", CMAB: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: map[string]interface{}{}, - RetryConfig: map[string]interface{}{ - "maxRetries": 3, - "initialBackoff": "invalid-duration", - "maxBackoff": "also-invalid", - "backoffMultiplier": 2.0, + Cache: config.CMABCacheConfig{}, + RetryConfig: config.CMABRetryConfig{ + MaxRetries: 3, + InitialBackoff: 200 * time.Millisecond, + MaxBackoff: 30 * time.Second, + BackoffMultiplier: 2.0, }, }, } @@ -928,7 +933,7 @@ func (s *DefaultLoaderTestSuite) TestCMABRetryConfigInvalidDurations() { loader := defaultLoader(config.AgentConfig{Client: conf}, s.registry, nil, s.upsMap, s.odpCacheMap, s.pcFactory, s.bpFactory) client, err := loader("sdkkey") - s.NoError(err) // Should not error, just use defaults for invalid durations + s.NoError(err) s.NotNil(client) } @@ -937,15 +942,16 @@ func (s *DefaultLoaderTestSuite) TestCMABConfigurationAllValidValues() { SdkKeyRegex: "sdkkey", CMAB: config.CMABConfig{ RequestTimeout: 10 * time.Second, - Cache: map[string]interface{}{ - "size": 2000, - "ttl": "45m", + Cache: config.CMABCacheConfig{ + Type: "memory", + Size: 2000, + TTL: 45 * time.Minute, }, - RetryConfig: map[string]interface{}{ - "maxRetries": 10, - "initialBackoff": "500ms", - "maxBackoff": "1m", - "backoffMultiplier": 3.0, + RetryConfig: config.CMABRetryConfig{ + MaxRetries: 10, + InitialBackoff: 500 * time.Millisecond, + MaxBackoff: 1 * time.Minute, + BackoffMultiplier: 3.0, }, }, } @@ -962,8 +968,8 @@ func (s *DefaultLoaderTestSuite) TestCMABWithZeroRequestTimeout() { SdkKeyRegex: "sdkkey", CMAB: config.CMABConfig{ RequestTimeout: 0, // Zero timeout - Cache: map[string]interface{}{}, - RetryConfig: map[string]interface{}{}, + Cache: config.CMABCacheConfig{}, + RetryConfig: config.CMABRetryConfig{}, }, } @@ -983,20 +989,20 @@ func (s *DefaultLoaderTestSuite) TestCMABConfigurationEdgeCases() { name: "Zero cache size", config: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: map[string]interface{}{ - "size": 0, - "ttl": "30m", + Cache: config.CMABCacheConfig{ + Size: 0, + TTL: 30 * time.Minute, }, - RetryConfig: map[string]interface{}{}, + RetryConfig: config.CMABRetryConfig{}, }, }, { name: "Zero max retries", config: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: map[string]interface{}{}, - RetryConfig: map[string]interface{}{ - "maxRetries": 0, + Cache: config.CMABCacheConfig{}, + RetryConfig: config.CMABRetryConfig{ + MaxRetries: 0, }, }, }, @@ -1004,20 +1010,20 @@ func (s *DefaultLoaderTestSuite) TestCMABConfigurationEdgeCases() { name: "Very short TTL", config: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: map[string]interface{}{ - "ttl": "1ms", + Cache: config.CMABCacheConfig{ + TTL: 1 * time.Millisecond, }, - RetryConfig: map[string]interface{}{}, + RetryConfig: config.CMABRetryConfig{}, }, }, { name: "Very long TTL", config: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: map[string]interface{}{ - "ttl": "24h", + Cache: config.CMABCacheConfig{ + TTL: 24 * time.Hour, }, - RetryConfig: map[string]interface{}{}, + RetryConfig: config.CMABRetryConfig{}, }, }, } @@ -1038,13 +1044,13 @@ func (s *DefaultLoaderTestSuite) TestCMABConfigurationEdgeCases() { } } -func (s *DefaultLoaderTestSuite) TestCMABConfigurationNilMaps() { +func (s *DefaultLoaderTestSuite) TestCMABConfigurationEmptyStructs() { conf := config.ClientConfig{ SdkKeyRegex: "sdkkey", CMAB: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: nil, // nil map - RetryConfig: nil, // nil map + Cache: config.CMABCacheConfig{}, // empty struct + RetryConfig: config.CMABRetryConfig{}, // empty struct }, } @@ -1081,12 +1087,13 @@ func (s *DefaultLoaderTestSuite) TestCMABWithExistingServices() { }, CMAB: config.CMABConfig{ RequestTimeout: 5 * time.Second, - Cache: map[string]interface{}{ - "size": 1000, - "ttl": "30m", + Cache: config.CMABCacheConfig{ + Type: "memory", + Size: 1000, + TTL: 30 * time.Minute, }, - RetryConfig: map[string]interface{}{ - "maxRetries": 5, + RetryConfig: config.CMABRetryConfig{ + MaxRetries: 5, }, }, }