Skip to content

Commit

Permalink
update cohort_sync_config fields: include polling and remove request …
Browse files Browse the repository at this point in the history
…delay, use enum for serverzone, update tests accordingly
  • Loading branch information
tyiuhc committed Aug 6, 2024
1 parent 28c1793 commit 82cc98a
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 63 deletions.
2 changes: 1 addition & 1 deletion pkg/experiment/local/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func Initialize(apiKey string, config *Config) *Client {
var cohortLoader *cohortLoader
var deploymentRunner *deploymentRunner
if config.CohortSyncConfig != nil {
cohortDownloadApi := newDirectCohortDownloadApi(config.CohortSyncConfig.ApiKey, config.CohortSyncConfig.SecretKey, config.CohortSyncConfig.MaxCohortSize, config.CohortSyncConfig.CohortRequestDelayMillis, config.CohortSyncConfig.CohortServerUrl, config.Debug)
cohortDownloadApi := newDirectCohortDownloadApi(config.CohortSyncConfig.ApiKey, config.CohortSyncConfig.SecretKey, config.CohortSyncConfig.MaxCohortSize, config.CohortSyncConfig.CohortServerUrl, config.Debug)
cohortLoader = newCohortLoader(cohortDownloadApi, cohortStorage)
}
deploymentRunner = newDeploymentRunner(config, newFlagConfigApiV2(apiKey, config.ServerUrl, config.FlagConfigPollerRequestTimeout), flagConfigStorage, cohortStorage, cohortLoader)
Expand Down
2 changes: 1 addition & 1 deletion pkg/experiment/local/client_eu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func init() {
SecretKey: secretKey,
}
clientEU = Initialize("server-Qlp7XiSu6JtP2S3JzA95PnP27duZgQCF",
&Config{CohortSyncConfig: &cohortSyncConfig, ServerZone: "eu"})
&Config{CohortSyncConfig: &cohortSyncConfig, ServerZone: EUServerZone})
err = clientEU.Start()
if err != nil {
panic(err)
Expand Down
32 changes: 16 additions & 16 deletions pkg/experiment/local/cohort_download_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,29 @@ import (
"time"
)

const cohortRequestDelay = 100 * time.Millisecond

type cohortDownloadApi interface {
getCohort(cohortID string, cohort *Cohort) (*Cohort, error)
}

type directCohortDownloadApi struct {
ApiKey string
SecretKey string
MaxCohortSize int
CohortRequestDelayMillis int
ServerUrl string
Debug bool
log *logger.Log
ApiKey string
SecretKey string
MaxCohortSize int
ServerUrl string
Debug bool
log *logger.Log
}

func newDirectCohortDownloadApi(apiKey, secretKey string, maxCohortSize, cohortRequestDelayMillis int, serverUrl string, debug bool) *directCohortDownloadApi {
func newDirectCohortDownloadApi(apiKey, secretKey string, maxCohortSize int, serverUrl string, debug bool) *directCohortDownloadApi {
api := &directCohortDownloadApi{
ApiKey: apiKey,
SecretKey: secretKey,
MaxCohortSize: maxCohortSize,
CohortRequestDelayMillis: cohortRequestDelayMillis,
ServerUrl: serverUrl,
Debug: debug,
log: logger.New(debug),
ApiKey: apiKey,
SecretKey: secretKey,
MaxCohortSize: maxCohortSize,
ServerUrl: serverUrl,
Debug: debug,
log: logger.New(debug),
}
return api
}
Expand All @@ -56,7 +56,7 @@ func (api *directCohortDownloadApi) getCohort(cohortID string, cohort *Cohort) (
}(err) {
return nil, err
}
time.Sleep(time.Duration(api.CohortRequestDelayMillis) * time.Millisecond)
time.Sleep(cohortRequestDelay)
continue
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/experiment/local/cohort_download_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestCohortDownloadApi(t *testing.T) {
httpmock.Activate()
defer httpmock.DeactivateAndReset()

api := newDirectCohortDownloadApi("api", "secret", 15000, 100, "https://server.amplitude.com", false)
api := newDirectCohortDownloadApi("api", "secret", 15000, "https://server.amplitude.com", false)

t.Run("test_cohort_download_success", func(t *testing.T) {
cohort := &Cohort{Id: "1234", LastModified: 0, Size: 1, MemberIds: []string{"user"}, GroupType: "userGroupType"}
Expand Down
44 changes: 26 additions & 18 deletions pkg/experiment/local/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,23 @@ package local
import (
"github.com/amplitude/analytics-go/amplitude"
"math"
"strings"
"time"
)

const EUFlagServerUrl = "https://flag.lab.eu.amplitude.com"
const EUCohortSyncUrl = "https://cohort-v2.lab.eu.amplitude.com"

type ServerZone int

const (
USServerZone ServerZone = iota
EUServerZone
)

type Config struct {
Debug bool
ServerUrl string
ServerZone string
ServerZone ServerZone
FlagConfigPollerInterval time.Duration
FlagConfigPollerRequestTimeout time.Duration
AssignmentConfig *AssignmentConfig
Expand All @@ -26,17 +32,17 @@ type AssignmentConfig struct {
}

type CohortSyncConfig struct {
ApiKey string
SecretKey string
MaxCohortSize int
CohortRequestDelayMillis int
CohortServerUrl string
ApiKey string
SecretKey string
MaxCohortSize int
CohortPollingInterval time.Duration
CohortServerUrl string
}

var DefaultConfig = &Config{
Debug: false,
ServerUrl: "https://api.lab.amplitude.com/",
ServerZone: "us",
ServerZone: USServerZone,
FlagConfigPollerInterval: 30 * time.Second,
FlagConfigPollerRequestTimeout: 10 * time.Second,
}
Expand All @@ -46,22 +52,23 @@ var DefaultAssignmentConfig = &AssignmentConfig{
}

var DefaultCohortSyncConfig = &CohortSyncConfig{
MaxCohortSize: math.MaxInt32,
CohortRequestDelayMillis: 5000,
CohortServerUrl: "https://cohort-v2.lab.amplitude.com",
MaxCohortSize: math.MaxInt32,
CohortPollingInterval: 60 * time.Second,
CohortServerUrl: "https://cohort-v2.lab.amplitude.com",
}

func fillConfigDefaults(c *Config) *Config {
if c == nil {
return DefaultConfig
}
if c.ServerZone == "" {
if c.ServerZone == 0 {
c.ServerZone = DefaultConfig.ServerZone
}
if c.ServerUrl == "" {
if strings.EqualFold(strings.ToLower(c.ServerZone), strings.ToLower(DefaultConfig.ServerZone)) {
switch c.ServerZone {
case USServerZone:
c.ServerUrl = DefaultConfig.ServerUrl
} else if strings.EqualFold(strings.ToLower(c.ServerZone), "eu") {
case EUServerZone:
c.ServerUrl = EUFlagServerUrl
}
}
Expand All @@ -80,14 +87,15 @@ func fillConfigDefaults(c *Config) *Config {
c.CohortSyncConfig.MaxCohortSize = DefaultCohortSyncConfig.MaxCohortSize
}

if c.CohortSyncConfig != nil && c.CohortSyncConfig.CohortRequestDelayMillis == 0 {
c.CohortSyncConfig.CohortRequestDelayMillis = DefaultCohortSyncConfig.CohortRequestDelayMillis
if c.CohortSyncConfig != nil && (c.CohortSyncConfig.CohortPollingInterval < DefaultCohortSyncConfig.CohortPollingInterval) {
c.CohortSyncConfig.CohortPollingInterval = DefaultCohortSyncConfig.CohortPollingInterval
}

if c.CohortSyncConfig != nil && c.CohortSyncConfig.CohortServerUrl == "" {
if strings.EqualFold(strings.ToLower(c.ServerZone), strings.ToLower(DefaultConfig.ServerZone)) {
switch c.ServerZone {
case USServerZone:
c.CohortSyncConfig.CohortServerUrl = DefaultCohortSyncConfig.CohortServerUrl
} else if strings.EqualFold(strings.ToLower(c.ServerZone), "eu") {
case EUServerZone:
c.CohortSyncConfig.CohortServerUrl = EUCohortSyncUrl
}
}
Expand Down
35 changes: 17 additions & 18 deletions pkg/experiment/local/config_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package local

import (
"strings"
"testing"
"time"
)
Expand All @@ -10,7 +9,7 @@ func TestFillConfigDefaults_ServerZoneAndServerUrl(t *testing.T) {
tests := []struct {
name string
input *Config
expectedZone string
expectedZone ServerZone
expectedUrl string
}{
{
Expand All @@ -27,35 +26,35 @@ func TestFillConfigDefaults_ServerZoneAndServerUrl(t *testing.T) {
},
{
name: "ServerZone US",
input: &Config{ServerZone: "us"},
expectedZone: "us",
input: &Config{ServerZone: USServerZone},
expectedZone: USServerZone,
expectedUrl: DefaultConfig.ServerUrl,
},
{
name: "ServerZone EU",
input: &Config{ServerZone: "eu"},
expectedZone: "eu",
input: &Config{ServerZone: EUServerZone},
expectedZone: EUServerZone,
expectedUrl: EUFlagServerUrl,
},
{
name: "Uppercase ServerZone EU",
input: &Config{ServerZone: "EU"},
expectedZone: "EU",
input: &Config{ServerZone: EUServerZone},
expectedZone: EUServerZone,
expectedUrl: EUFlagServerUrl,
},
{
name: "Custom ServerUrl",
input: &Config{ServerZone: "us", ServerUrl: "https://custom.url/"},
expectedZone: "us",
input: &Config{ServerZone: USServerZone, ServerUrl: "https://custom.url/"},
expectedZone: USServerZone,
expectedUrl: "https://custom.url/",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := fillConfigDefaults(tt.input)
if !strings.EqualFold(result.ServerZone, tt.expectedZone) {
t.Errorf("expected ServerZone %s, got %s", tt.expectedZone, result.ServerZone)
if result.ServerZone != tt.expectedZone {
t.Errorf("expected ServerZone %d, got %d", tt.expectedZone, result.ServerZone)
}
if result.ServerUrl != tt.expectedUrl {
t.Errorf("expected ServerUrl %s, got %s", tt.expectedUrl, result.ServerUrl)
Expand All @@ -73,23 +72,23 @@ func TestFillConfigDefaults_CohortSyncConfig(t *testing.T) {
{
name: "Nil CohortSyncConfig",
input: &Config{
ServerZone: "eu",
ServerZone: EUServerZone,
CohortSyncConfig: nil,
},
expectedUrl: "",
},
{
name: "CohortSyncConfig with empty CohortServerUrl",
input: &Config{
ServerZone: "eu",
ServerZone: EUServerZone,
CohortSyncConfig: &CohortSyncConfig{},
},
expectedUrl: EUCohortSyncUrl,
},
{
name: "CohortSyncConfig with custom CohortServerUrl",
input: &Config{
ServerZone: "us",
ServerZone: USServerZone,
CohortSyncConfig: &CohortSyncConfig{
CohortServerUrl: "https://custom-cohort.url/",
},
Expand Down Expand Up @@ -141,13 +140,13 @@ func TestFillConfigDefaults_DefaultValues(t *testing.T) {
{
name: "Custom values",
input: &Config{
ServerZone: "eu",
ServerZone: EUServerZone,
ServerUrl: "https://custom.url/",
FlagConfigPollerInterval: 60 * time.Second,
FlagConfigPollerRequestTimeout: 20 * time.Second,
},
expected: &Config{
ServerZone: "eu",
ServerZone: EUServerZone,
ServerUrl: "https://custom.url/",
FlagConfigPollerInterval: 60 * time.Second,
FlagConfigPollerRequestTimeout: 20 * time.Second,
Expand All @@ -159,7 +158,7 @@ func TestFillConfigDefaults_DefaultValues(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
result := fillConfigDefaults(tt.input)
if result.ServerZone != tt.expected.ServerZone {
t.Errorf("expected ServerZone %s, got %s", tt.expected.ServerZone, result.ServerZone)
t.Errorf("expected ServerZone %d, got %d", tt.expected.ServerZone, result.ServerZone)
}
if result.ServerUrl != tt.expected.ServerUrl {
t.Errorf("expected ServerUrl %s, got %s", tt.expected.ServerUrl, result.ServerUrl)
Expand Down
12 changes: 4 additions & 8 deletions pkg/experiment/local/deployment_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,12 @@ package local

import (
"fmt"
"strings"
"sync"
"time"

"github.com/amplitude/experiment-go-server/internal/evaluation"
"github.com/amplitude/experiment-go-server/internal/logger"
"strings"
"sync"
)

const CohortPollerInterval = 60 * time.Second

type deploymentRunner struct {
config *Config
flagConfigApi flagConfigApi
Expand Down Expand Up @@ -57,8 +53,8 @@ func (dr *deploymentRunner) start() error {
}
})

if dr.cohortLoader != nil {
dr.poller.Poll(CohortPollerInterval, func() {
if dr.config.CohortSyncConfig != nil {
dr.poller.Poll(dr.config.CohortSyncConfig.CohortPollingInterval, func() {
dr.updateStoredCohorts()
})
}
Expand Down

0 comments on commit 82cc98a

Please sign in to comment.