From b3247d4d7fb06456cbcb7a3de6be5680dee557a5 Mon Sep 17 00:00:00 2001 From: Edwin Mackenzie-Owen Date: Tue, 19 Mar 2024 18:39:35 +0100 Subject: [PATCH 1/7] Sanitize metric type prefixes When more than one prefix matches the same metric descriptor, this will throw the error "collected metric xxx was collected before with the same name and label values". For example, using the metric type prefixes foo.googleapis.com/bar (a prefix) and foo.googleapis.com/bar/baz (a metric) will result in an error because both match the metric foo.googleapis.com/bar/baz. Further, using the metric type prefixes foo.googleapis.com/bar/baz (a metric) and foo.googleapis.com/bar/baz_count (a metric) will result in an error because both match the metric foo.googleapis.com/bar/baz_count. While the first pitfall could be expected by the user, the latter will come as a complete surprise to anyone who is not aware that stackdriver-exporter internally uses an MQL query in the form of metric.type = starts_with("") to filter the metrics. Avoid this by sanitizing the provided metric type prefixes in the following way: - Drop any duplicate prefixes - Sort the prefixes (required by the next step) - Drop any prefixes that start with another prefix present in the input Signed-off-by: Edwin Mackenzie-Owen --- stackdriver_exporter.go | 44 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/stackdriver_exporter.go b/stackdriver_exporter.go index f71c4f4..a699da7 100644 --- a/stackdriver_exporter.go +++ b/stackdriver_exporter.go @@ -17,6 +17,7 @@ import ( "fmt" "net/http" "os" + "slices" "strings" "github.com/PuerkitoBio/rehttp" @@ -303,7 +304,7 @@ func main() { level.Info(logger).Log("msg", "Using Google Cloud Project IDs", "projectIDs", fmt.Sprintf("%v", projectIDs)) - metricsTypePrefixes := strings.Split(*monitoringMetricsTypePrefixes, ",") + metricsTypePrefixes := parseMetricTypePrefixes() metricExtraFilters := parseMetricExtraFilters() if *metricsPath == *stackdriverMetricsPath { @@ -353,6 +354,47 @@ func main() { } } +func parseMetricTypePrefixes() (metricTypePrefixes []string) { + inputPrefixes := strings.Split(*monitoringMetricsTypePrefixes, ",") + + // only keep unique prefixes + uniqueKeys := make(map[string]bool) + uniquePrefixes := []string{} + for _, prefix := range inputPrefixes { + if _, ok := uniqueKeys[prefix]; !ok { + uniqueKeys[prefix] = true + uniquePrefixes = append(uniquePrefixes, prefix) + } + } + + // drop prefixes that start with another existing prefix to avoid error: + // "collected metric xxx was collected before with the same name and label values" + slices.Sort(uniquePrefixes) + for i, prefix := range uniquePrefixes { + if i == 0 { + metricTypePrefixes = []string{prefix} + } else { + previousIndex := len(metricTypePrefixes) - 1 + + // current prefix starts with previous one + if strings.HasPrefix(prefix, metricTypePrefixes[previousIndex]) { + // drop current prefix + continue + } + + // previous prefix starts with current prefix + if strings.HasPrefix(metricTypePrefixes[previousIndex], prefix) { + // drop previous prefix + metricTypePrefixes = metricTypePrefixes[:previousIndex] + } + + metricTypePrefixes = append(metricTypePrefixes, prefix) + } + } + + return metricTypePrefixes +} + func parseMetricExtraFilters() []collectors.MetricFilter { var extraFilters []collectors.MetricFilter for _, ef := range *monitoringMetricsExtraFilter { From 4ae8653b7df2bbca22e4c0d535fc9425a1989557 Mon Sep 17 00:00:00 2001 From: Edwin Mackenzie-Owen Date: Thu, 13 Jun 2024 12:54:53 +0200 Subject: [PATCH 2/7] Remove condition that will never be true In alphanumerically sorted list of strings, abcdef will never come before abc. Signed-off-by: Edwin Mackenzie-Owen --- stackdriver_exporter.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/stackdriver_exporter.go b/stackdriver_exporter.go index a699da7..a13d9af 100644 --- a/stackdriver_exporter.go +++ b/stackdriver_exporter.go @@ -382,12 +382,6 @@ func parseMetricTypePrefixes() (metricTypePrefixes []string) { continue } - // previous prefix starts with current prefix - if strings.HasPrefix(metricTypePrefixes[previousIndex], prefix) { - // drop previous prefix - metricTypePrefixes = metricTypePrefixes[:previousIndex] - } - metricTypePrefixes = append(metricTypePrefixes, prefix) } } From 4d7601534750409da852a8c498fde0bd523f4bef Mon Sep 17 00:00:00 2001 From: Edwin Mackenzie-Owen Date: Thu, 13 Jun 2024 13:13:54 +0200 Subject: [PATCH 3/7] Add test for metrics prefix sanitization Signed-off-by: Edwin Mackenzie-Owen --- stackdriver_exporter.go | 7 +++---- stackdriver_exporter_test.go | 37 ++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 stackdriver_exporter_test.go diff --git a/stackdriver_exporter.go b/stackdriver_exporter.go index a13d9af..bd76544 100644 --- a/stackdriver_exporter.go +++ b/stackdriver_exporter.go @@ -304,7 +304,8 @@ func main() { level.Info(logger).Log("msg", "Using Google Cloud Project IDs", "projectIDs", fmt.Sprintf("%v", projectIDs)) - metricsTypePrefixes := parseMetricTypePrefixes() + inputPrefixes := strings.Split(*monitoringMetricsTypePrefixes, ",") + metricsTypePrefixes := parseMetricTypePrefixes(inputPrefixes) metricExtraFilters := parseMetricExtraFilters() if *metricsPath == *stackdriverMetricsPath { @@ -354,9 +355,7 @@ func main() { } } -func parseMetricTypePrefixes() (metricTypePrefixes []string) { - inputPrefixes := strings.Split(*monitoringMetricsTypePrefixes, ",") - +func parseMetricTypePrefixes(inputPrefixes []string) (metricTypePrefixes []string) { // only keep unique prefixes uniqueKeys := make(map[string]bool) uniquePrefixes := []string{} diff --git a/stackdriver_exporter_test.go b/stackdriver_exporter_test.go new file mode 100644 index 0000000..05d0e3e --- /dev/null +++ b/stackdriver_exporter_test.go @@ -0,0 +1,37 @@ +// Copyright 2024 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import "testing" +import "reflect" + +func TestParseMetricTypePrefixes(t *testing.T) { + inputPrefixes := []string{ + "redis.googleapis.com/stats/memory/usage", + "loadbalancing.googleapis.com/https/request_count", + "loadbalancing.googleapis.com", + "redis.googleapis.com/stats/memory/usage_ratio", + "redis.googleapis.com/stats/memory/usage_ratio", + } + expectedOutputPrefixes := []string{ + "loadbalancing.googleapis.com", + "redis.googleapis.com/stats/memory/usage", + } + + outputPrefixes := parseMetricTypePrefixes(inputPrefixes) + + if !reflect.DeepEqual(outputPrefixes, expectedOutputPrefixes) { + t.Errorf("Metric type prefix sanitization did not produce expected output. Expected:\n%s\nGot:\n%s", expectedOutputPrefixes, outputPrefixes) + } +} From f020f50eff10ef81251cc5d623ec4e82c1ebff07 Mon Sep 17 00:00:00 2001 From: Edwin Mackenzie-Owen Date: Thu, 22 Aug 2024 15:49:17 +0200 Subject: [PATCH 4/7] Use slices.Compact() for initial deduplication Signed-off-by: Edwin Mackenzie-Owen --- stackdriver_exporter.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/stackdriver_exporter.go b/stackdriver_exporter.go index bd76544..9a39bd0 100644 --- a/stackdriver_exporter.go +++ b/stackdriver_exporter.go @@ -356,19 +356,12 @@ func main() { } func parseMetricTypePrefixes(inputPrefixes []string) (metricTypePrefixes []string) { - // only keep unique prefixes - uniqueKeys := make(map[string]bool) - uniquePrefixes := []string{} - for _, prefix := range inputPrefixes { - if _, ok := uniqueKeys[prefix]; !ok { - uniqueKeys[prefix] = true - uniquePrefixes = append(uniquePrefixes, prefix) - } - } + // drop duplicate prefixes + slices.Sort(inputPrefixes) + uniquePrefixes := slices.Compact(inputPrefixes) // drop prefixes that start with another existing prefix to avoid error: // "collected metric xxx was collected before with the same name and label values" - slices.Sort(uniquePrefixes) for i, prefix := range uniquePrefixes { if i == 0 { metricTypePrefixes = []string{prefix} From c46b249977545c6c26e6508c03be3f5e21010407 Mon Sep 17 00:00:00 2001 From: Edwin Mackenzie-Owen Date: Thu, 22 Aug 2024 15:53:11 +0200 Subject: [PATCH 5/7] Improve comments Signed-off-by: Edwin Mackenzie-Owen --- stackdriver_exporter.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/stackdriver_exporter.go b/stackdriver_exporter.go index 9a39bd0..d7f68d6 100644 --- a/stackdriver_exporter.go +++ b/stackdriver_exporter.go @@ -356,21 +356,20 @@ func main() { } func parseMetricTypePrefixes(inputPrefixes []string) (metricTypePrefixes []string) { - // drop duplicate prefixes + // Drop duplicate prefixes. slices.Sort(inputPrefixes) uniquePrefixes := slices.Compact(inputPrefixes) - // drop prefixes that start with another existing prefix to avoid error: - // "collected metric xxx was collected before with the same name and label values" + // Drop prefixes that start with another existing prefix to avoid error: + // "collected metric xxx was collected before with the same name and label values". for i, prefix := range uniquePrefixes { if i == 0 { metricTypePrefixes = []string{prefix} } else { previousIndex := len(metricTypePrefixes) - 1 - // current prefix starts with previous one + // Drop current prefix if it starts with the previous one. if strings.HasPrefix(prefix, metricTypePrefixes[previousIndex]) { - // drop current prefix continue } From 5dca896eb444fb2ecfc72fc620ec96c8ab96d31a Mon Sep 17 00:00:00 2001 From: Edwin Mackenzie-Owen Date: Thu, 22 Aug 2024 16:08:42 +0200 Subject: [PATCH 6/7] Always initialize return slice Signed-off-by: Edwin Mackenzie-Owen --- stackdriver_exporter.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/stackdriver_exporter.go b/stackdriver_exporter.go index d7f68d6..5f3dab1 100644 --- a/stackdriver_exporter.go +++ b/stackdriver_exporter.go @@ -355,7 +355,9 @@ func main() { } } -func parseMetricTypePrefixes(inputPrefixes []string) (metricTypePrefixes []string) { +func parseMetricTypePrefixes(inputPrefixes []string) []string { + metricTypePrefixes := []string{} + // Drop duplicate prefixes. slices.Sort(inputPrefixes) uniquePrefixes := slices.Compact(inputPrefixes) @@ -364,7 +366,7 @@ func parseMetricTypePrefixes(inputPrefixes []string) (metricTypePrefixes []strin // "collected metric xxx was collected before with the same name and label values". for i, prefix := range uniquePrefixes { if i == 0 { - metricTypePrefixes = []string{prefix} + metricTypePrefixes = append(metricTypePrefixes, prefix) } else { previousIndex := len(metricTypePrefixes) - 1 From bba5c781ed6a861664b65b1967b8032b0b0edcac Mon Sep 17 00:00:00 2001 From: Edwin Mackenzie-Owen Date: Thu, 22 Aug 2024 16:30:10 +0200 Subject: [PATCH 7/7] Improve if condition Signed-off-by: Edwin Mackenzie-Owen --- stackdriver_exporter.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/stackdriver_exporter.go b/stackdriver_exporter.go index 5f3dab1..767b52c 100644 --- a/stackdriver_exporter.go +++ b/stackdriver_exporter.go @@ -365,18 +365,15 @@ func parseMetricTypePrefixes(inputPrefixes []string) []string { // Drop prefixes that start with another existing prefix to avoid error: // "collected metric xxx was collected before with the same name and label values". for i, prefix := range uniquePrefixes { - if i == 0 { - metricTypePrefixes = append(metricTypePrefixes, prefix) - } else { + if i != 0 { previousIndex := len(metricTypePrefixes) - 1 // Drop current prefix if it starts with the previous one. if strings.HasPrefix(prefix, metricTypePrefixes[previousIndex]) { continue } - - metricTypePrefixes = append(metricTypePrefixes, prefix) } + metricTypePrefixes = append(metricTypePrefixes, prefix) } return metricTypePrefixes