Skip to content

Commit

Permalink
Fixed FGaugeDelta with negative value not to send a 0 value first (it…
Browse files Browse the repository at this point in the history
… mistakenly had the same implementation of FGauge)
  • Loading branch information
quipo committed Aug 2, 2017
1 parent fc4c92d commit b46e2c3
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 17 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ The string `%HOST%` in the metric name will automatically be replaced with the h

* Added stdout client ("echo" service for debugging)
* Fixed [issue #23](https:/quipo/statsd/issues/23): GaugeDelta event Stats() should not send an absolute value of 0
* Fixed FGauge to only preserve the last value in the batch (it mistakenly had the same implementation of FGaugeDelta)
* Fixed FGauge's collation in the buffered client to only preserve the last value in the batch (it mistakenly had the same implementation of FGaugeDelta's collation)
* Fixed FGaugeDelta with negative value not to send a 0 value first (it mistakenly had the same implementation of FGauge)
* Added tests and compile-time checks that the default events implement the Event interface

* `v.1.2.0`: Sample rate support (thanks to [Hongjian Zhu](https:/hongjianzhu))
Expand Down
1 change: 0 additions & 1 deletion bufferedclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ func TestBufferedFloat64(t *testing.T) {
},
expected: KVfloat64Sorter{
{"a:b:c", 3.0},
{"d:e:f", 0},
{"d:e:f", -2.2},
{"g.h.i", +1.3},
{"zz." + hostname, 1.4}, // also test %HOST% replacement
Expand Down
15 changes: 15 additions & 0 deletions event/fgauge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,18 @@ func TestFGaugeUpdate(t *testing.T) {
t.Errorf("did not receive all metrics: Expected: %T %v, Actual: %T %v ", expected, expected, actual, actual)
}
}

func TestFGaugeUpdateNegative(t *testing.T) {
e1 := &FGauge{Name: "test", Value: float64(-10.1)}
e2 := &FGauge{Name: "test", Value: float64(-3.4)}
err := e1.Update(e2)
if nil != err {
t.Error(err)
}

expected := []string{"test:0|g", "test:-3.4|g"} // only the last value is flushed
actual := e1.Stats()
if !reflect.DeepEqual(expected, actual) {
t.Errorf("did not receive all metrics: Expected: %T %v, Actual: %T %v ", expected, expected, actual, actual)
}
}
11 changes: 1 addition & 10 deletions event/fgaugedelta.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,7 @@ func (e FGaugeDelta) Payload() interface{} {

// Stats returns an array of StatsD events as they travel over UDP
func (e FGaugeDelta) Stats() []string {
if e.Value < 0 {
// because a leading '+' or '-' in the value of a gauge denotes a delta, to send
// a negative gauge value we first set the gauge absolutely to 0, then send the
// negative value as a delta from 0 (that's just how the spec works :-)
return []string{
fmt.Sprintf("%s:%d|g", e.Name, 0),
fmt.Sprintf("%s:%g|g", e.Name, e.Value),
}
}
return []string{fmt.Sprintf("%s:%g|g", e.Name, e.Value)}
return []string{fmt.Sprintf("%s:%+g|g", e.Name, e.Value)}
}

// Key returns the name of this metric
Expand Down
22 changes: 21 additions & 1 deletion event/fgaugedelta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,27 @@ func TestFGaugeDeltaUpdate(t *testing.T) {
t.Error(err)
}

expected := []string{"test:20.2|g"}
expected := []string{"test:+20.2|g"}
actual := e1.Stats()
if !reflect.DeepEqual(expected, actual) {
t.Errorf("did not receive all metrics: Expected: %T %v, Actual: %T %v ", expected, expected, actual, actual)
}
}

func TestFGaugeDeltaUpdateNegative(t *testing.T) {
e1 := &FGaugeDelta{Name: "test", Value: float64(-15.1)}
e2 := &FGaugeDelta{Name: "test", Value: float64(10.0)}
e3 := &FGaugeDelta{Name: "test", Value: float64(-15.1)}
err := e1.Update(e2)
if nil != err {
t.Error(err)
}
err = e1.Update(e3)
if nil != err {
t.Error(err)
}

expected := []string{"test:-20.2|g"}
actual := e1.Stats()
if !reflect.DeepEqual(expected, actual) {
t.Errorf("did not receive all metrics: Expected: %T %v, Actual: %T %v ", expected, expected, actual, actual)
Expand Down
15 changes: 15 additions & 0 deletions event/gauge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,18 @@ func TestGaugeUpdate(t *testing.T) {
t.Errorf("did not receive all metrics: Expected: %T %v, Actual: %T %v ", expected, expected, actual, actual)
}
}

func TestGaugeUpdateNegative(t *testing.T) {
e1 := &Gauge{Name: "test", Value: int64(-10)}
e2 := &Gauge{Name: "test", Value: int64(-3)}
err := e1.Update(e2)
if nil != err {
t.Error(err)
}

expected := []string{"test:0|g", "test:-3|g"} // only the last value is flushed
actual := e1.Stats()
if !reflect.DeepEqual(expected, actual) {
t.Errorf("did not receive all metrics: Expected: %T %v, Actual: %T %v ", expected, expected, actual, actual)
}
}
5 changes: 1 addition & 4 deletions event/gaugedelta.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@ func (e GaugeDelta) Payload() interface{} {

// Stats returns an array of StatsD events as they travel over UDP
func (e GaugeDelta) Stats() []string {
if e.Value < 0 {
return []string{fmt.Sprintf("%s:%+d|g", e.Name, e.Value)}
}
return []string{fmt.Sprintf("%s:+%d|g", e.Name, e.Value)}
return []string{fmt.Sprintf("%s:%+d|g", e.Name, e.Value)}
}

// Key returns the name of this metric
Expand Down
20 changes: 20 additions & 0 deletions event/gaugedelta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,23 @@ func TestGaugeDeltaUpdate(t *testing.T) {
t.Errorf("did not receive all metrics: Expected: %T %v, Actual: %T %v ", expected, expected, actual, actual)
}
}

func TestGaugeDeltaUpdateNegative(t *testing.T) {
e1 := &GaugeDelta{Name: "test", Value: int64(-15)}
e2 := &GaugeDelta{Name: "test", Value: int64(10)}
e3 := &GaugeDelta{Name: "test", Value: int64(-15)}
err := e1.Update(e2)
if nil != err {
t.Error(err)
}
err = e1.Update(e3)
if nil != err {
t.Error(err)
}

expected := []string{"test:-20|g"}
actual := e1.Stats()
if !reflect.DeepEqual(expected, actual) {
t.Errorf("did not receive all metrics: Expected: %T %v, Actual: %T %v ", expected, expected, actual, actual)
}
}

0 comments on commit b46e2c3

Please sign in to comment.