Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: New Metrics metadata #175

Merged
merged 5 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ func NewClient(options ...ConfigOption) (*Client, error) {
httpClient: uc.options.httpClient,
customHeaders: uc.options.customHeaders,
disableMetrics: uc.options.disableMetrics,
started: uc.options.started,
},
metricsChannels{
errorChannels: errChannels,
Expand Down
10 changes: 5 additions & 5 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (

"testing"

"github.com/h2non/gock"
"github.com/stretchr/testify/assert"
"gopkg.in/h2non/gock.v1"
)

func TestClientWithoutListener(t *testing.T) {
Expand Down Expand Up @@ -1399,12 +1399,12 @@ func TestGetVariant_FallbackVariantFeatureEnabledSettingIsLeftUnchanged(t *testi
client.WaitForReady()

fallbackVariantFeatureEnabled := api.Variant{
Name: "fallback-variant-feature-enabled",
FeatureEnabled: true,
Name: "fallback-variant-feature-enabled",
FeatureEnabled: true,
}
fallbackVariantFeatureDisabled := api.Variant{
Name: "fallback-variant-feature-disabled",
FeatureEnabled: false,
Name: "fallback-variant-feature-disabled",
FeatureEnabled: false,
}

variantForEnabledFeatureNoVariants := client.GetVariant(enabledFeatureNoVariants, WithVariantFallback(&fallbackVariantFeatureDisabled))
Expand Down
7 changes: 7 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type configOption struct {
storage Storage
httpClient *http.Client
customHeaders http.Header
started *time.Time
}

// ConfigOption represents a option for configuring the client.
Expand Down Expand Up @@ -107,6 +108,11 @@ func WithStrategies(strategies ...strategy.Strategy) ConfigOption {
o.strategies = strategies
}
}
func WithStarted(startedAt time.Time) ConfigOption {
return func(o *configOption) {
o.started = &startedAt
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should started be configurable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly for tests here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would've used #[cfg(test)] here, don't know what the same is in go.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I've moved it into the test file where I used it instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't like that we're polluting our config options with something that shouldn't be user-configurable and I'm still not super convinced of its value. Maybe we can relax our tests instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this seems to work, but maybe there's a better way:

func clientDataMatcher() func(req *http.Request, ereq *gock.Request) (bool, error) {
	defaultStrategies := []string{
		"default",
		"applicationHostname",
		"gradualRolloutRandom",
		"gradualRolloutSessionId",
		"gradualRolloutUserId",
		"remoteAddress",
		"userWithId",
		"flexibleRollout",
	}
	return func(req *http.Request, ereq *gock.Request) (bool, error) {
		var data ClientData
		err := json.NewDecoder(req.Body).Decode(&data)
		if err != nil {
			return false, err
		}

		if data.Started.IsZero() {
			return false, nil
		}

		expectedData := ClientData{
			AppName:          mockAppName,
			InstanceID:       mockInstanceId,
			SDKVersion:       fmt.Sprintf("%s:%s", clientName, clientVersion),
			Strategies:       defaultStrategies,
			Interval:         0,
			PlatformVersion:  runtime.Version(),
			PlatformName:     "go",
			YggdrasilVersion: nil,
			SpecVersion:      specVersion,
		}

		return data.AppName == expectedData.AppName &&
			data.InstanceID == expectedData.InstanceID &&
			data.SDKVersion == expectedData.SDKVersion &&
			compareStringSlices(data.Strategies, expectedData.Strategies) &&
			data.Interval == expectedData.Interval &&
			data.PlatformVersion == expectedData.PlatformVersion &&
			data.PlatformName == expectedData.PlatformName &&
			data.YggdrasilVersion == expectedData.YggdrasilVersion &&
			data.SpecVersion == expectedData.SpecVersion, nil
	}
}

func compareStringSlices(a, b []string) bool {
	if len(a) != len(b) {
		return false
	}
	for i := range a {
		if a[i] != b[i] {
			return false
		}
	}
	return true
}

func TestMetrics_ClientDataIncludesNewMetadata(t *testing.T) {
	assert := assert.New(t)
	defer gock.OffAll()

	gock.New(mockerServer).
		Post("/client/register").
		AddMatcher(clientDataMatcher()).
		Reply(200)

	client, err := NewClient(
		WithUrl(mockerServer),
		WithMetricsInterval(50*time.Millisecond),
		WithAppName(mockAppName),
		WithInstanceId(mockInstanceId),
	)

	assert.Nil(err, "Client should open without a problem")

	time.Sleep(100 * time.Millisecond)

	err = client.Close()

	assert.Nil(err, "Client should close without errors")

	st.Expect(t, gock.IsDone(), true)
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I added your suggestion

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And removed started from the options


// WithStorage specifies which storage implementation the repository should use for storing feature
// toggles.
Expand Down Expand Up @@ -265,4 +271,5 @@ type metricsOptions struct {
disableMetrics bool
httpClient *http.Client
customHeaders http.Header
started *time.Time
}
2 changes: 1 addition & 1 deletion example_bootstrap_from_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (

"github.com/Unleash/unleash-client-go/v4"
"github.com/Unleash/unleash-client-go/v4/context"
"github.com/h2non/gock"
"github.com/stretchr/testify/assert"
"gopkg.in/h2non/gock.v1"
)

func Test_bootstrapFromFile(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ module github.com/Unleash/unleash-client-go/v4
require (
github.com/Masterminds/semver/v3 v3.1.1
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32 // indirect
github.com/h2non/gock v1.2.0
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/stretchr/objx v0.1.1 // indirect
github.com/stretchr/testify v1.2.2
github.com/twmb/murmur3 v1.1.8
gopkg.in/h2non/gock.v1 v1.0.10
)

go 1.13
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ github.com/Masterminds/semver/v3 v3.1.1 h1:hLg3sBzpNErnxhQtUy/mmLR2I9foDujNK030I
github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/h2non/gock v1.2.0 h1:K6ol8rfrRkUOefooBC8elXoaNGYkpp7y2qcxGG6BzUE=
github.com/h2non/gock v1.2.0/go.mod h1:tNhoxHYW2W42cYkYb1WqzdbYIieALC99kpYr7rH/BQk=
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 h1:2VTzZjLZBgl62/EtslCrtky5vbi9dd7HrQPQIx6wqiw=
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542/go.mod h1:Ow0tF8D4Kplbc8s8sSb3V2oUCygFHVp8gC3Dn6U4MNI=
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32 h1:W6apQkHrMkS0Muv8G/TipAy/FJl/rCYT0+EuS8+Z0z4=
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32/go.mod h1:9wM+0iRr9ahx58uYLpLIr5fm8diHn0JbqRycJi6w0Ms=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
Expand All @@ -12,5 +16,3 @@ github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/twmb/murmur3 v1.1.8 h1:8Yt9taO/WN3l08xErzjeschgZU2QSrwm1kclYq+0aRg=
github.com/twmb/murmur3 v1.1.8/go.mod h1:Qq/R7NUyOfr65zD+6Q5IHKsJLwP7exErjN6lyyq3OSQ=
gopkg.in/h2non/gock.v1 v1.0.10 h1:D4j796HhgidcxF0LnDyFXcoEbEZWoLEWf0kRh61p22w=
gopkg.in/h2non/gock.v1 v1.0.10/go.mod h1:KHI4Z1sxDW6P4N3DfTWSEza07YpkQP7KJBfglRMEjKY=
38 changes: 34 additions & 4 deletions metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"math"
"net/http"
"net/url"
"runtime"
"sync"
"time"

Expand All @@ -24,6 +25,18 @@ type MetricsData struct {

// Bucket is the payload data sent to the server.
Bucket api.Bucket `json:"bucket"`

// The runtime version of our Platform
PlatformVersion string `json:"platformVersion"`

// The runtime name of our Platform
PlatformName string `json:"platformName"`

// Which version of Yggdrasil is being used
YggdrasilVersion *string `json:"yggdrasilVersion"`

// Optional field that describes the sdk version (name:version)
SDKVersion string `json:"sdkVersion"`
}

// ClientData represents the data sent to the unleash during registration.
Expand All @@ -46,6 +59,12 @@ type ClientData struct {
// Interval specifies the time interval (in ms) that the client is using for refreshing
// feature toggles.
Interval int64 `json:"interval"`

PlatformVersion string `json:"platformVersion"`

PlatformName string `json:"platformName"`

YggdrasilVersion *string `json:"yggdrasilVersion"`
}

type metric struct {
Expand Down Expand Up @@ -73,10 +92,14 @@ type metrics struct {
}

func newMetrics(options metricsOptions, channels metricsChannels) *metrics {
started := time.Now()
if options.started != nil {
started = *options.started
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this can be reverted now, and we can set started: time.Now() like before.

m := &metrics{
metricsChannels: channels,
options: options,
started: time.Now(),
started: started,
close: make(chan struct{}),
closed: make(chan struct{}),
maxSkips: 10,
Expand Down Expand Up @@ -174,9 +197,13 @@ func (m *metrics) sendMetrics() {
}
bucket.Stop = time.Now()
payload := MetricsData{
AppName: m.options.appName,
InstanceID: m.options.instanceId,
Bucket: bucket,
AppName: m.options.appName,
InstanceID: m.options.instanceId,
Bucket: bucket,
SDKVersion: fmt.Sprintf("%s:%s", clientName, clientVersion),
PlatformName: "go",
PlatformVersion: runtime.Version(),
YggdrasilVersion: nil,
}

u, _ := m.options.url.Parse("./client/metrics")
Expand Down Expand Up @@ -305,5 +332,8 @@ func (m *metrics) getClientData() ClientData {
m.options.strategies,
m.started,
int64(m.options.metricsInterval.Seconds()),
runtime.Version(),
"go",
nil,
}
}
130 changes: 129 additions & 1 deletion metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,24 @@ package unleash

import (
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
"net/url"
"reflect"
"runtime"
"strings"
"sync/atomic"
"testing"
"time"

"github.com/Unleash/unleash-client-go/v4/api"
internalapi "github.com/Unleash/unleash-client-go/v4/internal/api"
"github.com/h2non/gock"
"github.com/nbio/st"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"gopkg.in/h2non/gock.v1"
)

func TestMetrics_RegisterInstance(t *testing.T) {
Expand Down Expand Up @@ -426,3 +429,128 @@ func TestMetrics_ErrorCountShouldDecreaseIfSuccessful(t *testing.T) {
assert.Equal(float64(0), client.metrics.errors)
assert.Nil(err, "Client should close without a problem")
}

func TestMetrics_ClientDataIncludesNewMetadata(t *testing.T) {
defaultStrategies := []string{
"default",
"applicationHostname",
"gradualRolloutRandom",
"gradualRolloutSessionId",
"gradualRolloutUserId",
"remoteAddress",
"userWithId",
"flexibleRollout",
}
assert := assert.New(t)
started := time.Now()
defer gock.OffAll()
gock.New(mockerServer).
Post("/client/register").
JSON(ClientData{
AppName: mockAppName,
InstanceID: mockInstanceId,
SDKVersion: fmt.Sprintf("%s:%s", clientName, clientVersion),
Strategies: defaultStrategies,
Started: started,
Interval: 0,
PlatformVersion: runtime.Version(),
PlatformName: "go",
YggdrasilVersion: nil,
}).
Reply(200)
client, err := NewClient(
WithUrl(mockerServer),
WithMetricsInterval(50*time.Millisecond),
WithAppName(mockAppName),
WithInstanceId(mockInstanceId),
WithStarted(started),
)

assert.Nil(err, "Client should open without a problem")

time.Sleep(100 * time.Millisecond)

err = client.Close()

assert.Nil(err, "Client should close without errors")

st.Expect(t, gock.IsDone(), true)
}

func TestMetrics_metricsData_includes_new_metadata(t *testing.T) {
assert := assert.New(t)
defer gock.OffAll()

gock.Observe(gock.DumpRequest)

gock.New(mockerServer).Post("/client/register").Reply(200)
gock.New(mockerServer).
Post("/client/metrics").
BodyString(`.*platformVersion.*`).
Reply(200)
gock.New(mockerServer).
Get("/client/features").
Reply(200).
JSON(api.FeatureResponse{
Features: []api.Feature{
{
Name: "parent",
Enabled: true,
Description: "parent toggle",
Strategies: []api.Strategy{
{
Id: 1,
Name: "flexibleRollout",
Constraints: []api.Constraint{},
Parameters: map[string]interface{}{
"rollout": 100,
"stickiness": "default",
},
},
},
},
{
Name: "child",
Enabled: true,
Description: "parent toggle",
Strategies: []api.Strategy{
{
Id: 1,
Name: "flexibleRollout",
Constraints: []api.Constraint{},
Parameters: map[string]interface{}{
"rollout": 100,
"stickiness": "default",
},
},
},
Dependencies: &[]api.Dependency{
{
Feature: "parent",
},
},
},
},
})

client, err := NewClient(
WithUrl(mockerServer),
WithMetricsInterval(50*time.Millisecond),
WithAppName(mockAppName),
WithInstanceId(mockInstanceId),
WithDisableMetrics(false),
)
assert.Nil(err, "Client should open without a problem")

client.WaitForReady()
client.IsEnabled("foo")
client.IsEnabled("bar")
client.IsEnabled("baz")

time.Sleep(320 * time.Millisecond)
err = client.Close()

assert.Nil(err, "Client should close without errors")

st.Expect(t, gock.IsDone(), true)
}
5 changes: 3 additions & 2 deletions repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package unleash
import (
"bytes"
"encoding/json"
"gopkg.in/h2non/gock.v1"
"net/http"
"net/http/httptest"
"strings"
"sync/atomic"
"testing"
"time"

"github.com/h2non/gock"

"github.com/Unleash/unleash-client-go/v4/api"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -186,4 +187,4 @@ func TestRepository_back_offs_are_gradually_reduced_on_success(t *testing.T) {
err = client.Close()
a.Equal(float64(3), client.repository.errors) // 4 failures, and then one success, should reduce error count to 3
a.Nil(err)
}
}
2 changes: 1 addition & 1 deletion spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import (

"github.com/Unleash/unleash-client-go/v4/api"
"github.com/Unleash/unleash-client-go/v4/context"
"github.com/h2non/gock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"
"gopkg.in/h2non/gock.v1"
)

const mockHost = "http://unleash-apu"
Expand Down
2 changes: 1 addition & 1 deletion unleash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"time"

"github.com/Unleash/unleash-client-go/v4"
"github.com/h2non/gock"
"github.com/stretchr/testify/assert"
"gopkg.in/h2non/gock.v1"
)

func Test_withVariants(t *testing.T) {
Expand Down
Loading