-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: ensure exported Prometheus metrics are unique #2173
Changes from 5 commits
5d3fad2
860bea0
25beb7a
8fdc687
c725c49
5b7592b
ff0c66f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
name: Fcp | ||
name: FcpPort | ||
query: api/cluster/counter/tables/fcp | ||
object: fcp | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
package main | ||
|
||
import ( | ||
"fmt" | ||
"github.com/Netapp/harvest-automation/test/installer" | ||
"github.com/Netapp/harvest-automation/test/utils" | ||
"github.com/netapp/harvest/v2/pkg/conf" | ||
"github.com/rs/zerolog/log" | ||
"sort" | ||
"strconv" | ||
"strings" | ||
"testing" | ||
|
@@ -16,28 +18,53 @@ func TestPollerMetrics(t *testing.T) { | |
if err != nil { | ||
log.Fatal().Err(err).Msg("Unable to load harvest config") | ||
} | ||
var duplicateMetrics []string | ||
for _, pollerName := range conf.Config.PollersOrdered { | ||
port, _ := conf.GetPrometheusExporterPorts(pollerName, true) | ||
portString := strconv.Itoa(port) | ||
var validCounters = 0 | ||
uniqueSetOfMetricLabels := make(map[string]bool) | ||
sb, err2 := utils.GetResponse("http://localhost:" + strings.TrimSpace(portString) + "/metrics") | ||
if err2 != nil { | ||
panic("Unable to get metric data") | ||
t.Fatalf("Unable to get metric data for %s", pollerName) | ||
} | ||
rows := strings.Split(sb, "\n") | ||
for i := range rows { | ||
row := rows[i] | ||
length := len(row) | ||
if length == 0 { | ||
if len(row) == 0 { | ||
continue | ||
} | ||
// Ignore comments | ||
if strings.HasPrefix(row, "#") { | ||
continue | ||
} | ||
openBracket := strings.Index(row, "{") | ||
firstSpace := strings.Index(row, " ") | ||
if openBracket == -1 { | ||
// this means the metric has this form | ||
// metric_without_labels 12.47 | ||
if firstSpace == -1 { | ||
continue | ||
} | ||
key := metricAndLabelKey(row[:firstSpace], "") | ||
if uniqueSetOfMetricLabels[key] { | ||
duplicateMetrics = append(duplicateMetrics, | ||
fmt.Sprintf("Duplicate metric poller=%s, got >1 want 1 of %s", pollerName, key)) | ||
} | ||
uniqueSetOfMetricLabels[key] = true | ||
continue | ||
} | ||
open := strings.Index(row, "{") | ||
closeBracket := strings.Index(row, "}") | ||
space := strings.Index(row, " ") | ||
if open > 0 && closeBracket > 0 && space > 0 { | ||
//objectName := row[0:open] | ||
//metricContent := row[open:(close+1)] | ||
metricValue, _ := strconv.Atoi(strings.TrimSpace(row[(closeBracket + 1):length])) | ||
|
||
if openBracket > 0 && closeBracket > 0 && firstSpace > 0 { | ||
// Turn metric and labels into a unique key | ||
key := metricAndLabelKey(row[:openBracket], row[openBracket+1:]) | ||
if uniqueSetOfMetricLabels[key] { | ||
duplicateMetrics = append(duplicateMetrics, | ||
fmt.Sprintf("Duplicate metric poller=%s, got >1 want 1 of %s", pollerName, key)) | ||
} | ||
uniqueSetOfMetricLabels[key] = true | ||
metricValue, _ := strconv.Atoi(strings.TrimSpace(row[(closeBracket + 1):])) | ||
if metricValue > 0 { | ||
validCounters++ | ||
} | ||
|
@@ -50,4 +77,79 @@ func TestPollerMetrics(t *testing.T) { | |
} | ||
log.Info().Int("numCounters", validCounters).Str("poller", pollerName).Msg("Valid Counters for poller") | ||
} | ||
sort.Strings(duplicateMetrics) | ||
for _, dupMetric := range duplicateMetrics { | ||
if shouldSkipMetric(dupMetric) { | ||
log.Info().Str("metric", dupMetric).Msg("Skip") | ||
continue | ||
} | ||
t.Errorf(dupMetric) | ||
} | ||
} | ||
|
||
func metricAndLabelKey(metric string, rest string) string { | ||
var ( | ||
scanner int | ||
labels []string | ||
) | ||
|
||
for { | ||
label, equalIndex := readLabel(rest, scanner) | ||
if equalIndex == 0 { | ||
break | ||
} | ||
if equalIndex != 0 { | ||
equalIndex++ | ||
if string(rest[equalIndex]) == `"` { | ||
// Scan until you hit another unescaped quote. | ||
// Can be any sequence of UTF-8 characters, but the backslash (\), | ||
// and double-quote (") characters have to be | ||
// escaped as \, and \" | ||
labelEnd := 0 | ||
for i := equalIndex + 1; i < len(rest); i++ { | ||
s := string(rest[i]) | ||
if s == `\` { | ||
i++ | ||
continue | ||
} | ||
if s == `"` { | ||
// done reading quoted | ||
labelEnd = i | ||
break | ||
} | ||
} | ||
labelValue := rest[equalIndex+1 : labelEnd] | ||
labels = append(labels, fmt.Sprintf(`%s="%s"`, label, labelValue)) | ||
scanner = labelEnd + 1 | ||
if string(rest[scanner]) == "," { | ||
scanner++ | ||
} | ||
} | ||
} | ||
} | ||
|
||
sort.Strings(labels) | ||
|
||
return metric + "{" + strings.Join(labels, ",") + "}" | ||
} | ||
|
||
func readLabel(sample string, i int) (string, int) { | ||
sub := sample[i:] | ||
equalsIndex := strings.Index(sub, "=") | ||
if equalsIndex == -1 { | ||
return sample, 0 | ||
} | ||
end := i + equalsIndex | ||
return sample[i:end], end | ||
} | ||
|
||
func shouldSkipMetric(dupMetric string) bool { | ||
// Skip metrics that are belongs to aggr_efficiency template as Rest collector don't have separate template | ||
skip := []string{"logical_used_wo_snapshots", "logical_used_wo_snapshots_flexclones", "physical_used_wo_snapshots", "physical_used_wo_snapshots_flexclones", "total_logical_used", "total_physical_used"} | ||
for _, s := range skip { | ||
if strings.Contains(dupMetric, s) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the metric name be an exact match, or should it support all metrics starting with a specific prefix? While supporting metrics with a prefix can be useful, an exact match is preferable in this case to avoid any potential skipping caused by a "contains" check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Start with exact and relax if needed. That means this check can change to a map instead of searching a slice. These are the metrics in question
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved the check to above where key as |
||
return true | ||
} | ||
} | ||
return false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we need to log the skipped metric. If we do, might be better to be explicit with something like "Ignore duplicate"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed as such, changing to trace.