From 12646c9da9552b8fc9621727e712d0de5465d5cf Mon Sep 17 00:00:00 2001 From: Markus Opolka Date: Mon, 26 Jun 2023 10:50:27 +0200 Subject: [PATCH 1/2] Add benchmark for perfdata - The perfdata code does a lot of string operations, we might want to optimize it in the future - Example usage go test -run=. -bench=BenchmarkPerfdataString -count=10 ./... | tee 1.benchmark --- .gitignore | 2 ++ perfdata/type_test.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/.gitignore b/.gitignore index 6c1c4b8..864c82e 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,8 @@ httpmock-record.yml # Test binary, built with `go test -c` *.test +*.profile +*.benchmark # Output of the go coverage tool, specifically when used with LiteIDE *.out diff --git a/perfdata/type_test.go b/perfdata/type_test.go index 23bdbd6..1ac94f0 100644 --- a/perfdata/type_test.go +++ b/perfdata/type_test.go @@ -8,6 +8,23 @@ import ( "github.com/stretchr/testify/assert" ) +func BenchmarkPerfdataString(b *testing.B) { + b.ReportAllocs() + + perf := Perfdata{ + Label: "test test=test", + Value: 10.1, + Uom: "%", + Warn: &check.Threshold{Upper: 80}, + Crit: &check.Threshold{Upper: 90}, + Min: 0, + Max: 100} + + for i := 0; i < b.N; i++ { + perf.String() + } +} + func ExamplePerfdata() { perf := Perfdata{ Label: "test", From 4e4b05cc167341eb09c5ed5eadcf77969fe66288 Mon Sep 17 00:00:00 2001 From: Markus Opolka Date: Mon, 26 Jun 2023 10:31:58 +0200 Subject: [PATCH 2/2] Refactor perfdata - Moved util.go into type.go since 'util' is too generic a name - Made formatting function private, since they should not be used outside the module - Refactored tests to be table driven - Less restrictive character set for labels --- perfdata/type.go | 33 +++++++++++++-- perfdata/type_test.go | 93 ++++++++++++++++++++++++------------------- perfdata/util.go | 43 -------------------- perfdata/util_test.go | 24 ----------- 4 files changed, 81 insertions(+), 112 deletions(-) delete mode 100644 perfdata/util.go delete mode 100644 perfdata/util_test.go diff --git a/perfdata/type.go b/perfdata/type.go index 9ce2ad0..be38c06 100644 --- a/perfdata/type.go +++ b/perfdata/type.go @@ -1,10 +1,32 @@ package perfdata import ( + "fmt" "github.com/NETWAYS/go-check" "strings" ) +// Replace not allowed characters inside a label +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 { + switch v := value.(type) { + case float64: + return check.FormatFloat(v) + case float32: + return check.FormatFloat(float64(v)) + case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64: + return fmt.Sprintf("%d", v) + case fmt.Stringer, string: + return fmt.Sprint(v) + default: + panic(fmt.Sprintf("unsupported type for perfdata: %T", value)) + } +} + // Perfdata represents all properties of performance data for Icinga // // Implements fmt.Stringer to return the plaintext format for a plugin output. @@ -31,9 +53,14 @@ type Perfdata struct { func (p Perfdata) String() string { var sb strings.Builder - sb.WriteString(FormatLabel(p.Label) + "=") + // Add quotes if string contains any whitespace + if strings.ContainsAny(p.Label, "\t\n\f\r ") { + sb.WriteString(`'` + replacer.Replace(p.Label) + `'` + "=") + } else { + sb.WriteString(replacer.Replace(p.Label) + "=") + } - sb.WriteString(FormatNumeric(p.Value)) + sb.WriteString(formatNumeric(p.Value)) sb.WriteString(p.Uom) // Thresholds @@ -50,7 +77,7 @@ func (p Perfdata) String() string { sb.WriteString(";") if value != nil { - sb.WriteString(FormatNumeric(value)) + sb.WriteString(formatNumeric(value)) } } diff --git a/perfdata/type_test.go b/perfdata/type_test.go index 1ac94f0..3220409 100644 --- a/perfdata/type_test.go +++ b/perfdata/type_test.go @@ -1,7 +1,6 @@ package perfdata import ( - "fmt" "testing" "github.com/NETWAYS/go-check" @@ -21,53 +20,63 @@ func BenchmarkPerfdataString(b *testing.B) { Max: 100} for i := 0; i < b.N; i++ { - perf.String() + p := perf.String() + _ = p } } -func ExamplePerfdata() { - perf := Perfdata{ - Label: "test", - Value: 10.1, - Uom: "%", - Warn: &check.Threshold{Upper: 80}, - Crit: &check.Threshold{Upper: 90}, - Min: 0, - Max: 100} - - fmt.Println(perf) - - // Output: - // test=10.1%;80;90;0;100 -} - -func TestPerfdata(t *testing.T) { - perf := Perfdata{ - Label: "test", - Value: 2, +func TestRenderPerfdata(t *testing.T) { + testcases := map[string]struct { + perf Perfdata + expected string + }{ + "simple": { + perf: Perfdata{ + Label: "test", + Value: 2, + }, + expected: "test=2", + }, + "with-special-chars": { + perf: Perfdata{ + Label: "test_🖥️_'test", + Value: 2, + }, + expected: "test_🖥️__test=2", + }, + "with-uom": { + perf: Perfdata{ + Label: "test", + Value: 2, + Uom: "%", + }, + expected: "test=2%", + }, + "with-thresholds": { + perf: Perfdata{ + Label: "foo bar", + Value: 2.76, + Uom: "m", + Warn: &check.Threshold{Lower: 10, Upper: 25, Inside: true}, + Crit: &check.Threshold{Lower: 15, Upper: 20, Inside: false}, + }, + expected: "'foo bar'=2.76m;@10:25;15:20", + }, } - assert.Equal(t, "test=2", perf.String()) -} - -func TestPerfdata2(t *testing.T) { - perf := Perfdata{ - Label: "test", - Value: 2, - Uom: "%", + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.expected, tc.perf.String()) + }) } - - assert.Equal(t, "test=2%", perf.String()) } -func TestPerfdata3(t *testing.T) { - perf := Perfdata{ - Label: "foo bar", - Value: 2.76, - Uom: "m", - Warn: &check.Threshold{Lower: 10, Upper: 25, Inside: true}, - Crit: &check.Threshold{Lower: 15, Upper: 20, Inside: false}, - } - - assert.Equal(t, "'foo bar'=2.76m;@10:25;15:20", perf.String()) +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)) } diff --git a/perfdata/util.go b/perfdata/util.go deleted file mode 100644 index 843c6d4..0000000 --- a/perfdata/util.go +++ /dev/null @@ -1,43 +0,0 @@ -package perfdata - -import ( - "fmt" - "github.com/NETWAYS/go-check" - "regexp" - "strings" -) - -// Lists all allowed characters inside a label, so we can replace any non-matching -var validInLabelRe = regexp.MustCompile(`[^a-zA-Z0-9 _\-+:/.;]+`) - -// 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 { - switch v := value.(type) { - case float64: - return check.FormatFloat(v) - case float32: - return check.FormatFloat(float64(v)) - case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64: - return fmt.Sprintf("%d", v) - case fmt.Stringer, string: - return fmt.Sprint(v) - default: - panic(fmt.Sprintf("unsupported type for perfdata: %T", value)) - } -} - -// FormatLabel returns a sane perfdata label -// -// All groups of invalid characters will be replaced by a single underscore. -func FormatLabel(label string) string { - // Replace invalid character groups by an underscore - label = validInLabelRe.ReplaceAllString(label, "_") - - if strings.ContainsAny(label, " ") { - return `'` + label + `'` - } - - return label -} diff --git a/perfdata/util_test.go b/perfdata/util_test.go deleted file mode 100644 index ebf6d8f..0000000 --- a/perfdata/util_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package perfdata - -import ( - "github.com/stretchr/testify/assert" - "testing" -) - -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)) -} - -func TestFormatLabel(t *testing.T) { - assert.Equal(t, "test", FormatLabel("test")) - assert.Equal(t, "'test test'", FormatLabel("test test")) - assert.Equal(t, "test_x", FormatLabel("test\t\n\\x")) - assert.Equal(t, "t_est_x", FormatLabel("t%$%^est\t\n\\x")) - assert.Equal(t, "test/:x", FormatLabel("test/:x")) -}