From 9de6fc72ad4a9017512c00ea72dc5228257d7d2f Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Thu, 20 Jul 2017 13:49:30 -0700 Subject: [PATCH 01/10] Clean up newlines --- samplers/samplers_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/samplers/samplers_test.go b/samplers/samplers_test.go index 07c4805db..3f8baf470 100644 --- a/samplers/samplers_test.go +++ b/samplers/samplers_test.go @@ -11,7 +11,6 @@ import ( ) func TestCounterEmpty(t *testing.T) { - c := NewCounter("a.b.c", []string{"a:b"}) c.Sample(1, 1.0) @@ -32,7 +31,6 @@ func TestCounterEmpty(t *testing.T) { } func TestCounterRate(t *testing.T) { - c := NewCounter("a.b.c", []string{"a:b"}) c.Sample(5, 1.0) @@ -43,7 +41,6 @@ func TestCounterRate(t *testing.T) { } func TestCounterSampleRate(t *testing.T) { - c := NewCounter("a.b.c", []string{"a:b"}) c.Sample(5, 0.5) @@ -80,7 +77,6 @@ func TestCounterMerge(t *testing.T) { } func TestGauge(t *testing.T) { - g := NewGauge("a.b.c", []string{"a:b"}) assert.Equal(t, "a.b.c", g.Name, "Name") @@ -253,7 +249,6 @@ func TestHisto(t *testing.T) { } func TestHistoAvgOnly(t *testing.T) { - h := NewHist("a.b.c", []string{"a:b"}) assert.Equal(t, "a.b.c", h.Name, "Name") @@ -290,7 +285,6 @@ func TestHistoAvgOnly(t *testing.T) { } func TestHistoSampleRate(t *testing.T) { - h := NewHist("a.b.c", []string{"a:b"}) assert.Equal(t, "a.b.c", h.Name, "Name") From f9f01bd650d97de5eaa1be7df59f3db90a45a87e Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Thu, 20 Jul 2017 14:21:21 -0700 Subject: [PATCH 02/10] Change set metric processing in SSF packets --- samplers/parser.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/samplers/parser.go b/samplers/parser.go index bc1abd7e7..09362c0e4 100644 --- a/samplers/parser.go +++ b/samplers/parser.go @@ -72,7 +72,11 @@ func ParseMetricSSF(metric *ssf.SSFSample) (*UDPMetric, error) { return nil, invalidMetricTypeError } h.Write([]byte(ret.Type)) - ret.Value = float64(metric.Value) + if metric.Metric == ssf.SSFSample_SET { + ret.Value = metric.Message + } else { + ret.Value = float64(metric.Value) + } ret.SampleRate = metric.SampleRate tempTags := make([]string, len(metric.Tags)) for key, value := range metric.Tags { From 96b78b415921bb7da1932735bbe2a789eb5b6376 Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Thu, 20 Jul 2017 14:43:57 -0700 Subject: [PATCH 03/10] Fix gofmt --- samplers/samplers_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/samplers/samplers_test.go b/samplers/samplers_test.go index 3f8baf470..fefe59e2e 100644 --- a/samplers/samplers_test.go +++ b/samplers/samplers_test.go @@ -151,7 +151,6 @@ func TestSetMerge(t *testing.T) { } func TestHisto(t *testing.T) { - h := NewHist("a.b.c", []string{"a:b"}) assert.Equal(t, "a.b.c", h.Name, "Name") From 82bdfa958b9885cbdaf612b13d9f6130d24fc266 Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Thu, 20 Jul 2017 17:19:55 -0700 Subject: [PATCH 04/10] Fix bug that made tags array double size --- samplers/parser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samplers/parser.go b/samplers/parser.go index 09362c0e4..38d16a669 100644 --- a/samplers/parser.go +++ b/samplers/parser.go @@ -78,7 +78,7 @@ func ParseMetricSSF(metric *ssf.SSFSample) (*UDPMetric, error) { ret.Value = float64(metric.Value) } ret.SampleRate = metric.SampleRate - tempTags := make([]string, len(metric.Tags)) + tempTags := make([]string, 0) for key, value := range metric.Tags { if key == "veneurlocalonly" { // delete the tag from the list From 2505d1060fd068e271a1944b4ebe1427c28eba11 Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Thu, 20 Jul 2017 17:24:55 -0700 Subject: [PATCH 05/10] Add tests for SSF parsing --- parser_test.go | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/parser_test.go b/parser_test.go index 3db1c771f..a25700997 100644 --- a/parser_test.go +++ b/parser_test.go @@ -3,10 +3,90 @@ package veneur import ( "testing" + "fmt" "github.com/stretchr/testify/assert" "github.com/stripe/veneur/samplers" + "github.com/stripe/veneur/ssf" ) +func freshSSFMetric() *ssf.SSFSample { + return &ssf.SSFSample{ + Metric: 0, + Name: "test.ssf_metric", + Value: 5, + Message: "test_msg", + Status: 0, + SampleRate: 1, + Tags: map[string]string{ + "tag1": "value1", + "tag2": "value2", + }, + } +} + +func TestParserSSF(t *testing.T) { + standardMetric := freshSSFMetric() + m, _ := samplers.ParseMetricSSF(standardMetric) + fmt.Printf("%v\n", m) + assert.NotNil(t, m, "Got nil metric!") + assert.Equal(t, standardMetric.Name, m.Name, "Name") + assert.Equal(t, float64(standardMetric.Value), m.Value, "Value") + assert.Equal(t, "counter", m.Type, "Type") +} + +func TestParserSSFGauge(t *testing.T) { + standardMetric := freshSSFMetric() + standardMetric.Metric = ssf.SSFSample_GAUGE + m, _ := samplers.ParseMetricSSF(standardMetric) + assert.NotNil(t, m, "Got nil metric!") + assert.Equal(t, standardMetric.Name, m.Name, "Name") + assert.Equal(t, float64(standardMetric.Value), m.Value, "Value") + assert.Equal(t, "gauge", m.Type, "Type") +} + +func TestParserSSFSet(t *testing.T) { + standardMetric := freshSSFMetric() + standardMetric.Metric = ssf.SSFSample_SET + m, _ := samplers.ParseMetricSSF(standardMetric) + assert.NotNil(t, m, "Got nil metric!") + assert.Equal(t, standardMetric.Name, m.Name, "Name") + assert.Equal(t, standardMetric.Message, m.Value, "Value") + assert.Equal(t, "set", m.Type, "Type") +} + +func TestParserSSFWithTags(t *testing.T) { + standardMetric := freshSSFMetric() + m, _ := samplers.ParseMetricSSF(standardMetric) + assert.NotNil(t, m, "Got nil metric!") + assert.Equal(t, standardMetric.Name, m.Name, "Name") + assert.Equal(t, float64(standardMetric.Value), m.Value, "Value") + assert.Equal(t, "counter", m.Type, "Type") + assert.Equal(t, 2, len(m.Tags), "# of Tags") +} + +func TestParserSSFWithSampleRate(t *testing.T) { + standardMetric := freshSSFMetric() + standardMetric.SampleRate = 0.1 + m, _ := samplers.ParseMetricSSF(standardMetric) + assert.NotNil(t, m, "Got nil metric!") + assert.Equal(t, standardMetric.Name, m.Name, "Name") + assert.Equal(t, float64(standardMetric.Value), m.Value, "Value") + assert.Equal(t, "counter", m.Type, "Type") + assert.Equal(t, float32(0.1), m.SampleRate, "Sample Rate") +} + +func TestParserSSFWithSampleRateAndTags(t *testing.T) { + standardMetric := freshSSFMetric() + standardMetric.SampleRate = 0.1 + m, _ := samplers.ParseMetricSSF(standardMetric) + assert.NotNil(t, m, "Got nil metric!") + assert.Equal(t, standardMetric.Name, m.Name, "Name") + assert.Equal(t, float64(standardMetric.Value), m.Value, "Value") + assert.Equal(t, "counter", m.Type, "Type") + assert.Equal(t, float32(0.1), m.SampleRate, "Sample Rate") + assert.Len(t, m.Tags, 2, "Tags") +} + func TestParser(t *testing.T) { m, _ := samplers.ParseMetric([]byte("a.b.c:1|c")) assert.NotNil(t, m, "Got nil metric!") From fbf6ec3ff5f18a4d37cb90d8e4ef98bf99393e7a Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Thu, 20 Jul 2017 17:46:00 -0700 Subject: [PATCH 06/10] Remove debug prints --- parser_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/parser_test.go b/parser_test.go index a25700997..c78491bdd 100644 --- a/parser_test.go +++ b/parser_test.go @@ -3,7 +3,6 @@ package veneur import ( "testing" - "fmt" "github.com/stretchr/testify/assert" "github.com/stripe/veneur/samplers" "github.com/stripe/veneur/ssf" @@ -27,7 +26,6 @@ func freshSSFMetric() *ssf.SSFSample { func TestParserSSF(t *testing.T) { standardMetric := freshSSFMetric() m, _ := samplers.ParseMetricSSF(standardMetric) - fmt.Printf("%v\n", m) assert.NotNil(t, m, "Got nil metric!") assert.Equal(t, standardMetric.Name, m.Name, "Name") assert.Equal(t, float64(standardMetric.Value), m.Value, "Value") From 15e95d8b6b83b1e25c888e7bfac2ee34e247a856 Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Fri, 21 Jul 2017 15:35:34 -0700 Subject: [PATCH 07/10] Fix empty tag bug --- samplers/parser.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/samplers/parser.go b/samplers/parser.go index 38d16a669..e915ec5f8 100644 --- a/samplers/parser.go +++ b/samplers/parser.go @@ -92,6 +92,9 @@ func ParseMetricSSF(metric *ssf.SSFSample) (*UDPMetric, error) { tempTags = append(tempTags, fmt.Sprintf("%s:%s", key, value)) } } + if len(tempTags) == 0 { + tempTags = append(tempTags, ":") + } sort.Strings(tempTags) ret.Tags = tempTags ret.JoinedTags = strings.Join(tempTags, ",") From df903b50fa3d82169a2574d6f1cc64c5671d5b1d Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Fri, 21 Jul 2017 15:38:00 -0700 Subject: [PATCH 08/10] Add ValidMetric; and metrics for invalid packets (both spans and metrics). --- server.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/server.go b/server.go index a1e5f5c42..6d7add89a 100644 --- a/server.go +++ b/server.go @@ -547,6 +547,17 @@ func ValidTrace(sample ssf.SSFSpan) bool { return ret } +// ValidMetric takes in an SSF sample and determines if it is valid or not. +func ValidMetric(sample samplers.UDPMetric) bool { + ret := true + ret = ret && sample.Name != "" + ret = ret && sample.Value != nil + if sample.SampleRate == 0 { + sample.SampleRate = 1 + } + return ret +} + // HandleTracePacket accepts an incoming packet as bytes and sends it to the // appropriate worker. func (s *Server) HandleTracePacket(packet []byte) { @@ -577,11 +588,17 @@ func (s *Server) HandleTracePacket(packet []byte) { s.Statsd.Count("packet.error_total", 1, []string{"packet_type:ssf_metric", "reason:parse"}, 1.0) return } - s.Workers[metric.Digest%uint32(len(s.Workers))].PacketChan <- *metric + if ValidMetric(*metric) { + s.Workers[metric.Digest%uint32(len(s.Workers))].PacketChan <- *metric + } else { + s.Statsd.Count("packet.error_total", 1, []string{"packet_type:ssf_metric", "reason:not_valid"}, 1.0) + } } if ValidTrace(*newSample) { s.Statsd.Incr("packet.spans.received_total", nil, .1) s.TraceWorker.TraceChan <- *newSample + } else { + s.Statsd.Count("packet.error_total", 1, []string{"packet_type:ssf_span", "reason:not_valid"}, 1.0) } } From c9016f3a9cf9cf264f97d3fd690d7f7862e236e9 Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Mon, 24 Jul 2017 09:50:12 -0700 Subject: [PATCH 09/10] Removed bug fix for nonexistent bug --- samplers/parser.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/samplers/parser.go b/samplers/parser.go index e915ec5f8..38d16a669 100644 --- a/samplers/parser.go +++ b/samplers/parser.go @@ -92,9 +92,6 @@ func ParseMetricSSF(metric *ssf.SSFSample) (*UDPMetric, error) { tempTags = append(tempTags, fmt.Sprintf("%s:%s", key, value)) } } - if len(tempTags) == 0 { - tempTags = append(tempTags, ":") - } sort.Strings(tempTags) ret.Tags = tempTags ret.JoinedTags = strings.Join(tempTags, ",") From bfe2fe690181535880058e1dcfc4295584629a5a Mon Sep 17 00:00:00 2001 From: Yasha Mostofi Date: Mon, 24 Jul 2017 10:51:18 -0700 Subject: [PATCH 10/10] Add 1.5.2 pending changelog --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e66a3060..7e0a2b437 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +# 1.5.2, pending + +## Bugfixes +* Correctly parse `Set` metrics if sent via SSF. Thanks [redsn0w422](https://github.com/redsn0w422)! +* Return correct array of tags after parsing an SSF metric. Thanks [redsn0w422](https://github.com/redsn0w422)! + +## Improvements +* Added tests for `parseMetricSSF`. Thanks [redsn0w422](https://github.com/redsn0w422)! + # 1.5.1, 2017-07-18 ## Bugfixes