fix(agent): handle float time with fractions of seconds correctly (#12491)

config/types.go

@@ -2,7 +2,6 @@ package config
 
 import (
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/alecthomas/units"
@@ -15,7 +14,7 @@ type Duration time.Duration
 type Size int64
 
 // UnmarshalTOML parses the duration from the TOML config file
-func (d *Duration) UnmarshalTOML(b []byte) error {
+func (d *Duration) UnmarshalText(b []byte) error {
 	// convert to string
 	durStr := string(b)
 
@@ -30,14 +29,12 @@ func (d *Duration) UnmarshalTOML(b []byte) error {
 	// Second try parsing as float seconds
 	sF, err := strconv.ParseFloat(durStr, 64)
 	if err == nil {
-		dur := time.Second * time.Duration(sF)
+		dur := float64(time.Second) * sF
 		*d = Duration(dur)
 		return nil
 	}
 
 	// Finally, try value is a TOML string (e.g. "3s", 3s) or literal (e.g. '3s')
-	durStr = strings.ReplaceAll(durStr, "'", "")
-	durStr = strings.ReplaceAll(durStr, "\"", "")
 	if durStr == "" {
 		durStr = "0s"
 	}
@@ -57,23 +54,12 @@ func (d *Duration) UnmarshalTOML(b []byte) error {
 	return nil
 }
 
-func (d *Duration) UnmarshalText(text []byte) error {
+func (s *Size) UnmarshalText(b []byte) error {
-	return d.UnmarshalTOML(text)
-}
-
-func (s *Size) UnmarshalTOML(b []byte) error {
-	var err error
 	if len(b) == 0 {
 		return nil
 	}
-	str := string(b)
-	if b[0] == '"' || b[0] == '\'' {
-		str, err = strconv.Unquote(str)
-		if err != nil {
-			return err
-		}
-	}
 
+	str := string(b)
 	val, err := strconv.ParseInt(str, 10, 64)
 	if err == nil {
 		*s = Size(val)
@@ -86,7 +72,3 @@ func (s *Size) UnmarshalTOML(b []byte) error {
 	*s = Size(val)
 	return nil
 }
-
-func (s *Size) UnmarshalText(text []byte) error {
-	return s.UnmarshalTOML(text)
-}

config/types_test.go

@@ -4,9 +4,12 @@ import (
 	"testing"
 	"time"
 
-	"github.com/influxdata/telegraf/config"
-	"github.com/influxdata/telegraf/plugins/processors/reverse_dns"
 	"github.com/stretchr/testify/require"
+
+	"github.com/influxdata/telegraf"
+	"github.com/influxdata/telegraf/config"
+	"github.com/influxdata/telegraf/plugins/inputs"
+	"github.com/influxdata/telegraf/plugins/processors/reverse_dns"
 )
 
 func TestConfigDuration(t *testing.T) {
@@ -33,58 +36,232 @@ func TestConfigDuration(t *testing.T) {
 func TestDuration(t *testing.T) {
 	var d config.Duration
 
-	require.NoError(t, d.UnmarshalTOML([]byte(`"1s"`)))
+	d = config.Duration(0)
+	require.NoError(t, d.UnmarshalText([]byte(`1s`)))
 	require.Equal(t, time.Second, time.Duration(d))
 
 	d = config.Duration(0)
-	require.NoError(t, d.UnmarshalTOML([]byte(`1s`)))
+	require.NoError(t, d.UnmarshalText([]byte(`10`)))
-	require.Equal(t, time.Second, time.Duration(d))
-
-	d = config.Duration(0)
-	require.NoError(t, d.UnmarshalTOML([]byte(`'1s'`)))
-	require.Equal(t, time.Second, time.Duration(d))
-
-	d = config.Duration(0)
-	require.NoError(t, d.UnmarshalTOML([]byte(`10`)))
 	require.Equal(t, 10*time.Second, time.Duration(d))
 
 	d = config.Duration(0)
-	require.NoError(t, d.UnmarshalTOML([]byte(`1.5`)))
+	require.NoError(t, d.UnmarshalText([]byte(`1.5`)))
-	require.Equal(t, time.Second, time.Duration(d))
+	require.Equal(t, 1500*time.Millisecond, time.Duration(d))
 
 	d = config.Duration(0)
-	require.NoError(t, d.UnmarshalTOML([]byte(``)))
+	require.NoError(t, d.UnmarshalText([]byte(``)))
 	require.Equal(t, 0*time.Second, time.Duration(d))
 
-	d = config.Duration(0)
+	require.Error(t, d.UnmarshalText([]byte(`"1"`)))  // string missing unit
-	require.NoError(t, d.UnmarshalTOML([]byte(`""`)))
+	require.Error(t, d.UnmarshalText([]byte(`'2'`)))  // string missing unit
-	require.Equal(t, 0*time.Second, time.Duration(d))
+	require.Error(t, d.UnmarshalText([]byte(`'ns'`))) // string missing time
-
+	require.Error(t, d.UnmarshalText([]byte(`'us'`))) // string missing time
-	require.Error(t, d.UnmarshalTOML([]byte(`"1"`)))  // string missing unit
-	require.Error(t, d.UnmarshalTOML([]byte(`'2'`)))  // string missing unit
-	require.Error(t, d.UnmarshalTOML([]byte(`'ns'`))) // string missing time
-	require.Error(t, d.UnmarshalTOML([]byte(`'us'`))) // string missing time
 }
 
 func TestSize(t *testing.T) {
 	var s config.Size
 
-	require.NoError(t, s.UnmarshalTOML([]byte(`"1B"`)))
+	require.NoError(t, s.UnmarshalText([]byte(`1B`)))
 	require.Equal(t, int64(1), int64(s))
 
 	s = config.Size(0)
-	require.NoError(t, s.UnmarshalTOML([]byte(`1`)))
+	require.NoError(t, s.UnmarshalText([]byte(`1`)))
 	require.Equal(t, int64(1), int64(s))
 
 	s = config.Size(0)
-	require.NoError(t, s.UnmarshalTOML([]byte(`'1'`)))
+	require.NoError(t, s.UnmarshalText([]byte(`1GB`)))
-	require.Equal(t, int64(1), int64(s))
-
-	s = config.Size(0)
-	require.NoError(t, s.UnmarshalTOML([]byte(`"1GB"`)))
 	require.Equal(t, int64(1000*1000*1000), int64(s))
 
 	s = config.Size(0)
-	require.NoError(t, s.UnmarshalTOML([]byte(`"12GiB"`)))
+	require.NoError(t, s.UnmarshalText([]byte(`12GiB`)))
 	require.Equal(t, int64(12*1024*1024*1024), int64(s))
 }
+
+func TestTOMLParsingStringDurations(t *testing.T) {
+	cfg := []byte(`
+[[inputs.typesmockup]]
+	durations = [
+		"1s",
+		'''1s''',
+		'1s',
+		"1.5s",
+		"",
+		'',
+		"2h",
+		"42m",
+		"100ms",
+		"100us",
+		"100ns"
+	]
+`)
+
+	expected := []time.Duration{
+		1 * time.Second,
+		1 * time.Second,
+		1 * time.Second,
+		1500 * time.Millisecond,
+		0,
+		0,
+		2 * time.Hour,
+		42 * time.Minute,
+		100 * time.Millisecond,
+		100 * time.Microsecond,
+		100 * time.Nanosecond,
+	}
+
+	// Load the data
+	c := config.NewConfig()
+	err := c.LoadConfigData(cfg)
+	require.NoError(t, err)
+	require.Len(t, c.Inputs, 1)
+	plugin := c.Inputs[0].Input.(*MockupTypesPlugin)
+
+	require.Empty(t, plugin.Sizes)
+	require.Len(t, plugin.Durations, len(expected))
+	for i, actual := range plugin.Durations {
+		require.EqualValuesf(t, expected[i], actual, "case %d failed", i)
+	}
+}
+
+func TestTOMLParsingIntegerDurations(t *testing.T) {
+	cfg := []byte(`
+[[inputs.typesmockup]]
+	durations = [
+		1,
+		10,
+		3601
+	]
+`)
+
+	expected := []time.Duration{
+		1 * time.Second,
+		10 * time.Second,
+		3601 * time.Second,
+	}
+
+	// Load the data
+	c := config.NewConfig()
+	err := c.LoadConfigData(cfg)
+	require.NoError(t, err)
+	require.Len(t, c.Inputs, 1)
+	plugin := c.Inputs[0].Input.(*MockupTypesPlugin)
+
+	require.Empty(t, plugin.Sizes)
+	require.Len(t, plugin.Durations, len(expected))
+	for i, actual := range plugin.Durations {
+		require.EqualValuesf(t, expected[i], actual, "case %d failed", i)
+	}
+}
+
+func TestTOMLParsingFloatDurations(t *testing.T) {
+	cfg := []byte(`
+[[inputs.typesmockup]]
+	durations = [
+		42.0,
+		1.5
+	]
+`)
+
+	expected := []time.Duration{
+		42 * time.Second,
+		1500 * time.Millisecond,
+	}
+
+	// Load the data
+	c := config.NewConfig()
+	err := c.LoadConfigData(cfg)
+	require.NoError(t, err)
+	require.Len(t, c.Inputs, 1)
+	plugin := c.Inputs[0].Input.(*MockupTypesPlugin)
+
+	require.Empty(t, plugin.Sizes)
+	require.Len(t, plugin.Durations, len(expected))
+	for i, actual := range plugin.Durations {
+		require.EqualValuesf(t, expected[i], actual, "case %d failed", i)
+	}
+}
+
+func TestTOMLParsingStringSizes(t *testing.T) {
+	cfg := []byte(`
+[[inputs.typesmockup]]
+	sizes = [
+		"1B",
+		"1",
+		'1',
+		'''15kB''',
+		"""15KiB""",
+		"1GB",
+		"12GiB"
+	]
+`)
+
+	expected := []int64{
+		1,
+		1,
+		1,
+		15 * 1000,
+		15 * 1024,
+		1000 * 1000 * 1000,
+		12 * 1024 * 1024 * 1024,
+	}
+
+	// Load the data
+	c := config.NewConfig()
+	err := c.LoadConfigData(cfg)
+	require.NoError(t, err)
+	require.Len(t, c.Inputs, 1)
+	plugin := c.Inputs[0].Input.(*MockupTypesPlugin)
+
+	require.Empty(t, plugin.Durations)
+	require.Len(t, plugin.Sizes, len(expected))
+	for i, actual := range plugin.Sizes {
+		require.EqualValuesf(t, expected[i], actual, "case %d failed", i)
+	}
+}
+
+func TestTOMLParsingIntegerSizes(t *testing.T) {
+	cfg := []byte(`
+[[inputs.typesmockup]]
+	sizes = [
+		0,
+		1,
+		1000,
+		1024
+	]
+`)
+
+	expected := []int64{
+		0,
+		1,
+		1000,
+		1024,
+	}
+
+	// Load the data
+	c := config.NewConfig()
+	err := c.LoadConfigData(cfg)
+	require.NoError(t, err)
+	require.Len(t, c.Inputs, 1)
+	plugin := c.Inputs[0].Input.(*MockupTypesPlugin)
+
+	require.Empty(t, plugin.Durations)
+	require.Len(t, plugin.Sizes, len(expected))
+	for i, actual := range plugin.Sizes {
+		require.EqualValuesf(t, expected[i], actual, "case %d failed", i)
+	}
+}
+
+/*** Mockup (input) plugin for testing to avoid cyclic dependencies ***/
+type MockupTypesPlugin struct {
+	Durations []config.Duration `toml:"durations"`
+	Sizes     []config.Size     `toml:"sizes"`
+}
+
+func (*MockupTypesPlugin) SampleConfig() string                { return "Mockup test types plugin" }
+func (*MockupTypesPlugin) Gather(_ telegraf.Accumulator) error { return nil }
+
+// Register the mockup plugin on loading
+func init() {
+	// Register the mockup input plugin for the required names
+	inputs.Add("typesmockup", func() telegraf.Input { return &MockupTypesPlugin{} })
+}