From af077c074aa27bb92b980f9e5d6515748bc00f39 Mon Sep 17 00:00:00 2001 From: Hugo Biarge Date: Fri, 9 Aug 2024 11:18:29 +0200 Subject: [PATCH 1/4] chore: Allow providing an externally created http client for the remote client. --- pkg/experiment/remote/client.go | 9 ++++++++- pkg/experiment/remote/config.go | 7 ++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/experiment/remote/client.go b/pkg/experiment/remote/client.go index 456a70b..8ca429a 100644 --- a/pkg/experiment/remote/client.go +++ b/pkg/experiment/remote/client.go @@ -39,9 +39,16 @@ func Initialize(apiKey string, config *Config) *Client { log: logger.New(config.Debug), apiKey: apiKey, config: config, - client: &http.Client{}, } + + if config.HttpClient != nil { + client.client = config.HttpClient + } else { + client.client = &http.Client{} + } + client.log.Debug("config: %v", *config) + clients[apiKey] = client } initMutex.Unlock() return client diff --git a/pkg/experiment/remote/config.go b/pkg/experiment/remote/config.go index aedd236..9a7075c 100644 --- a/pkg/experiment/remote/config.go +++ b/pkg/experiment/remote/config.go @@ -1,12 +1,16 @@ package remote -import "time" +import ( + "net/http" + "time" +) type Config struct { Debug bool ServerUrl string FetchTimeout time.Duration RetryBackoff *RetryBackoff + HttpClient *http.Client } var DefaultConfig = &Config{ @@ -14,6 +18,7 @@ var DefaultConfig = &Config{ ServerUrl: "https://api.lab.amplitude.com/", FetchTimeout: 500 * time.Millisecond, RetryBackoff: DefaultRetryBackoff, + HttpClient: nil, } type RetryBackoff struct { From 5eac03db6781d8a5b5ace066ade78e127296b92f Mon Sep 17 00:00:00 2001 From: Hugo Biarge Date: Fri, 9 Aug 2024 18:15:10 +0200 Subject: [PATCH 2/4] Pass the current context to Fetch and FetchV2 --- pkg/experiment/remote/client.go | 26 ++++++++++---------------- pkg/experiment/remote/client_test.go | 12 ++++++++---- pkg/experiment/remote/config.go | 3 --- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/pkg/experiment/remote/client.go b/pkg/experiment/remote/client.go index 8ca429a..389ef9e 100644 --- a/pkg/experiment/remote/client.go +++ b/pkg/experiment/remote/client.go @@ -39,14 +39,8 @@ func Initialize(apiKey string, config *Config) *Client { log: logger.New(config.Debug), apiKey: apiKey, config: config, + client: &http.Client{}, } - - if config.HttpClient != nil { - client.client = config.HttpClient - } else { - client.client = &http.Client{} - } - client.log.Debug("config: %v", *config) clients[apiKey] = client } @@ -55,8 +49,8 @@ func Initialize(apiKey string, config *Config) *Client { } // Deprecated: Use FetchV2 -func (c *Client) Fetch(user *experiment.User) (map[string]experiment.Variant, error) { - variants, err := c.FetchV2(user) +func (c *Client) Fetch(ctx context.Context, user *experiment.User) (map[string]experiment.Variant, error) { + variants, err := c.FetchV2(ctx, user) if err != nil { return nil, err } @@ -66,12 +60,12 @@ func (c *Client) Fetch(user *experiment.User) (map[string]experiment.Variant, er // FetchV2 fetches variants for a user from the remote evaluation service. // Unlike Fetch, this method returns all variants, including default variants. -func (c *Client) FetchV2(user *experiment.User) (map[string]experiment.Variant, error) { - variants, err := c.doFetch(user, c.config.FetchTimeout) +func (c *Client) FetchV2(ctx context.Context, user *experiment.User) (map[string]experiment.Variant, error) { + variants, err := c.doFetch(ctx, user, c.config.FetchTimeout) if err != nil { c.log.Error("fetch error: %v", err) if c.config.RetryBackoff.FetchRetries > 0 && shouldRetryFetch(err) { - return c.retryFetch(user) + return c.retryFetch(ctx, user) } else { return nil, err } @@ -79,7 +73,7 @@ func (c *Client) FetchV2(user *experiment.User) (map[string]experiment.Variant, return variants, err } -func (c *Client) doFetch(user *experiment.User, timeout time.Duration) (map[string]experiment.Variant, error) { +func (c *Client) doFetch(ctx context.Context, user *experiment.User, timeout time.Duration) (map[string]experiment.Variant, error) { addLibraryContext(user) endpoint, err := url.Parse(c.config.ServerUrl) if err != nil { @@ -94,7 +88,7 @@ func (c *Client) doFetch(user *experiment.User, timeout time.Duration) (map[stri return nil, err } c.log.Debug("fetch variants for user %s", string(jsonBytes)) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() req, err := http.NewRequestWithContext(ctx, "GET", endpoint.String(), nil) if err != nil { @@ -116,7 +110,7 @@ func (c *Client) doFetch(user *experiment.User, timeout time.Duration) (map[stri return c.parseResponse(resp) } -func (c *Client) retryFetch(user *experiment.User) (map[string]experiment.Variant, error) { +func (c *Client) retryFetch(ctx context.Context, user *experiment.User) (map[string]experiment.Variant, error) { var err error var variants map[string]experiment.Variant var timer *time.Timer @@ -125,7 +119,7 @@ func (c *Client) retryFetch(user *experiment.User) (map[string]experiment.Varian c.log.Debug("retry attempt %v", i) timer = time.NewTimer(delay) <-timer.C - variants, err = c.doFetch(user, c.config.RetryBackoff.FetchRetryTimeout) + variants, err = c.doFetch(ctx, user, c.config.RetryBackoff.FetchRetryTimeout) if err == nil && variants != nil { c.log.Debug("retry attempt %v success", i) return variants, nil diff --git a/pkg/experiment/remote/client_test.go b/pkg/experiment/remote/client_test.go index 59d9d45..da66a2d 100644 --- a/pkg/experiment/remote/client_test.go +++ b/pkg/experiment/remote/client_test.go @@ -1,6 +1,7 @@ package remote import ( + "context" "fmt" "net/http" "net/http/httptest" @@ -14,19 +15,20 @@ import ( func TestClient_Fetch_DoesNotReturnDefaultVariants(t *testing.T) { client := Initialize("server-qz35UwzJ5akieoAdIgzM4m9MIiOLXLoz", nil) + ctx := context.Background() user := &experiment.User{} - result, err := client.Fetch(user) + result, err := client.Fetch(ctx, user) require.NoError(t, err) require.NotNil(t, result) variant := result["sdk-ci-test"] require.Empty(t, variant) } - func TestClient_FetchV2_ReturnsDefaultVariants(t *testing.T) { client := Initialize("server-qz35UwzJ5akieoAdIgzM4m9MIiOLXLoz", nil) + ctx := context.Background() user := &experiment.User{} - result, err := client.FetchV2(user) + result, err := client.FetchV2(ctx, user) require.NoError(t, err) require.NotNil(t, result) variant := result["sdk-ci-test"] @@ -48,6 +50,8 @@ func TestClient_FetchRetryWithDifferentResponseCodes(t *testing.T) { {0, "Other Exception", 2}, } + ctx := context.Background() + for _, data := range testData { // Mock client initialization with httptest config := &Config{ @@ -92,7 +96,7 @@ func TestClient_FetchRetryWithDifferentResponseCodes(t *testing.T) { fmt.Printf("%d %s\n", data.responseCode, data.errorMessage) // Perform the fetch and catch the exception - _, err := client.Fetch(&experiment.User{UserId: "test_user"}) + _, err := client.Fetch(ctx, &experiment.User{UserId: "test_user"}) if err != nil { fmt.Println(err.Error()) } diff --git a/pkg/experiment/remote/config.go b/pkg/experiment/remote/config.go index 9a7075c..4f9349c 100644 --- a/pkg/experiment/remote/config.go +++ b/pkg/experiment/remote/config.go @@ -1,7 +1,6 @@ package remote import ( - "net/http" "time" ) @@ -10,7 +9,6 @@ type Config struct { ServerUrl string FetchTimeout time.Duration RetryBackoff *RetryBackoff - HttpClient *http.Client } var DefaultConfig = &Config{ @@ -18,7 +16,6 @@ var DefaultConfig = &Config{ ServerUrl: "https://api.lab.amplitude.com/", FetchTimeout: 500 * time.Millisecond, RetryBackoff: DefaultRetryBackoff, - HttpClient: nil, } type RetryBackoff struct { From a3c5462196e1651a32917410f3e8beb93124898d Mon Sep 17 00:00:00 2001 From: Hugo Biarge Date: Tue, 13 Aug 2024 18:58:20 +0200 Subject: [PATCH 3/4] Create a new method instead of generate breaking changes to current ones. --- pkg/experiment/remote/client.go | 12 +++++++++--- pkg/experiment/remote/client_test.go | 11 +++-------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/pkg/experiment/remote/client.go b/pkg/experiment/remote/client.go index 389ef9e..c06d36e 100644 --- a/pkg/experiment/remote/client.go +++ b/pkg/experiment/remote/client.go @@ -49,8 +49,8 @@ func Initialize(apiKey string, config *Config) *Client { } // Deprecated: Use FetchV2 -func (c *Client) Fetch(ctx context.Context, user *experiment.User) (map[string]experiment.Variant, error) { - variants, err := c.FetchV2(ctx, user) +func (c *Client) Fetch(user *experiment.User) (map[string]experiment.Variant, error) { + variants, err := c.FetchV2(user) if err != nil { return nil, err } @@ -60,7 +60,13 @@ func (c *Client) Fetch(ctx context.Context, user *experiment.User) (map[string]e // FetchV2 fetches variants for a user from the remote evaluation service. // Unlike Fetch, this method returns all variants, including default variants. -func (c *Client) FetchV2(ctx context.Context, user *experiment.User) (map[string]experiment.Variant, error) { +func (c *Client) FetchV2(user *experiment.User) (map[string]experiment.Variant, error) { + ctx := context.Background() + return c.FetchV2WithContext(ctx, user) +} + +// FetchV2WithContext fetches variants for a user from the remote evaluation service with a context. +func (c *Client) FetchV2WithContext(ctx context.Context, user *experiment.User) (map[string]experiment.Variant, error) { variants, err := c.doFetch(ctx, user, c.config.FetchTimeout) if err != nil { c.log.Error("fetch error: %v", err) diff --git a/pkg/experiment/remote/client_test.go b/pkg/experiment/remote/client_test.go index da66a2d..0198988 100644 --- a/pkg/experiment/remote/client_test.go +++ b/pkg/experiment/remote/client_test.go @@ -1,7 +1,6 @@ package remote import ( - "context" "fmt" "net/http" "net/http/httptest" @@ -15,9 +14,8 @@ import ( func TestClient_Fetch_DoesNotReturnDefaultVariants(t *testing.T) { client := Initialize("server-qz35UwzJ5akieoAdIgzM4m9MIiOLXLoz", nil) - ctx := context.Background() user := &experiment.User{} - result, err := client.Fetch(ctx, user) + result, err := client.Fetch(user) require.NoError(t, err) require.NotNil(t, result) variant := result["sdk-ci-test"] @@ -26,9 +24,8 @@ func TestClient_Fetch_DoesNotReturnDefaultVariants(t *testing.T) { func TestClient_FetchV2_ReturnsDefaultVariants(t *testing.T) { client := Initialize("server-qz35UwzJ5akieoAdIgzM4m9MIiOLXLoz", nil) - ctx := context.Background() user := &experiment.User{} - result, err := client.FetchV2(ctx, user) + result, err := client.FetchV2(user) require.NoError(t, err) require.NotNil(t, result) variant := result["sdk-ci-test"] @@ -50,8 +47,6 @@ func TestClient_FetchRetryWithDifferentResponseCodes(t *testing.T) { {0, "Other Exception", 2}, } - ctx := context.Background() - for _, data := range testData { // Mock client initialization with httptest config := &Config{ @@ -96,7 +91,7 @@ func TestClient_FetchRetryWithDifferentResponseCodes(t *testing.T) { fmt.Printf("%d %s\n", data.responseCode, data.errorMessage) // Perform the fetch and catch the exception - _, err := client.Fetch(ctx, &experiment.User{UserId: "test_user"}) + _, err := client.Fetch(&experiment.User{UserId: "test_user"}) if err != nil { fmt.Println(err.Error()) } From 7de20cffaeabb8e657e11bda927280615b9ce320 Mon Sep 17 00:00:00 2001 From: Hugo Biarge Date: Tue, 20 Aug 2024 08:59:16 +0200 Subject: [PATCH 4/4] Reorder FetchV2WithContext signature --- pkg/experiment/remote/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/experiment/remote/client.go b/pkg/experiment/remote/client.go index c06d36e..83e8eb6 100644 --- a/pkg/experiment/remote/client.go +++ b/pkg/experiment/remote/client.go @@ -62,11 +62,11 @@ func (c *Client) Fetch(user *experiment.User) (map[string]experiment.Variant, er // Unlike Fetch, this method returns all variants, including default variants. func (c *Client) FetchV2(user *experiment.User) (map[string]experiment.Variant, error) { ctx := context.Background() - return c.FetchV2WithContext(ctx, user) + return c.FetchV2WithContext(user, ctx) } // FetchV2WithContext fetches variants for a user from the remote evaluation service with a context. -func (c *Client) FetchV2WithContext(ctx context.Context, user *experiment.User) (map[string]experiment.Variant, error) { +func (c *Client) FetchV2WithContext(user *experiment.User, ctx context.Context) (map[string]experiment.Variant, error) { variants, err := c.doFetch(ctx, user, c.config.FetchTimeout) if err != nil { c.log.Error("fetch error: %v", err)