Skip to content
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

Support UTF-8 label matchers: Rename feature flags and make package public #3604

Merged
merged 3 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 27 additions & 27 deletions featurecontrol/featurecontrol.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,40 +23,40 @@ import (
)

const (
fcReceiverNameInMetrics = "receiver-name-in-metrics"
fcClassicMatchersParsing = "classic-matchers-parsing"
fcUTF8MatchersParsing = "utf8-matchers-parsing"
FeatureReceiverNameInMetrics = "receiver-name-in-metrics"
FeatureClassicMatchers = "classic-matchers"
gotjosh marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is changing the actual feature value that appears in configuration considered a breaking change? Won't any configurations that had the old toggle start to crash with Unknown option '%s' for --enable-feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't happen, we haven't made any public releases of Alertmanager with the old feature flags.

FeatureUTF8Matchers = "utf8-matchers"
)

var AllowedFlags = []string{
fcReceiverNameInMetrics,
fcClassicMatchersParsing,
fcUTF8MatchersParsing,
FeatureReceiverNameInMetrics,
FeatureClassicMatchers,
FeatureUTF8Matchers,
}

type Flagger interface {
EnableReceiverNamesInMetrics() bool
ClassicMatchersParsing() bool
UTF8MatchersParsing() bool
ClassicMatchers() bool
UTF8Matchers() bool
}

type Flags struct {
logger log.Logger
enableReceiverNamesInMetrics bool
classicMatchersParsing bool
utf8MatchersParsing bool
classicMatchers bool
utf8Matchers bool
}

func (f *Flags) EnableReceiverNamesInMetrics() bool {
return f.enableReceiverNamesInMetrics
}

func (f *Flags) ClassicMatchersParsing() bool {
return f.classicMatchersParsing
func (f *Flags) ClassicMatchers() bool {
return f.classicMatchers
}

func (f *Flags) UTF8MatchersParsing() bool {
return f.utf8MatchersParsing
func (f *Flags) UTF8Matchers() bool {
return f.utf8Matchers
}

type flagOption func(flags *Flags)
Expand All @@ -67,15 +67,15 @@ func enableReceiverNameInMetrics() flagOption {
}
}

func enableClassicMatchersParsing() flagOption {
func enableClassicMatchers() flagOption {
return func(configs *Flags) {
configs.classicMatchersParsing = true
configs.classicMatchers = true
}
}

func enableUTF8MatchersParsing() flagOption {
func enableUTF8Matchers() flagOption {
return func(configs *Flags) {
configs.utf8MatchersParsing = true
configs.utf8Matchers = true
}
}

Expand All @@ -89,14 +89,14 @@ func NewFlags(logger log.Logger, features string) (Flagger, error) {

for _, feature := range strings.Split(features, ",") {
switch feature {
case fcReceiverNameInMetrics:
case FeatureReceiverNameInMetrics:
opts = append(opts, enableReceiverNameInMetrics())
level.Warn(logger).Log("msg", "Experimental receiver name in metrics enabled")
case fcClassicMatchersParsing:
opts = append(opts, enableClassicMatchersParsing())
case FeatureClassicMatchers:
opts = append(opts, enableClassicMatchers())
level.Warn(logger).Log("msg", "Classic matchers parsing enabled")
case fcUTF8MatchersParsing:
opts = append(opts, enableUTF8MatchersParsing())
case FeatureUTF8Matchers:
opts = append(opts, enableUTF8Matchers())
level.Warn(logger).Log("msg", "UTF-8 matchers parsing enabled")
default:
return nil, fmt.Errorf("Unknown option '%s' for --enable-feature", feature)
Expand All @@ -107,8 +107,8 @@ func NewFlags(logger log.Logger, features string) (Flagger, error) {
opt(fc)
}

if fc.classicMatchersParsing && fc.utf8MatchersParsing {
return nil, errors.New("Both classic and UTF-8 matchers parsing is enabled, please choose one or remove the flag for both")
if fc.classicMatchers && fc.utf8Matchers {
return nil, errors.New("Both classic and UTF-8 matchers is enabled, please choose one or remove the flag for both")
}

return fc, nil
Expand All @@ -118,6 +118,6 @@ type NoopFlags struct{}

func (n NoopFlags) EnableReceiverNamesInMetrics() bool { return false }

func (n NoopFlags) ClassicMatchersParsing() bool { return false }
func (n NoopFlags) ClassicMatchers() bool { return false }

func (n NoopFlags) UTF8MatchersParsing() bool { return false }
func (n NoopFlags) UTF8Matchers() bool { return false }
4 changes: 2 additions & 2 deletions featurecontrol/featurecontrol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestFlags(t *testing.T) {
}{
{
name: "with only valid feature flags",
featureFlags: fcReceiverNameInMetrics,
featureFlags: FeatureReceiverNameInMetrics,
},
{
name: "with only invalid feature flags",
Expand All @@ -39,7 +39,7 @@ func TestFlags(t *testing.T) {
},
{
name: "with both, valid and invalid feature flags",
featureFlags: strings.Join([]string{fcReceiverNameInMetrics, "somethingbad"}, ","),
featureFlags: strings.Join([]string{FeatureReceiverNameInMetrics, "somethingbad"}, ","),
err: errors.New("Unknown option 'somethingbad' for --enable-feature"),
},
}
Expand Down
4 changes: 2 additions & 2 deletions matchers/compat/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ func Matchers(s string) (labels.Matchers, error) {

// InitFromFlags initializes the compat package from the flagger.
func InitFromFlags(l log.Logger, f featurecontrol.Flagger) {
if f.ClassicMatchersParsing() {
if f.ClassicMatchers() {
parseMatcher = classicMatcherParser(l)
parseMatchers = classicMatchersParser(l)
} else if f.UTF8MatchersParsing() {
} else if f.UTF8Matchers() {
parseMatcher = utf8MatcherParser(l)
parseMatchers = utf8MatchersParser(l)
} else {
Expand Down
Binary file added matchers/repl/repl
grobinson-grafana marked this conversation as resolved.
Show resolved Hide resolved
Binary file not shown.
Loading