From 798e8d0f9f4507dfa331d052885c99d7afaddf6c Mon Sep 17 00:00:00 2001 From: Cameron Sparr Date: Thu, 24 Mar 2016 16:53:26 -0600 Subject: [PATCH] Deprecate statsd convert_names option, expose separator closes #876 --- CHANGELOG.md | 1 + plugins/inputs/statsd/statsd.go | 36 ++++++------- plugins/inputs/statsd/statsd_test.go | 75 +++++++++++++++++----------- 3 files changed, 63 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94d327fbd..b1c58e60e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - [#789](https://github.com/influxdata/telegraf/pull/789): Support multiple field specification and `field*` in graphite templates. Thanks @chrusty! - [#762](https://github.com/influxdata/telegraf/pull/762): Nagios parser for the exec plugin. Thanks @titilambert! - [#848](https://github.com/influxdata/telegraf/issues/848): Provide option to omit host tag from telegraf agent. +- [#928](https://github.com/influxdata/telegraf/pull/928): Deprecating the statsd "convert_names" options, expose separator config. ### Bugfixes - [#890](https://github.com/influxdata/telegraf/issues/890): Create TLS config even if only ssl_ca is provided. diff --git a/plugins/inputs/statsd/statsd.go b/plugins/inputs/statsd/statsd.go index 0e7a911e1..d31e6bfc9 100644 --- a/plugins/inputs/statsd/statsd.go +++ b/plugins/inputs/statsd/statsd.go @@ -21,6 +21,8 @@ const ( UDP_PACKET_SIZE int = 1500 defaultFieldName = "value" + + defaultSeparator = "_" ) var dropwarn = "ERROR: Message queue full. Discarding line [%s] " + @@ -47,6 +49,8 @@ type Statsd struct { DeleteTimings bool ConvertNames bool + // MetricSeparator is the separator between parts of the metric name. + MetricSeparator string // This flag enables parsing of tags in the dogstatsd extention to the // statsd protocol (http://docs.datadoghq.com/guides/dogstatsd/) ParseDataDogTags bool @@ -76,23 +80,6 @@ type Statsd struct { listener *net.UDPConn } -func NewStatsd() *Statsd { - s := Statsd{} - - // Make data structures - s.done = make(chan struct{}) - s.in = make(chan []byte, s.AllowedPendingMessages) - s.gauges = make(map[string]cachedgauge) - s.counters = make(map[string]cachedcounter) - s.sets = make(map[string]cachedset) - s.timings = make(map[string]cachedtimings) - - s.ConvertNames = true - s.UDPPacketSize = UDP_PACKET_SIZE - - return &s -} - // One statsd metric, form is :||@ type metric struct { name string @@ -149,8 +136,8 @@ const sampleConfig = ` ## Percentiles to calculate for timing & histogram stats percentiles = [90] - ## convert measurement names, "." to "_" and "-" to "__" - convert_names = true + ## separator to use between elements of a statsd metric + metric_separator = "_" ## Parses tags in the datadog statsd format ## http://docs.datadoghq.com/guides/dogstatsd/ @@ -257,6 +244,15 @@ func (s *Statsd) Start(_ telegraf.Accumulator) error { s.timings = prevInstance.timings } + if s.ConvertNames { + log.Printf("WARNING statsd: convert_names config option is deprecated," + + " please use metric_separator instead") + } + + if s.MetricSeparator == "" { + s.MetricSeparator = defaultSeparator + } + s.wg.Add(2) // Start the UDP listener go s.udpListen() @@ -500,7 +496,7 @@ func (s *Statsd) parseName(bucket string) (string, string, map[string]string) { var field string name := bucketparts[0] - p, err := graphite.NewGraphiteParser(".", s.Templates, nil) + p, err := graphite.NewGraphiteParser(s.MetricSeparator, s.Templates, nil) if err == nil { p.DefaultTags = tags name, tags, field, _ = p.ApplyTemplate(name) diff --git a/plugins/inputs/statsd/statsd_test.go b/plugins/inputs/statsd/statsd_test.go index 5dffdc9cd..743e80135 100644 --- a/plugins/inputs/statsd/statsd_test.go +++ b/plugins/inputs/statsd/statsd_test.go @@ -8,9 +8,26 @@ import ( "github.com/influxdata/telegraf/testutil" ) +func NewTestStatsd() *Statsd { + s := Statsd{} + + // Make data structures + s.done = make(chan struct{}) + s.in = make(chan []byte, s.AllowedPendingMessages) + s.gauges = make(map[string]cachedgauge) + s.counters = make(map[string]cachedcounter) + s.sets = make(map[string]cachedset) + s.timings = make(map[string]cachedtimings) + + s.MetricSeparator = "_" + s.UDPPacketSize = UDP_PACKET_SIZE + + return &s +} + // Invalid lines should return an error func TestParse_InvalidLines(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() invalid_lines := []string{ "i.dont.have.a.pipe:45g", "i.dont.have.a.colon45|c", @@ -34,7 +51,7 @@ func TestParse_InvalidLines(t *testing.T) { // Invalid sample rates should be ignored and not applied func TestParse_InvalidSampleRate(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() invalid_lines := []string{ "invalid.sample.rate:45|c|0.1", "invalid.sample.rate.2:45|c|@foo", @@ -84,9 +101,9 @@ func TestParse_InvalidSampleRate(t *testing.T) { } } -// Names should be parsed like . -> _ and - -> __ +// Names should be parsed like . -> _ func TestParse_DefaultNameParsing(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() valid_lines := []string{ "valid:1|c", "valid.foo-bar:11|c", @@ -108,7 +125,7 @@ func TestParse_DefaultNameParsing(t *testing.T) { 1, }, { - "valid_foo__bar", + "valid_foo-bar", 11, }, } @@ -123,7 +140,7 @@ func TestParse_DefaultNameParsing(t *testing.T) { // Test that template name transformation works func TestParse_Template(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.Templates = []string{ "measurement.measurement.host.service", } @@ -165,7 +182,7 @@ func TestParse_Template(t *testing.T) { // Test that template filters properly func TestParse_TemplateFilter(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.Templates = []string{ "cpu.idle.* measurement.measurement.host", } @@ -207,7 +224,7 @@ func TestParse_TemplateFilter(t *testing.T) { // Test that most specific template is chosen func TestParse_TemplateSpecificity(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.Templates = []string{ "cpu.* measurement.foo.host", "cpu.idle.* measurement.measurement.host", @@ -245,7 +262,7 @@ func TestParse_TemplateSpecificity(t *testing.T) { // Test that most specific template is chosen func TestParse_TemplateFields(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.Templates = []string{ "* measurement.measurement.field", } @@ -359,7 +376,7 @@ func TestParse_Fields(t *testing.T) { // Test that tags within the bucket are parsed correctly func TestParse_Tags(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() tests := []struct { bucket string @@ -412,7 +429,7 @@ func TestParse_Tags(t *testing.T) { // Test that DataDog tags are parsed func TestParse_DataDogTags(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.ParseDataDogTags = true lines := []string{ @@ -490,7 +507,7 @@ func tagsForItem(m interface{}) map[string]string { // Test that statsd buckets are parsed to measurement names properly func TestParseName(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() tests := []struct { in_name string @@ -506,7 +523,7 @@ func TestParseName(t *testing.T) { }, { "foo.bar-baz", - "foo_bar__baz", + "foo_bar-baz", }, } @@ -517,8 +534,8 @@ func TestParseName(t *testing.T) { } } - // Test with ConvertNames = false - s.ConvertNames = false + // Test with separator == "." + s.MetricSeparator = "." tests = []struct { in_name string @@ -549,7 +566,7 @@ func TestParseName(t *testing.T) { // Test that measurements with the same name, but different tags, are treated // as different outputs func TestParse_MeasurementsWithSameName(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() // Test that counters work valid_lines := []string{ @@ -607,8 +624,8 @@ func TestParse_MeasurementsWithMultipleValues(t *testing.T) { "valid.multiple.mixed:1|c:1|ms:2|s:1|g", } - s_single := NewStatsd() - s_multiple := NewStatsd() + s_single := NewTestStatsd() + s_multiple := NewTestStatsd() for _, line := range single_lines { err := s_single.parseStatsdLine(line) @@ -701,7 +718,7 @@ func TestParse_MeasurementsWithMultipleValues(t *testing.T) { // Valid lines should be parsed and their values should be cached func TestParse_ValidLines(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() valid_lines := []string{ "valid:45|c", "valid:45|s", @@ -720,7 +737,7 @@ func TestParse_ValidLines(t *testing.T) { // Tests low-level functionality of gauges func TestParse_Gauges(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() // Test that gauge +- values work valid_lines := []string{ @@ -786,7 +803,7 @@ func TestParse_Gauges(t *testing.T) { // Tests low-level functionality of sets func TestParse_Sets(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() // Test that sets work valid_lines := []string{ @@ -834,7 +851,7 @@ func TestParse_Sets(t *testing.T) { // Tests low-level functionality of counters func TestParse_Counters(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() // Test that counters work valid_lines := []string{ @@ -888,7 +905,7 @@ func TestParse_Counters(t *testing.T) { // Tests low-level functionality of timings func TestParse_Timings(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.Percentiles = []int{90} acc := &testutil.Accumulator{} @@ -925,7 +942,7 @@ func TestParse_Timings(t *testing.T) { // Tests low-level functionality of timings when multiple fields is enabled // and a measurement template has been defined which can parse field names func TestParse_Timings_MultipleFieldsWithTemplate(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.Templates = []string{"measurement.field"} s.Percentiles = []int{90} acc := &testutil.Accumulator{} @@ -974,7 +991,7 @@ func TestParse_Timings_MultipleFieldsWithTemplate(t *testing.T) { // but a measurement template hasn't been defined so we can't parse field names // In this case the behaviour should be the same as normal behaviour func TestParse_Timings_MultipleFieldsWithoutTemplate(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.Templates = []string{} s.Percentiles = []int{90} acc := &testutil.Accumulator{} @@ -1022,7 +1039,7 @@ func TestParse_Timings_MultipleFieldsWithoutTemplate(t *testing.T) { } func TestParse_Timings_Delete(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.DeleteTimings = true fakeacc := &testutil.Accumulator{} var err error @@ -1046,7 +1063,7 @@ func TestParse_Timings_Delete(t *testing.T) { // Tests the delete_gauges option func TestParse_Gauges_Delete(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.DeleteGauges = true fakeacc := &testutil.Accumulator{} var err error @@ -1072,7 +1089,7 @@ func TestParse_Gauges_Delete(t *testing.T) { // Tests the delete_sets option func TestParse_Sets_Delete(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.DeleteSets = true fakeacc := &testutil.Accumulator{} var err error @@ -1098,7 +1115,7 @@ func TestParse_Sets_Delete(t *testing.T) { // Tests the delete_counters option func TestParse_Counters_Delete(t *testing.T) { - s := NewStatsd() + s := NewTestStatsd() s.DeleteCounters = true fakeacc := &testutil.Accumulator{} var err error