From 8d96dd71c7739c1e6c2f2c86509df08f23752841 Mon Sep 17 00:00:00 2001 From: Daniel Nelson Date: Thu, 19 Sep 2019 20:03:10 -0700 Subject: [PATCH] Allow graphite parser to create Inf and NaN values (#6420) --- plugins/parsers/graphite/errors.go | 14 --------- plugins/parsers/graphite/parser.go | 7 +---- plugins/parsers/graphite/parser_test.go | 42 ++++++++++++++++++++----- testutil/metric.go | 8 +++-- 4 files changed, 41 insertions(+), 30 deletions(-) delete mode 100644 plugins/parsers/graphite/errors.go diff --git a/plugins/parsers/graphite/errors.go b/plugins/parsers/graphite/errors.go deleted file mode 100644 index 2cd2f5583..000000000 --- a/plugins/parsers/graphite/errors.go +++ /dev/null @@ -1,14 +0,0 @@ -package graphite - -import "fmt" - -// An UnsupposedValueError is returned when a parsed value is not -// supposed. -type UnsupposedValueError struct { - Field string - Value float64 -} - -func (err *UnsupposedValueError) Error() string { - return fmt.Sprintf(`field "%s" value: "%v" is unsupported`, err.Field, err.Value) -} diff --git a/plugins/parsers/graphite/parser.go b/plugins/parsers/graphite/parser.go index 75c0475e3..f50217711 100644 --- a/plugins/parsers/graphite/parser.go +++ b/plugins/parsers/graphite/parser.go @@ -9,9 +9,8 @@ import ( "strings" "time" - "github.com/influxdata/telegraf/internal/templating" - "github.com/influxdata/telegraf" + "github.com/influxdata/telegraf/internal/templating" "github.com/influxdata/telegraf/metric" ) @@ -121,10 +120,6 @@ func (p *GraphiteParser) ParseLine(line string) (telegraf.Metric, error) { return nil, fmt.Errorf(`field "%s" value: %s`, fields[0], err) } - if math.IsNaN(v) || math.IsInf(v, 0) { - return nil, &UnsupposedValueError{Field: fields[0], Value: v} - } - fieldValues := map[string]interface{}{} if field != "" { fieldValues[field] = v diff --git a/plugins/parsers/graphite/parser_test.go b/plugins/parsers/graphite/parser_test.go index d84551add..9254574b6 100644 --- a/plugins/parsers/graphite/parser_test.go +++ b/plugins/parsers/graphite/parser_test.go @@ -1,14 +1,14 @@ package graphite import ( - "reflect" + "math" "strconv" "testing" "time" "github.com/influxdata/telegraf/internal/templating" "github.com/influxdata/telegraf/metric" - + "github.com/influxdata/telegraf/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -355,14 +355,40 @@ func TestParse(t *testing.T) { func TestParseNaN(t *testing.T) { p, err := NewGraphiteParser("", []string{"measurement*"}, nil) - assert.NoError(t, err) + require.NoError(t, err) - _, err = p.ParseLine("servers.localhost.cpu_load NaN 1435077219") - assert.Error(t, err) + m, err := p.ParseLine("servers.localhost.cpu_load NaN 1435077219") + require.NoError(t, err) - if _, ok := err.(*UnsupposedValueError); !ok { - t.Fatalf("expected *ErrUnsupportedValue, got %v", reflect.TypeOf(err)) - } + expected := testutil.MustMetric( + "servers.localhost.cpu_load", + map[string]string{}, + map[string]interface{}{ + "value": math.NaN(), + }, + time.Unix(1435077219, 0), + ) + + testutil.RequireMetricEqual(t, expected, m) +} + +func TestParseInf(t *testing.T) { + p, err := NewGraphiteParser("", []string{"measurement*"}, nil) + require.NoError(t, err) + + m, err := p.ParseLine("servers.localhost.cpu_load +Inf 1435077219") + require.NoError(t, err) + + expected := testutil.MustMetric( + "servers.localhost.cpu_load", + map[string]string{}, + map[string]interface{}{ + "value": math.Inf(1), + }, + time.Unix(1435077219, 0), + ) + + testutil.RequireMetricEqual(t, expected, m) } func TestFilterMatchDefault(t *testing.T) { diff --git a/testutil/metric.go b/testutil/metric.go index 25e23fa20..da3ace0f2 100644 --- a/testutil/metric.go +++ b/testutil/metric.go @@ -129,7 +129,7 @@ func IgnoreTime() cmp.Option { } // MetricEqual returns true if the metrics are equal. -func MetricEqual(expected, actual telegraf.Metric) bool { +func MetricEqual(expected, actual telegraf.Metric, opts ...cmp.Option) bool { var lhs, rhs *metricDiff if expected != nil { lhs = newMetricDiff(expected) @@ -138,7 +138,8 @@ func MetricEqual(expected, actual telegraf.Metric) bool { rhs = newMetricDiff(actual) } - return cmp.Equal(lhs, rhs) + opts = append(opts, cmpopts.EquateNaNs()) + return cmp.Equal(lhs, rhs, opts...) } // RequireMetricEqual halts the test with an error if the metrics are not @@ -154,6 +155,7 @@ func RequireMetricEqual(t *testing.T, expected, actual telegraf.Metric, opts ... rhs = newMetricDiff(actual) } + opts = append(opts, cmpopts.EquateNaNs()) if diff := cmp.Diff(lhs, rhs, opts...); diff != "" { t.Fatalf("telegraf.Metric\n--- expected\n+++ actual\n%s", diff) } @@ -172,6 +174,8 @@ func RequireMetricsEqual(t *testing.T, expected, actual []telegraf.Metric, opts for _, m := range actual { rhs = append(rhs, newMetricDiff(m)) } + + opts = append(opts, cmpopts.EquateNaNs()) if diff := cmp.Diff(lhs, rhs, opts...); diff != "" { t.Fatalf("[]telegraf.Metric\n--- expected\n+++ actual\n%s", diff) }