ci: ensure exported Prometheus metrics are unique #2173
Conversation
integration/test/metric_test.go
Outdated
| // 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.
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.
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
aggr_logical_used_wo_snapshots
aggr_logical_used_wo_snapshots_flexclones
aggr_physical_used_wo_snapshots
aggr_physical_used_wo_snapshots_flexclones
aggr_total_logical_used
aggr_total_physical_used
There was a problem hiding this comment.
Moved the check to above where key as aggr_total_logical_used{aggr=\"astra_aggr2\",cluster=\"A250-41-42-43\",datacenter=\"Rest\",node=\"A250-42\"} and now HasPrefix would be used instead contains.
integration/test/metric_test.go
Outdated
| sort.Strings(duplicateMetrics) | ||
| for _, dupMetric := range duplicateMetrics { | ||
| if shouldSkipMetric(dupMetric) { | ||
| log.Info().Str("metric", dupMetric).Msg("Skip") |
There was a problem hiding this comment.
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.
not needed as such, changing to trace.
There was a problem hiding this comment.
Looks good. Nice job finding this issue @rahulguptajss and @Hardikl for fixing
No description provided.