From 4114e409612249a2ff18b1ca54f0131178a7606f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20K=C3=A4stle?= Date: Mon, 17 Jun 2024 14:28:05 +0200 Subject: [PATCH 1/6] Introduce error returns for perfdata formatting This patch changes the function signature of Performance data formatting functions to return errors. This is a preparation step to allow the rejection of certain values, which are valid values for that data type, but not valid as measurements --- perfdata/list.go | 10 ++++++-- perfdata/type.go | 34 +++++++++++++++++++-------- perfdata/type_test.go | 54 +++++++++++++++++++++++++++++++++++-------- 3 files changed, 78 insertions(+), 20 deletions(-) diff --git a/perfdata/list.go b/perfdata/list.go index cca58a0..70e9716 100644 --- a/perfdata/list.go +++ b/perfdata/list.go @@ -13,8 +13,14 @@ func (l PerfdataList) String() string { var out strings.Builder for _, p := range l { - out.WriteString(" ") - out.WriteString(p.String()) + + pfDataString, err := p.String() + + // Ignore perfdata points which fail to format + if err == nil { + out.WriteString(" ") + out.WriteString(pfDataString) + } } return strings.Trim(out.String(), " ") diff --git a/perfdata/type.go b/perfdata/type.go index e5f2563..4436b36 100644 --- a/perfdata/type.go +++ b/perfdata/type.go @@ -13,16 +13,20 @@ var replacer = strings.NewReplacer("=", "_", "`", "_", "'", "_", "\"", "_") // formatNumeric returns a string representation of various possible numerics // // This supports most internal types of Go and all fmt.Stringer interfaces. -func formatNumeric(value interface{}) string { +// Returns an eror in some known cases where the value of a data type does not +// represent a valid measurement, e.g INF for floats +// This error can probably ignored in most cases and the perfdata point omitted, +// but silently dropping the value and returning the empty strings seems like bad style +func formatNumeric(value interface{}) (string, error) { switch v := value.(type) { case float64: - return check.FormatFloat(v) + return check.FormatFloat(v), nil case float32: - return check.FormatFloat(float64(v)) + return check.FormatFloat(float64(v)), nil case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64: - return fmt.Sprintf("%d", v) + return fmt.Sprintf("%d", v), nil case fmt.Stringer, string: - return fmt.Sprint(v) + return fmt.Sprint(v), nil default: panic(fmt.Sprintf("unsupported type for perfdata: %T", value)) } @@ -51,7 +55,10 @@ type Perfdata struct { } // String returns the proper format for the plugin output -func (p Perfdata) String() string { +// Returns an eror in some known cases where the value of a data type does not +// represent a valid measurement, see the explanation for "formatNumeric" for +// perfdata values. +func (p Perfdata) String() (string, error) { var sb strings.Builder // Add quotes if string contains any whitespace @@ -61,7 +68,12 @@ func (p Perfdata) String() string { sb.WriteString(replacer.Replace(p.Label) + "=") } - sb.WriteString(formatNumeric(p.Value)) + pfVal, err := formatNumeric(p.Value) + if err != nil { + return "", err + } + + sb.WriteString(pfVal) sb.WriteString(p.Uom) // Thresholds @@ -78,9 +90,13 @@ func (p Perfdata) String() string { sb.WriteString(";") if value != nil { - sb.WriteString(formatNumeric(value)) + pfVal, err := formatNumeric(value) + // Attention: we ignore limits if they are faulty + if err == nil { + sb.WriteString(pfVal) + } } } - return strings.TrimRight(sb.String(), ";") + return strings.TrimRight(sb.String(), ";"), nil } diff --git a/perfdata/type_test.go b/perfdata/type_test.go index 17dbe7d..4fdcfcc 100644 --- a/perfdata/type_test.go +++ b/perfdata/type_test.go @@ -20,8 +20,9 @@ func BenchmarkPerfdataString(b *testing.B) { Max: 100} for i := 0; i < b.N; i++ { - p := perf.String() + p, err := perf.String() _ = p + _ = err } } @@ -73,17 +74,52 @@ func TestRenderPerfdata(t *testing.T) { for name, tc := range testcases { t.Run(name, func(t *testing.T) { - assert.Equal(t, tc.expected, tc.perf.String()) + pfVal, err := tc.perf.String() + assert.NoError(t, err) + assert.Equal(t, tc.expected, pfVal) }) } } +type pfFormatTest struct { + Result string + InputValue interface{} +} + func TestFormatNumeric(t *testing.T) { - assert.Equal(t, "10", formatNumeric(10)) - assert.Equal(t, "-10", formatNumeric(-10)) - assert.Equal(t, "10", formatNumeric(uint8(10))) - assert.Equal(t, "1234.567", formatNumeric(1234.567)) - assert.Equal(t, "1234.567", formatNumeric(float32(1234.567))) - assert.Equal(t, "1234.567", formatNumeric("1234.567")) - assert.Equal(t, "1234567890.988", formatNumeric(1234567890.9877)) + testdata := []pfFormatTest{ + { + Result: "10", + InputValue: 10, + }, + { + Result: "-10", + InputValue: -10, + }, + { + Result: "10", + InputValue: uint8(10), + }, + { + Result: "1234.567", + InputValue: 1234.567, + }, + { + Result: "1234.567", + InputValue: float32(1234.567), + }, + {Result: "1234.567", + InputValue: "1234.567", + }, + { + Result: "1234567890.988", + InputValue: 1234567890.9877, + }, + } + + for _, val := range testdata { + formatted, err := formatNumeric(val.InputValue) + assert.NoError(t, err) + assert.Equal(t, val.Result, formatted) + } } From a3834b83a0e824f53073147ffe9fbf896f9a88a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20K=C3=A4stle?= Date: Mon, 17 Jun 2024 14:36:47 +0200 Subject: [PATCH 2/6] Return errors for invalid performance data values --- perfdata/type.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/perfdata/type.go b/perfdata/type.go index 4436b36..06d2fd2 100644 --- a/perfdata/type.go +++ b/perfdata/type.go @@ -2,6 +2,7 @@ package perfdata import ( "fmt" + "math" "strings" "github.com/NETWAYS/go-check" @@ -10,6 +11,20 @@ import ( // Replace not allowed characters inside a label var replacer = strings.NewReplacer("=", "_", "`", "_", "'", "_", "\"", "_") +type InfValueError struct { +} + +func (i InfValueError) Error() string { + return "Performance data value is infinite" +} + +type NanValueError struct { +} + +func (i NanValueError) Error() string { + return "Performance data value is NaN (not a number)" +} + // formatNumeric returns a string representation of various possible numerics // // This supports most internal types of Go and all fmt.Stringer interfaces. @@ -20,6 +35,14 @@ var replacer = strings.NewReplacer("=", "_", "`", "_", "'", "_", "\"", "_") func formatNumeric(value interface{}) (string, error) { switch v := value.(type) { case float64: + if math.IsInf(v, 0) { + return "", InfValueError{} + } + + if math.IsNaN(v) { + return "", NanValueError{} + } + return check.FormatFloat(v), nil case float32: return check.FormatFloat(float64(v)), nil From 62f377cfbfa13d8012a1aa726ce1f45d3e3600d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20K=C3=A4stle?= Date: Mon, 17 Jun 2024 17:04:59 +0200 Subject: [PATCH 3/6] Make linter happy --- perfdata/list.go | 1 - 1 file changed, 1 deletion(-) diff --git a/perfdata/list.go b/perfdata/list.go index 70e9716..28fadc8 100644 --- a/perfdata/list.go +++ b/perfdata/list.go @@ -13,7 +13,6 @@ func (l PerfdataList) String() string { var out strings.Builder for _, p := range l { - pfDataString, err := p.String() // Ignore perfdata points which fail to format From 00d7d3533dea090bdd82c410e5c0e5288b34b53a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20K=C3=A4stle?= Date: Mon, 17 Jun 2024 17:39:05 +0200 Subject: [PATCH 4/6] Also add errors for float32 values --- perfdata/type.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/perfdata/type.go b/perfdata/type.go index 06d2fd2..b972c3d 100644 --- a/perfdata/type.go +++ b/perfdata/type.go @@ -45,6 +45,14 @@ func formatNumeric(value interface{}) (string, error) { return check.FormatFloat(v), nil case float32: + if math.IsInf(float64(v), 0) { + return "", InfValueError{} + } + + if math.IsNaN(float64(v)) { + return "", NanValueError{} + } + return check.FormatFloat(float64(v)), nil case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64: return fmt.Sprintf("%d", v), nil From 59dacbeb52b84fe78b51100b7a62373d285f1be8 Mon Sep 17 00:00:00 2001 From: RincewindsHat Date: Tue, 18 Jun 2024 22:55:29 +0200 Subject: [PATCH 5/6] Restore behaviour of String() for perfdata, add new validation function This commit restores the previous signature of String() for perfdata by moving the functionality in a new method (which is also exported). This new method validates the data and throws errors, the String() method catches them and just returns the empty string then. --- perfdata/list.go | 2 +- perfdata/type.go | 9 ++++++++- perfdata/type_test.go | 27 ++++++++++++++++++++++++--- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/perfdata/list.go b/perfdata/list.go index 28fadc8..05ebb05 100644 --- a/perfdata/list.go +++ b/perfdata/list.go @@ -13,7 +13,7 @@ func (l PerfdataList) String() string { var out strings.Builder for _, p := range l { - pfDataString, err := p.String() + pfDataString, err := p.ValidatedString() // Ignore perfdata points which fail to format if err == nil { diff --git a/perfdata/type.go b/perfdata/type.go index b972c3d..5eff196 100644 --- a/perfdata/type.go +++ b/perfdata/type.go @@ -86,10 +86,17 @@ type Perfdata struct { } // String returns the proper format for the plugin output +// on errors (occurs with invalid data, the empty string is returned +func (p Perfdata) String() string { + tmp, _ := p.ValidatedString() + return tmp +} + +// ValidatedString returns the proper format for the plugin output // Returns an eror in some known cases where the value of a data type does not // represent a valid measurement, see the explanation for "formatNumeric" for // perfdata values. -func (p Perfdata) String() (string, error) { +func (p Perfdata) ValidatedString() (string, error) { var sb strings.Builder // Add quotes if string contains any whitespace diff --git a/perfdata/type_test.go b/perfdata/type_test.go index 4fdcfcc..6168df3 100644 --- a/perfdata/type_test.go +++ b/perfdata/type_test.go @@ -1,6 +1,7 @@ package perfdata import ( + "math" "testing" "github.com/NETWAYS/go-check" @@ -20,9 +21,8 @@ func BenchmarkPerfdataString(b *testing.B) { Max: 100} for i := 0; i < b.N; i++ { - p, err := perf.String() + p := perf.String() _ = p - _ = err } } @@ -72,13 +72,34 @@ func TestRenderPerfdata(t *testing.T) { }, } + testcasesWithErrors := map[string]struct { + perf Perfdata + expected string + }{ + "invalid-value": { + perf: Perfdata{ + Label: "to infinity", + Value: math.Inf(+1), + }, + expected: "", + }, + } + for name, tc := range testcases { t.Run(name, func(t *testing.T) { - pfVal, err := tc.perf.String() + pfVal, err := tc.perf.ValidatedString() assert.NoError(t, err) assert.Equal(t, tc.expected, pfVal) }) } + + for name, tc := range testcasesWithErrors { + t.Run(name, func(t *testing.T) { + pfVal, err := tc.perf.ValidatedString() + assert.Error(t, err) + assert.Equal(t, tc.expected, pfVal) + }) + } } type pfFormatTest struct { From 643ee88bbcc61c8453bff371aedf06adfe6d7edd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20K=C3=A4stle?= Date: Thu, 20 Jun 2024 13:59:49 +0200 Subject: [PATCH 6/6] Remove custom error types --- perfdata/type.go | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/perfdata/type.go b/perfdata/type.go index 5eff196..4814208 100644 --- a/perfdata/type.go +++ b/perfdata/type.go @@ -1,6 +1,7 @@ package perfdata import ( + "errors" "fmt" "math" "strings" @@ -11,20 +12,6 @@ import ( // Replace not allowed characters inside a label var replacer = strings.NewReplacer("=", "_", "`", "_", "'", "_", "\"", "_") -type InfValueError struct { -} - -func (i InfValueError) Error() string { - return "Performance data value is infinite" -} - -type NanValueError struct { -} - -func (i NanValueError) Error() string { - return "Performance data value is NaN (not a number)" -} - // formatNumeric returns a string representation of various possible numerics // // This supports most internal types of Go and all fmt.Stringer interfaces. @@ -36,21 +23,21 @@ func formatNumeric(value interface{}) (string, error) { switch v := value.(type) { case float64: if math.IsInf(v, 0) { - return "", InfValueError{} + return "", errors.New("Perfdata value is inifinite") } if math.IsNaN(v) { - return "", NanValueError{} + return "", errors.New("Perfdata value is inifinite") } return check.FormatFloat(v), nil case float32: if math.IsInf(float64(v), 0) { - return "", InfValueError{} + return "", errors.New("Perfdata value is inifinite") } if math.IsNaN(float64(v)) { - return "", NanValueError{} + return "", errors.New("Perfdata value is inifinite") } return check.FormatFloat(float64(v)), nil