From 13a020f4912cbb7345bf376d855e35a2a3a92d39 Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Wed, 19 Oct 2022 10:15:32 -0600 Subject: [PATCH] fix(inputs.zookeeper): add the ability to parse floats as floats (#12023) --- plugins/inputs/zookeeper/README.md | 6 ++ plugins/inputs/zookeeper/sample.conf | 6 ++ plugins/inputs/zookeeper/zookeeper.go | 30 ++++++--- plugins/inputs/zookeeper/zookeeper_test.go | 71 ++++++++++++++-------- 4 files changed, 80 insertions(+), 33 deletions(-) diff --git a/plugins/inputs/zookeeper/README.md b/plugins/inputs/zookeeper/README.md index a27cf9779..ce52198dd 100644 --- a/plugins/inputs/zookeeper/README.md +++ b/plugins/inputs/zookeeper/README.md @@ -24,6 +24,12 @@ specific to using `mntr` to collect the Java Properties format. ## Timeout for metric collections from all servers. Minimum timeout is "1s". # timeout = "5s" + ## Float Parsing - the initial implementation forced any value unable to be + ## parsed as an int to be a string. Setting this to "float" will attempt to + ## parse float values as floats and not strings. This would break existing + ## metrics and may cause issues if a value switches between a float and int. + # parse_floats = "string" + ## Optional TLS Config # enable_tls = false # tls_ca = "/etc/telegraf/ca.pem" diff --git a/plugins/inputs/zookeeper/sample.conf b/plugins/inputs/zookeeper/sample.conf index 3aa1c5d09..df6f7ab37 100644 --- a/plugins/inputs/zookeeper/sample.conf +++ b/plugins/inputs/zookeeper/sample.conf @@ -10,6 +10,12 @@ ## Timeout for metric collections from all servers. Minimum timeout is "1s". # timeout = "5s" + ## Float Parsing - the initial implementation forced any value unable to be + ## parsed as an int to be a string. Setting this to "float" will attempt to + ## parse float values as floats and not strings. This would break existing + ## metrics and may cause issues if a value switches between a float and int. + # parse_floats = "string" + ## Optional TLS Config # enable_tls = false # tls_ca = "/etc/telegraf/ca.pem" diff --git a/plugins/inputs/zookeeper/zookeeper.go b/plugins/inputs/zookeeper/zookeeper.go index 68f260b28..c7adfafa5 100644 --- a/plugins/inputs/zookeeper/zookeeper.go +++ b/plugins/inputs/zookeeper/zookeeper.go @@ -26,8 +26,9 @@ var zookeeperFormatRE = regexp.MustCompile(`^zk_(\w[\w\.\-]*)\s+([\w\.\-]+)`) // Zookeeper is a zookeeper plugin type Zookeeper struct { - Servers []string - Timeout config.Duration + Servers []string `toml:"servers"` + Timeout config.Duration `toml:"timeout"` + ParseFloats string `toml:"parse_floats"` EnableTLS bool `toml:"enable_tls"` EnableSSL bool `toml:"enable_ssl" deprecated:"1.7.0;use 'enable_tls' instead"` @@ -129,16 +130,29 @@ func (z *Zookeeper) gatherServer(ctx context.Context, address string, acc telegr measurement := strings.TrimPrefix(parts[1], "zk_") if measurement == "server_state" { zookeeperState = parts[2] - } else { - sValue := parts[2] + continue + } - iVal, err := strconv.ParseInt(sValue, 10, 64) + sValue := parts[2] + + // First attempt to parse as an int + iVal, err := strconv.ParseInt(sValue, 10, 64) + if err == nil { + fields[measurement] = iVal + continue + } + + // If set, attempt to parse as a float + if z.ParseFloats == "float" { + fVal, err := strconv.ParseFloat(sValue, 64) if err == nil { - fields[measurement] = iVal - } else { - fields[measurement] = sValue + fields[measurement] = fVal + continue } } + + // Finally, save as a string + fields[measurement] = sValue } srv := "localhost" diff --git a/plugins/inputs/zookeeper/zookeeper_test.go b/plugins/inputs/zookeeper/zookeeper_test.go index 994735358..d2d74168f 100644 --- a/plugins/inputs/zookeeper/zookeeper_test.go +++ b/plugins/inputs/zookeeper/zookeeper_test.go @@ -34,35 +34,56 @@ func TestZookeeperGeneratesMetricsIntegration(t *testing.T) { require.NoError(t, container.Terminate(), "terminating container failed") }() - z := &Zookeeper{ - Servers: []string{ - fmt.Sprintf("%s:%s", container.Address, container.Ports[servicePort]), + var testset = []struct { + name string + zookeeper Zookeeper + }{ + { + name: "floats as strings", + zookeeper: Zookeeper{ + Servers: []string{ + fmt.Sprintf("%s:%s", container.Address, container.Ports[servicePort]), + }, + }, + }, + { + name: "floats as floats", + zookeeper: Zookeeper{ + Servers: []string{ + fmt.Sprintf("%s:%s", container.Address, container.Ports[servicePort]), + }, + ParseFloats: "float", + }, }, } + for _, tt := range testset { + t.Run(tt.name, func(t *testing.T) { + var acc testutil.Accumulator + require.NoError(t, acc.GatherError(tt.zookeeper.Gather)) - var acc testutil.Accumulator + intMetrics := []string{ + "max_latency", + "min_latency", + "packets_received", + "packets_sent", + "outstanding_requests", + "znode_count", + "watch_count", + "ephemerals_count", + "approximate_data_size", + "open_file_descriptor_count", + "max_file_descriptor_count", + } - require.NoError(t, acc.GatherError(z.Gather)) + for _, metric := range intMetrics { + require.True(t, acc.HasInt64Field("zookeeper", metric), metric) + } - intMetrics := []string{ - "max_latency", - "min_latency", - "packets_received", - "packets_sent", - "outstanding_requests", - "znode_count", - "watch_count", - "ephemerals_count", - "approximate_data_size", - "open_file_descriptor_count", - "max_file_descriptor_count", + if tt.zookeeper.ParseFloats == "float" { + require.True(t, acc.HasFloatField("zookeeper", "avg_latency"), "avg_latency not a float") + } else { + require.True(t, acc.HasStringField("zookeeper", "avg_latency"), "avg_latency not a string") + } + }) } - - for _, metric := range intMetrics { - require.True(t, acc.HasInt64Field("zookeeper", metric), metric) - } - - // Currently we output floats as strings (see #8863), but the desired behavior is to have floats - require.True(t, acc.HasStringField("zookeeper", "avg_latency"), "avg_latency") - // require.True(t, acc.HasFloat64Field("zookeeper", "avg_latency"), "avg_latency") }