Repository: Unleash/unleash-client-go
Pull request #124 (2022-12-15 10:15:05)

force getVariant to also add to the feature toggle metrics counter
This forces resolving a variant to correctly add to the feature toggle counter metrics. Also fixes an issue where variants wouldn't get counted for the default/disabled case (brings this SDK inline with the behaviour of its siblings)

Also, other unrelated changes:
1. Moved unleash_test.go to unleash_mock.go because it was in `unleash` package and not having any test
2. Improved the Readme due to an unexisting command


Patch:
diff --git a/README.md b/README.md
index 9f8caca..3032f35 100644
--- a/README.md
+++ b/README.md
@@ -285,4 +285,4 @@ Run code-style checks:(currently failing)
 
 Run race-tests:
 
-    make test-all
+    make test-race
diff --git a/client.go b/client.go
index 12d534d..ca30ef7 100644
--- a/client.go
+++ b/client.go
@@ -315,6 +315,15 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) (enabled b
 //
 // It is safe to call this method from multiple goroutines concurrently.
 func (uc *Client) GetVariant(feature string, options ...VariantOption) *api.Variant {
+	variant := uc.getVariantWithoutMetrics(feature, options...)
+	defer func() {
+		uc.metrics.countVariants(feature, variant.Enabled, variant.Name)
+	}()
+	return variant
+}
+
+// getVariantWithoutMetrics abstracts away the logic for resolving a variant without metrics
+func (uc *Client) getVariantWithoutMetrics(feature string, options ...VariantOption) *api.Variant {
 	defaultVariant := api.GetDefaultVariant()
 	var opts variantOption
 	for _, o := range options {
@@ -360,12 +369,7 @@ func (uc *Client) GetVariant(feature string, options ...VariantOption) *api.Vari
 		return defaultVariant
 	}
 
-	variant := f.GetVariant(ctx)
-
-	defer func() {
-		uc.metrics.countVariants(feature, variant.Name)
-	}()
-	return variant
+	return f.GetVariant(ctx)
 }
 
 // Close stops the client from syncing data from the server.
diff --git a/metrics.go b/metrics.go
index 6945941..0efb729 100644
--- a/metrics.go
+++ b/metrics.go
@@ -230,11 +230,13 @@ func (m *metrics) count(name string, enabled bool) {
 	m.metricsChannels.count <- metric{Name: name, Enabled: enabled}
 }
 
-func (m *metrics) countVariants(name string, variantName string) {
+func (m *metrics) countVariants(name string, enabled bool, variantName string) {
 	if m.options.disableMetrics {
 		return
 	}
 
+	m.add(name, enabled, 1)
+	m.metricsChannels.count <- metric{Name: name, Enabled: enabled}
 	t, _ := m.bucket.Toggles[name]
 	if len(t.Variants) == 0 {
 		t.Variants = make(map[string]int32)
diff --git a/unleash_mock.go b/unleash_mock.go
new file mode 100644
index 0000000..76be4de
--- /dev/null
+++ b/unleash_mock.go
@@ -0,0 +1,49 @@
+package unleash
+
+import (
+	"encoding/json"
+	"net/http"
+
+	"github.com/stretchr/testify/mock"
+)
+
+const (
+	mockerServer   = "http://foo.com"
+	mockAppName    = "unleash-client-go-tests"
+	mockInstanceId = "1234"
+)
+
+type MockedListener struct {
+	mock.Mock
+}
+
+func (l *MockedListener) OnError(err error) {
+	l.Called(err)
+}
+
+func (l *MockedListener) OnWarning(warning error) {
+	l.Called(warning)
+}
+
+func (l *MockedListener) OnReady() {
+	l.Called()
+}
+
+func (l *MockedListener) OnCount(name string, enabled bool) {
+	l.Called(name, enabled)
+}
+
+func (l *MockedListener) OnSent(payload MetricsData) {
+	l.Called(payload)
+}
+
+func (l *MockedListener) OnRegistered(payload ClientData) {
+	l.Called(payload)
+}
+
+func writeJSON(rw http.ResponseWriter, x interface{}) {
+	enc := json.NewEncoder(rw)
+	if err := enc.Encode(x); err != nil {
+		panic(err)
+	}
+}


Test patch:
diff --git a/client_test.go b/client_test.go
index 1167a01..77bc4a3 100644
--- a/client_test.go
+++ b/client_test.go
@@ -351,6 +351,7 @@ func TestClientWithVariantContext(t *testing.T) {
 
 	mockListener := &MockedListener{}
 	mockListener.On("OnReady").Return()
+	mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Return()
 	mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
 	mockListener.On("OnError", mock.AnythingOfType("*errors.errorString"))
 
@@ -854,6 +855,7 @@ func TestClient_VariantShouldFailWhenSegmentConstraintsDontMatch(t *testing.T) {
 
 	mockListener := &MockedListener{}
 	mockListener.On("OnReady").Return()
+	mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Return()
 	mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
 	mockListener.On("OnError").Return()
 
diff --git a/metrics_test.go b/metrics_test.go
index e84cc48..0fea62a 100644
--- a/metrics_test.go
+++ b/metrics_test.go
@@ -53,6 +53,43 @@ func TestMetrics_RegisterInstance(t *testing.T) {
 	assert.True(gock.IsDone(), "there should be no more mocks")
 }
 
+func TestMetrics_VariantsCountToggles(t *testing.T) {
+	assert := assert.New(t)
+	defer gock.OffAll()
+
+	gock.New(mockerServer).
+		Post("/client/register").
+		MatchHeader("UNLEASH-APPNAME", mockAppName).
+		MatchHeader("UNLEASH-INSTANCEID", mockInstanceId).
+		Reply(200)
+
+	gock.New(mockerServer).
+		Get("/client/features").
+		Reply(200).
+		JSON(api.FeatureResponse{})
+
+	mockListener := &MockedListener{}
+	mockListener.On("OnReady").Return()
+	mockListener.On("OnCount", "foo", false).Return()
+	mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
+
+	client, err := NewClient(
+		WithUrl(mockerServer),
+		WithAppName(mockAppName),
+		WithInstanceId(mockInstanceId),
+		WithListener(mockListener),
+	)
+
+	client.WaitForReady()
+	client.GetVariant("foo")
+
+	assert.EqualValues(client.metrics.bucket.Toggles["foo"].No, 1)
+	client.Close()
+
+	assert.Nil(err, "client should not return an error")
+	assert.True(gock.IsDone(), "there should be no more mocks")
+}
+
 func TestMetrics_DoPost(t *testing.T) {
 	assert := assert.New(t)
 	defer gock.OffAll()
diff --git a/unleash_test.go b/unleash_test.go
index 76be4de..f4d8503 100644
--- a/unleash_test.go
+++ b/unleash_test.go
@@ -1,49 +1,66 @@
-package unleash
+package unleash_test
 
 import (
-	"encoding/json"
-	"net/http"
+	"fmt"
+	"os"
+	"testing"
+	"time"
 
-	"github.com/stretchr/testify/mock"
+	"github.com/Unleash/unleash-client-go/v3"
 )
 
-const (
-	mockerServer   = "http://foo.com"
-	mockAppName    = "unleash-client-go-tests"
-	mockInstanceId = "1234"
-)
-
-type MockedListener struct {
-	mock.Mock
-}
-
-func (l *MockedListener) OnError(err error) {
-	l.Called(err)
-}
-
-func (l *MockedListener) OnWarning(warning error) {
-	l.Called(warning)
-}
+func Test_withVariants(t *testing.T) {
+	demoReader, err := os.Open("demo_app_toggles.json")
+	if err != nil {
+		t.Fail()
+	}
+	err = unleash.Initialize(
+		unleash.WithListener(&unleash.DebugListener{}),
+		unleash.WithAppName("my-application"),
+		unleash.WithRefreshInterval(5*time.Second),
+		unleash.WithMetricsInterval(5*time.Second),
+		unleash.WithStorage(&unleash.BootstrapStorage{Reader: demoReader}),
+		unleash.WithUrl("https://localhost:4242"),
+	)
 
-func (l *MockedListener) OnReady() {
-	l.Called()
-}
+	if err != nil {
+		t.Fail()
+	}
 
-func (l *MockedListener) OnCount(name string, enabled bool) {
-	l.Called(name, enabled)
+	feature := unleash.GetVariant("Demo")
+	fmt.Printf("feature is %v\n", feature)
+	if feature.Enabled == false {
+		t.Fatalf("Expected feature to be enabled")
+	}
+	if feature.Name != "small" && feature.Name != "medium" {
+		t.Fatalf("Expected one of the variant names")
+	}
+	if feature.Payload.Value != "35" && feature.Payload.Value != "55" {
+		t.Fatalf("Expected one of the variant payloads")
+	}
 }
 
-func (l *MockedListener) OnSent(payload MetricsData) {
-	l.Called(payload)
-}
+func Test_withVariantsAndANonExistingStrategyName(t *testing.T) {
+	demoReader, err := os.Open("demo_app_toggles.json")
+	if err != nil {
+		t.Fail()
+	}
+	err = unleash.Initialize(
+		unleash.WithListener(&unleash.DebugListener{}),
+		unleash.WithAppName("my-application"),
+		unleash.WithRefreshInterval(5*time.Second),
+		unleash.WithMetricsInterval(5*time.Second),
+		unleash.WithStorage(&unleash.BootstrapStorage{Reader: demoReader}),
+		unleash.WithUrl("https://localhost:4242"),
+	)
 
-func (l *MockedListener) OnRegistered(payload ClientData) {
-	l.Called(payload)
-}
+	if err != nil {
+		t.Fail()
+	}
 
-func writeJSON(rw http.ResponseWriter, x interface{}) {
-	enc := json.NewEncoder(rw)
-	if err := enc.Encode(x); err != nil {
-		panic(err)
+	feature := unleash.GetVariant("AuditLog")
+	fmt.Printf("feature is %v\n", feature)
+	if feature.Enabled == true {
+		t.Fatalf("Expected feature to be disabled because Environment does not exist as strategy")
 	}
 }