Skip to content

Commit

Permalink
Remove metrics from compat package (prometheus#3714)
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Gokhan Sari <[email protected]>
  • Loading branch information
grobinson-grafana authored and th0th committed Mar 23, 2024
1 parent 525a555 commit 22efa9b
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 195 deletions.
2 changes: 1 addition & 1 deletion cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/alertmanager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
60 changes: 0 additions & 60 deletions matchers/compat/metrics.go

This file was deleted.

77 changes: 15 additions & 62 deletions matchers/compat/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand All @@ -57,65 +56,44 @@ 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)
}
}

// 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)
}
}

// 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)
Expand All @@ -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)
}
Expand All @@ -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)
Expand All @@ -168,15 +132,13 @@ 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
}
// 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
}
Expand All @@ -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.
Expand All @@ -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())
Expand All @@ -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
}
Expand Down
Loading

0 comments on commit 22efa9b

Please sign in to comment.