From f0c10b401299c2fa5140e3b21cfb5fee3b82894f Mon Sep 17 00:00:00 2001 From: Daniel Nelson Date: Wed, 7 Jun 2017 13:37:54 -0700 Subject: [PATCH] Fix metric splitting edge cases (#2896) Metrics needing one extra byte to fit the output buffer would not be split, so we would emit lines without a line ending. Metrics which overflowed by exactly one field length would be split one field too late, causing truncated fields. --- metric/metric.go | 4 ++-- metric/metric_test.go | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/metric/metric.go b/metric/metric.go index edf3b7794..0dbc1fa85 100644 --- a/metric/metric.go +++ b/metric/metric.go @@ -218,7 +218,7 @@ func (m *metric) SerializeTo(dst []byte) int { } func (m *metric) Split(maxSize int) []telegraf.Metric { - if m.Len() < maxSize { + if m.Len() <= maxSize { return []telegraf.Metric{m} } var out []telegraf.Metric @@ -248,7 +248,7 @@ func (m *metric) Split(maxSize int) []telegraf.Metric { // if true, then we need to create a metric _not_ including the currently // selected field - if len(m.fields[i:j])+len(fields)+constant > maxSize { + if len(m.fields[i:j])+len(fields)+constant >= maxSize { // if false, then we'll create a metric including the currently // selected field anyways. This means that the given maxSize is too // small for a single field to fit. diff --git a/metric/metric_test.go b/metric/metric_test.go index dd231f8c4..eccb60e63 100644 --- a/metric/metric_test.go +++ b/metric/metric_test.go @@ -10,6 +10,7 @@ import ( "github.com/influxdata/telegraf" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewMetric(t *testing.T) { @@ -458,7 +459,7 @@ func TestSplitMetric(t *testing.T) { assert.Len(t, split70, 3) split60 := m.Split(60) - assert.Len(t, split60, 4) + assert.Len(t, split60, 5) } // test splitting metric into various max lengths @@ -578,6 +579,42 @@ func TestSplitMetric_OneField(t *testing.T) { assert.Equal(t, "cpu,host=localhost float=100001 1480940990034083306\n", split[0].String()) } +func TestSplitMetric_ExactSize(t *testing.T) { + now := time.Unix(0, 1480940990034083306) + tags := map[string]string{ + "host": "localhost", + } + fields := map[string]interface{}{ + "float": float64(100001), + "int": int64(100001), + "bool": true, + "false": false, + "string": "test", + } + m, err := New("cpu", tags, fields, now) + assert.NoError(t, err) + actual := m.Split(m.Len()) + // check that no copy was made + require.Equal(t, &m, &actual[0]) +} + +func TestSplitMetric_NoRoomForNewline(t *testing.T) { + now := time.Unix(0, 1480940990034083306) + tags := map[string]string{ + "host": "localhost", + } + fields := map[string]interface{}{ + "float": float64(100001), + "int": int64(100001), + "bool": true, + "false": false, + } + m, err := New("cpu", tags, fields, now) + assert.NoError(t, err) + actual := m.Split(m.Len() - 1) + require.Equal(t, 2, len(actual)) +} + func TestNewMetricAggregate(t *testing.T) { now := time.Now()