Deprecate statsd convert_names option, expose separator

closes #876
2016-03-24 16:53:26 -06:00 · 2016-03-24 16:53:26 -06:00 · b1cfb1afe4
parent 2f215356d6
commit b1cfb1afe4
3 changed files with 63 additions and 49 deletions
--- 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.
--- 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 <bucket>:<value>|<mtype>|@<samplerate>
 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)
--- 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