Skip to content

Commit

Permalink
FeatureEnabledForID defaults should be a percentage (#11)
Browse files Browse the repository at this point in the history
  • Loading branch information
Christopher Burnett authored Aug 16, 2017
1 parent 320bd90 commit 3d5177a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 19 deletions.
4 changes: 2 additions & 2 deletions snapshot/iface.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ type IFace interface {
// @param key supplies the feature key to lookup.
// @param id supplies the ID to use in the CRC check.
// @param defaultValue supplies the default value that will be used if either the feature key
// does not exist or it is not a bool.
FeatureEnabledForID(key string, id uint64, defaultValue bool) bool
// does not exist or it is not a valid percentage.
FeatureEnabledForID(key string, id uint64, defaultPercentage uint32) bool

// Fetch raw runtime data based on key.
// @param key supplies the key to fetch.
Expand Down
4 changes: 2 additions & 2 deletions snapshot/nil.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ func (n *Nil) FeatureEnabled(key string, defaultValue uint64) bool {
return defaultRandomGenerator.Random()%100 < min(defaultValue, 100)
}

func (n *Nil) FeatureEnabledForID(key string, id uint64, defaultValue bool) bool {
return defaultValue
func (n *Nil) FeatureEnabledForID(key string, id uint64, defaultPercentage uint32) bool {
return true
}

func (n *Nil) Get(key string) string { return "" }
Expand Down
22 changes: 10 additions & 12 deletions snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,29 +60,29 @@ func (s *Snapshot) FeatureEnabled(key string, defaultValue uint64) bool {

// FeatureEnabledForID checks that the crc32 of the id and key's byte value falls within the mod of
// the 0-100 value for the given feature. Use this method for "sticky" features
func (s *Snapshot) FeatureEnabledForID(key string, id uint64, defaultValue bool) bool {
func (s *Snapshot) FeatureEnabledForID(key string, id uint64, defaultPercentage uint32) bool {
if e, ok := s.Entries()[key]; ok {
if e.Uint64Valid {
return withinPercentile(id, uint32(e.Uint64Value), key)
return enabled(id, uint32(e.Uint64Value), key)
}
}

return defaultValue
return enabled(id, defaultPercentage, key)
}

func (s *Snapshot) Get(key string) string {
entry, ok := s.entries[key]
e, ok := s.entries[key]
if ok {
return entry.StringValue
return e.StringValue
} else {
return ""
}
}

func (s *Snapshot) GetInteger(key string, defaultValue uint64) uint64 {
entry, ok := s.entries[key]
if ok && entry.Uint64Valid {
return entry.Uint64Value
e, ok := s.entries[key]
if ok && e.Uint64Valid {
return e.Uint64Value
} else {
return defaultValue
}
Expand All @@ -104,12 +104,10 @@ func (s *Snapshot) SetEntry(key string, e *entry.Entry) {
s.entries[key] = e
}

func withinPercentile(id uint64, percentage uint32, feature string) bool {
func enabled(id uint64, percentage uint32, feature string) bool {
uid := crc(id, feature)

m := uid % 100

return m < percentage
return uid%100 < percentage
}

func crc(id uint64, feature string) uint32 {
Expand Down
13 changes: 10 additions & 3 deletions snapshot/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,25 @@ func TestSnapshot_FeatureEnabledForID(t *testing.T) {
key := "test"
ss := NewMock()
ss.SetUInt64(key, 100)
assert.True(t, ss.FeatureEnabledForID(key, 1, true))
assert.True(t, ss.FeatureEnabledForID(key, 1, 100))

ss.SetUInt64(key, 0)
assert.False(t, ss.FeatureEnabledForID(key, 1, true))
assert.False(t, ss.FeatureEnabledForID(key, 1, 100))

enabled := 0
for i := 1; i < 101; i++ {
ss.SetUInt64(key, uint64(i))
if ss.FeatureEnabledForID(key, uint64(i), true) {
if ss.FeatureEnabledForID(key, uint64(i), 100) {
enabled++
}
}

assert.Equal(t, 47, enabled)
}

func TestSnapshot_FeatureEnabledForIDDisabled(t *testing.T) {
key := "test"
ss := NewMock()
assert.True(t, ss.FeatureEnabledForID(key, 1, 100))
assert.False(t, ss.FeatureEnabledForID(key, 1, 0))
}

0 comments on commit 3d5177a

Please sign in to comment.