Remove metrics from compat package (#3714)

This commit removes the metrics from the compat package
in favour of the existing logging and the additional tools
at hand, such as amtool, to validate Alertmanager configurations.

Due to the global nature of the compat package, a consequence
of config.Load, these metrics have proven to be less useful
in practice than expected, both in Alertmanager and other projects
such as Mimir.

There are a number of reasons for this:

1. Because the compat package is global, these metrics cannot be
   reset each time config.Load is called, as in multi-tenant
   projects like Mimir loading a config for one tenant would reset
   the metrics for all tenants. This is also the reason the metrics
   are counters and not gauges.

2. Since the metrics are counters, it is difficult to create
   meaningful dashboards for Alertmanager as, unlike in Mimir,
   configurations are not reloaded at fixed intervals, and as such,
   operators cannot use rate to track configuration changes
   over time.

In Alertmanager, there are much better tools available to validate
that an Alertmanager configuration is compatible with the UTF-8
parser, including both the existing logging from Alertmanager
server and amtool check-config.

In other projects like Mimir, we can track configurations for
individual tenants using log aggregation and storage systems
such as Loki. This gives operators far more information than
what is possible with the metrics, including the timestamp,
input and ID of tenant configurations that are incompatible
or have disagreement.

Signed-off-by: George Robinson <george.robinson@grafana.com>
This commit is contained in:
George Robinson 2024-02-08 09:59:03 +00:00 committed by GitHub
parent 3d49ff83c7
commit f69a508665
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 43 additions and 195 deletions

View File

@ -61,7 +61,7 @@ func initMatchersCompat(_ *kingpin.ParseContext) error {
if err != nil {
kingpin.Fatalf("error parsing the feature flag list: %v\n", err)
}
compat.InitFromFlags(logger, compat.NewMetrics(nil), featureConfig)
compat.InitFromFlags(logger, featureConfig)
return nil
}

View File

@ -186,7 +186,7 @@ func run() int {
level.Error(logger).Log("msg", "error parsing the feature flag list", "err", err)
return 1
}
compat.InitFromFlags(logger, compat.RegisteredMetrics, ff)
compat.InitFromFlags(logger, ff)
err = os.MkdirAll(*dataDir, 0o777)
if err != nil {

View File

@ -1,60 +0,0 @@
// Copyright 2023 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 compat
import (
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
)
const (
OriginAPI = "api"
OriginConfig = "config"
)
var DefaultOrigins = []string{
OriginAPI,
OriginConfig,
}
var RegisteredMetrics = NewMetrics(prometheus.DefaultRegisterer)
type Metrics struct {
Total *prometheus.CounterVec
DisagreeTotal *prometheus.CounterVec
IncompatibleTotal *prometheus.CounterVec
InvalidTotal *prometheus.CounterVec
}
func NewMetrics(r prometheus.Registerer) *Metrics {
m := &Metrics{
Total: promauto.With(r).NewCounterVec(prometheus.CounterOpts{
Name: "alertmanager_matchers_parse_total",
Help: "Total number of matcher inputs parsed, including invalid inputs.",
}, []string{"origin"}),
DisagreeTotal: promauto.With(r).NewCounterVec(prometheus.CounterOpts{
Name: "alertmanager_matchers_disagree_total",
Help: "Total number of matcher inputs which produce different parsings (disagreement).",
}, []string{"origin"}),
IncompatibleTotal: promauto.With(r).NewCounterVec(prometheus.CounterOpts{
Name: "alertmanager_matchers_incompatible_total",
Help: "Total number of matcher inputs that are incompatible with the UTF-8 parser.",
}, []string{"origin"}),
InvalidTotal: promauto.With(r).NewCounterVec(prometheus.CounterOpts{
Name: "alertmanager_matchers_invalid_total",
Help: "Total number of matcher inputs that could not be parsed.",
}, []string{"origin"}),
}
return m
}

View File

@ -21,7 +21,6 @@ import (
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/common/model"
"github.com/prometheus/alertmanager/featurecontrol"
@ -31,8 +30,8 @@ import (
var (
isValidLabelName = isValidClassicLabelName(log.NewNopLogger())
parseMatcher = ClassicMatcherParser(log.NewNopLogger(), RegisteredMetrics)
parseMatchers = ClassicMatchersParser(log.NewNopLogger(), RegisteredMetrics)
parseMatcher = ClassicMatcherParser(log.NewNopLogger())
parseMatchers = ClassicMatchersParser(log.NewNopLogger())
)
// IsValidLabelName returns true if the string is a valid label name.
@ -57,33 +56,26 @@ func Matchers(input, origin string) (labels.Matchers, error) {
}
// InitFromFlags initializes the compat package from the flagger.
func InitFromFlags(l log.Logger, m *Metrics, f featurecontrol.Flagger) {
func InitFromFlags(l log.Logger, f featurecontrol.Flagger) {
if f.ClassicMode() {
isValidLabelName = isValidClassicLabelName(l)
parseMatcher = ClassicMatcherParser(l, m)
parseMatchers = ClassicMatchersParser(l, m)
parseMatcher = ClassicMatcherParser(l)
parseMatchers = ClassicMatchersParser(l)
} else if f.UTF8StrictMode() {
isValidLabelName = isValidUTF8LabelName(l)
parseMatcher = UTF8MatcherParser(l, m)
parseMatchers = UTF8MatchersParser(l, m)
parseMatcher = UTF8MatcherParser(l)
parseMatchers = UTF8MatchersParser(l)
} else {
isValidLabelName = isValidUTF8LabelName(l)
parseMatcher = FallbackMatcherParser(l, m)
parseMatchers = FallbackMatchersParser(l, m)
parseMatcher = FallbackMatcherParser(l)
parseMatchers = FallbackMatchersParser(l)
}
}
// ClassicMatcherParser uses the pkg/labels parser to parse the matcher in
// the input string.
func ClassicMatcherParser(l log.Logger, m *Metrics) ParseMatcher {
func ClassicMatcherParser(l log.Logger) ParseMatcher {
return func(input, origin string) (matcher *labels.Matcher, err error) {
defer func() {
lbs := prometheus.Labels{"origin": origin}
m.Total.With(lbs).Inc()
if err != nil {
m.InvalidTotal.With(lbs).Inc()
}
}()
level.Debug(l).Log("msg", "Parsing with classic matchers parser", "input", input, "origin", origin)
return labels.ParseMatcher(input)
}
@ -91,15 +83,8 @@ func ClassicMatcherParser(l log.Logger, m *Metrics) ParseMatcher {
// ClassicMatchersParser uses the pkg/labels parser to parse zero or more
// matchers in the input string. It returns an error if the input is invalid.
func ClassicMatchersParser(l log.Logger, m *Metrics) ParseMatchers {
func ClassicMatchersParser(l log.Logger) ParseMatchers {
return func(input, origin string) (matchers labels.Matchers, err error) {
defer func() {
lbs := prometheus.Labels{"origin": origin}
m.Total.With(lbs).Inc()
if err != nil {
m.InvalidTotal.With(lbs).Inc()
}
}()
level.Debug(l).Log("msg", "Parsing with classic matchers parser", "input", input, "origin", origin)
return labels.ParseMatchers(input)
}
@ -107,15 +92,8 @@ func ClassicMatchersParser(l log.Logger, m *Metrics) ParseMatchers {
// UTF8MatcherParser uses the new matchers/parse parser to parse the matcher
// in the input string. If this fails it does not revert to the pkg/labels parser.
func UTF8MatcherParser(l log.Logger, m *Metrics) ParseMatcher {
func UTF8MatcherParser(l log.Logger) ParseMatcher {
return func(input, origin string) (matcher *labels.Matcher, err error) {
defer func() {
lbs := prometheus.Labels{"origin": origin}
m.Total.With(lbs).Inc()
if err != nil {
m.InvalidTotal.With(lbs).Inc()
}
}()
level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser", "input", input, "origin", origin)
if strings.HasPrefix(input, "{") || strings.HasSuffix(input, "}") {
return nil, fmt.Errorf("unexpected open or close brace: %s", input)
@ -127,15 +105,8 @@ func UTF8MatcherParser(l log.Logger, m *Metrics) ParseMatcher {
// UTF8MatchersParser uses the new matchers/parse parser to parse zero or more
// matchers in the input string. If this fails it does not revert to the
// pkg/labels parser.
func UTF8MatchersParser(l log.Logger, m *Metrics) ParseMatchers {
func UTF8MatchersParser(l log.Logger) ParseMatchers {
return func(input, origin string) (matchers labels.Matchers, err error) {
defer func() {
lbs := prometheus.Labels{"origin": origin}
m.Total.With(lbs).Inc()
if err != nil {
m.InvalidTotal.With(lbs).Inc()
}
}()
level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser", "input", input, "origin", origin)
return parse.Matchers(input)
}
@ -144,15 +115,8 @@ func UTF8MatchersParser(l log.Logger, m *Metrics) ParseMatchers {
// FallbackMatcherParser uses the new matchers/parse parser to parse zero or more
// matchers in the string. If this fails it reverts to the pkg/labels parser and
// emits a warning log line.
func FallbackMatcherParser(l log.Logger, m *Metrics) ParseMatcher {
func FallbackMatcherParser(l log.Logger) ParseMatcher {
return func(input, origin string) (matcher *labels.Matcher, err error) {
lbs := prometheus.Labels{"origin": origin}
defer func() {
m.Total.With(lbs).Inc()
if err != nil {
m.InvalidTotal.With(lbs).Inc()
}
}()
level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to classic matchers parser", "input", input, "origin", origin)
if strings.HasPrefix(input, "{") || strings.HasSuffix(input, "}") {
return nil, fmt.Errorf("unexpected open or close brace: %s", input)
@ -168,7 +132,6 @@ func FallbackMatcherParser(l log.Logger, m *Metrics) ParseMatcher {
}
// The input is valid in the pkg/labels parser, but not the matchers/parse
// parser. This means the input is not forwards compatible.
m.IncompatibleTotal.With(lbs).Inc()
suggestion := cMatcher.String()
level.Warn(l).Log("msg", "Alertmanager is moving to a new parser for labels and matchers, and this input is incompatible. Alertmanager has instead parsed the input using the old matchers parser as a fallback. To make this input compatible with the new parser please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue.", "input", input, "origin", origin, "err", nErr, "suggestion", suggestion)
return cMatcher, nil
@ -176,7 +139,6 @@ func FallbackMatcherParser(l log.Logger, m *Metrics) ParseMatcher {
// If the input is valid in both parsers, but produces different results,
// then there is disagreement.
if nErr == nil && cErr == nil && !reflect.DeepEqual(nMatcher, cMatcher) {
m.DisagreeTotal.With(lbs).Inc()
level.Warn(l).Log("msg", "Matchers input has disagreement", "input", input, "origin", origin)
return cMatcher, nil
}
@ -187,15 +149,8 @@ func FallbackMatcherParser(l log.Logger, m *Metrics) ParseMatcher {
// FallbackMatchersParser uses the new matchers/parse parser to parse the
// matcher in the input string. If this fails it falls back to the pkg/labels
// parser and emits a warning log line.
func FallbackMatchersParser(l log.Logger, m *Metrics) ParseMatchers {
func FallbackMatchersParser(l log.Logger) ParseMatchers {
return func(input, origin string) (matchers labels.Matchers, err error) {
lbs := prometheus.Labels{"origin": origin}
defer func() {
m.Total.With(lbs).Inc()
if err != nil {
m.InvalidTotal.With(lbs).Inc()
}
}()
level.Debug(l).Log("msg", "Parsing with UTF-8 matchers parser, with fallback to classic matchers parser", "input", input, "origin", origin)
// Parse the input in both parsers to look for disagreement and incompatible
// inputs.
@ -208,7 +163,6 @@ func FallbackMatchersParser(l log.Logger, m *Metrics) ParseMatchers {
}
// The input is valid in the pkg/labels parser, but not the matchers/parse
// parser. This means the input is not forwards compatible.
m.IncompatibleTotal.With(lbs).Inc()
var sb strings.Builder
for i, n := range cMatchers {
sb.WriteString(n.String())
@ -226,7 +180,6 @@ func FallbackMatchersParser(l log.Logger, m *Metrics) ParseMatchers {
// then there is disagreement. We need to compare to labels.Matchers(cMatchers)
// as cMatchers is a []*labels.Matcher not labels.Matchers.
if nErr == nil && cErr == nil && !reflect.DeepEqual(nMatchers, labels.Matchers(cMatchers)) {
m.DisagreeTotal.With(lbs).Inc()
level.Warn(l).Log("msg", "Matchers input has disagreement", "input", input, "origin", origin)
return cMatchers, nil
}

View File

@ -17,8 +17,6 @@ import (
"testing"
"github.com/go-kit/log"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/prometheus/common/model"
"github.com/stretchr/testify/require"
@ -31,32 +29,22 @@ func TestFallbackMatcherParser(t *testing.T) {
input string
expected *labels.Matcher
err string
total float64
disagreeTotal float64
incompatibleTotal float64
invalidTotal float64
}{{
name: "input is accepted",
input: "foo=bar",
expected: mustNewMatcher(t, labels.MatchEqual, "foo", "bar"),
total: 1,
}, {
name: "input is accepted in neither",
input: "foo!bar",
err: "bad matcher format: foo!bar",
total: 1,
invalidTotal: 1,
}, {
name: "input is accepted in matchers/parse but not pkg/labels",
input: "foo🙂=bar",
expected: mustNewMatcher(t, labels.MatchEqual, "foo🙂", "bar"),
total: 1,
}, {
name: "input is accepted in pkg/labels but not matchers/parse",
input: "foo=!bar\\n",
expected: mustNewMatcher(t, labels.MatchEqual, "foo", "!bar\n"),
total: 1,
incompatibleTotal: 1,
}, {
// This input causes disagreement because \xf0\x9f\x99\x82 is the byte sequence for 🙂,
// which is not understood by pkg/labels but is understood by matchers/parse. In such cases,
@ -64,14 +52,11 @@ func TestFallbackMatcherParser(t *testing.T) {
name: "input causes disagreement",
input: "foo=\"\\xf0\\x9f\\x99\\x82\"",
expected: mustNewMatcher(t, labels.MatchEqual, "foo", "\\xf0\\x9f\\x99\\x82"),
total: 1,
disagreeTotal: 1,
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
m := NewMetrics(prometheus.NewRegistry())
f := FallbackMatcherParser(log.NewNopLogger(), m)
f := FallbackMatcherParser(log.NewNopLogger())
matcher, err := f(test.input, "test")
if test.err != "" {
require.EqualError(t, err, test.err)
@ -79,10 +64,6 @@ func TestFallbackMatcherParser(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, test.expected, matcher)
}
requireMetric(t, test.total, m.Total)
requireMetric(t, test.disagreeTotal, m.DisagreeTotal)
requireMetric(t, test.incompatibleTotal, m.IncompatibleTotal)
requireMetric(t, test.invalidTotal, m.InvalidTotal)
})
}
}
@ -93,10 +74,6 @@ func TestFallbackMatchersParser(t *testing.T) {
input string
expected labels.Matchers
err string
total float64
disagreeTotal float64
incompatibleTotal float64
invalidTotal float64
}{{
name: "input is accepted",
input: "{foo=bar,bar=baz}",
@ -104,13 +81,10 @@ func TestFallbackMatchersParser(t *testing.T) {
mustNewMatcher(t, labels.MatchEqual, "foo", "bar"),
mustNewMatcher(t, labels.MatchEqual, "bar", "baz"),
},
total: 1,
}, {
name: "input is accepted in neither",
input: "{foo!bar}",
err: "bad matcher format: foo!bar",
total: 1,
invalidTotal: 1,
}, {
name: "input is accepted in matchers/parse but not pkg/labels",
input: "{foo🙂=bar,bar=baz🙂}",
@ -118,7 +92,6 @@ func TestFallbackMatchersParser(t *testing.T) {
mustNewMatcher(t, labels.MatchEqual, "foo🙂", "bar"),
mustNewMatcher(t, labels.MatchEqual, "bar", "baz🙂"),
},
total: 1,
}, {
name: "is accepted in pkg/labels but not matchers/parse",
input: "{foo=!bar,bar=$baz\\n}",
@ -126,8 +99,6 @@ func TestFallbackMatchersParser(t *testing.T) {
mustNewMatcher(t, labels.MatchEqual, "foo", "!bar"),
mustNewMatcher(t, labels.MatchEqual, "bar", "$baz\n"),
},
total: 1,
incompatibleTotal: 1,
}, {
// This input causes disagreement because \xf0\x9f\x99\x82 is the byte sequence for 🙂,
// which is not understood by pkg/labels but is understood by matchers/parse. In such cases,
@ -137,14 +108,11 @@ func TestFallbackMatchersParser(t *testing.T) {
expected: labels.Matchers{
mustNewMatcher(t, labels.MatchEqual, "foo", "\\xf0\\x9f\\x99\\x82"),
},
total: 1,
disagreeTotal: 1,
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
m := NewMetrics(prometheus.NewRegistry())
f := FallbackMatchersParser(log.NewNopLogger(), m)
f := FallbackMatchersParser(log.NewNopLogger())
matchers, err := f(test.input, "test")
if test.err != "" {
require.EqualError(t, err, test.err)
@ -152,10 +120,6 @@ func TestFallbackMatchersParser(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, test.expected, matchers)
}
requireMetric(t, test.total, m.Total)
requireMetric(t, test.disagreeTotal, m.DisagreeTotal)
requireMetric(t, test.incompatibleTotal, m.IncompatibleTotal)
requireMetric(t, test.invalidTotal, m.InvalidTotal)
})
}
}
@ -235,12 +199,3 @@ func TestIsValidUTF8LabelName(t *testing.T) {
})
}
}
func requireMetric(t *testing.T, expected float64, m *prometheus.CounterVec) {
if expected == 0 {
require.Equal(t, 0, testutil.CollectAndCount(m))
} else {
require.Equal(t, 1, testutil.CollectAndCount(m))
require.Equal(t, expected, testutil.ToFloat64(m))
}
}

View File

@ -1334,12 +1334,12 @@ func TestValidateUTF8Matcher(t *testing.T) {
// Change the mode to UTF-8 mode.
ff, err := featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureUTF8StrictMode)
require.NoError(t, err)
compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff)
compat.InitFromFlags(log.NewNopLogger(), ff)
// Restore the mode to classic at the end of the test.
ff, err = featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureClassicMode)
require.NoError(t, err)
defer compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff)
defer compat.InitFromFlags(log.NewNopLogger(), ff)
for _, c := range cases {
checkErr(t, c.err, validateMatcher(c.m))

View File

@ -341,12 +341,12 @@ func TestValidateUTF8Ls(t *testing.T) {
// Change the mode to UTF-8 mode.
ff, err := featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureUTF8StrictMode)
require.NoError(t, err)
compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff)
compat.InitFromFlags(log.NewNopLogger(), ff)
// Restore the mode to classic at the end of the test.
ff, err = featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureClassicMode)
require.NoError(t, err)
defer compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff)
defer compat.InitFromFlags(log.NewNopLogger(), ff)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {