diff --git a/README.md b/README.md index b200650..fe54a8b 100644 --- a/README.md +++ b/README.md @@ -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://github.com/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://github.com/hongjianzhu)) diff --git a/bufferedclient_test.go b/bufferedclient_test.go index f10f1be..a332700 100644 --- a/bufferedclient_test.go +++ b/bufferedclient_test.go @@ -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 diff --git a/event/fgauge_test.go b/event/fgauge_test.go index c37f890..903fa31 100644 --- a/event/fgauge_test.go +++ b/event/fgauge_test.go @@ -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) + } +} diff --git a/event/fgaugedelta.go b/event/fgaugedelta.go index 450af2f..be4d070 100644 --- a/event/fgaugedelta.go +++ b/event/fgaugedelta.go @@ -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 diff --git a/event/fgaugedelta_test.go b/event/fgaugedelta_test.go index 1a2dcbb..16ab089 100644 --- a/event/fgaugedelta_test.go +++ b/event/fgaugedelta_test.go @@ -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) diff --git a/event/gauge_test.go b/event/gauge_test.go index 7f042a6..59de71c 100644 --- a/event/gauge_test.go +++ b/event/gauge_test.go @@ -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) + } +} diff --git a/event/gaugedelta.go b/event/gaugedelta.go index d7e7df7..8ee1ee9 100644 --- a/event/gaugedelta.go +++ b/event/gaugedelta.go @@ -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 diff --git a/event/gaugedelta_test.go b/event/gaugedelta_test.go index 0148273..dc8a27b 100644 --- a/event/gaugedelta_test.go +++ b/event/gaugedelta_test.go @@ -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) + } +}