metrics/filter's glob implementation is order dependent

[andrewmains12]

The filter syntax used by the aggregator is currently dependent on the order of alternatives listed in certain cases:

msg.{error, errors,success}

should match any of:

"msg.success", "msg.error", "msg.errors",

but actually fails to match msg.errors.

Reading through the code, this is due to the fact that our implementation proceeds after the first alternative matches, with no way of backing up on later failure.

// src/metrics/filters/filter.go:574
func (f *multiCharSequenceFilter) matches(val []byte) ([]byte, bool) {
	...
	for _, pattern := range f.patterns {
		if f.backwards && bytes.HasSuffix(val, pattern) {
			return val[:len(val)-len(pattern)], true
		}

		if !f.backwards && bytes.HasPrefix(val, pattern) {
			return val[len(pattern):], true
		}
	}

	return nil, false
}

That is, we match:

"msg."
"error"
// fail to match "s"!

Potential fix:

This is a bit difficult to fix with our current homegrown glob approach; to solve this properly, we need some form of backtracking (see here for a good explanation of an efficient approach, which is used by the regexp package). We may want to consider moving our implementation to either use filepath.Glob (or similar; that implementation special cases / and doesn't support {a,b} constructs, I don't think) or regexp (translate our glob syntax into a regex).

General Issues

1. What service is experiencing the issue? (M3Coordinator, M3DB, M3Aggregator, etc)

M3Aggregator

2. What is the configuration of the service? Please include any YAML files, as well as namespace / placement configuration (with any sensitive information anonymized if necessary).

N/A; discovered via unittest.

3. How are you using the service? For example, are you performing read/writes to the service via Prometheus, or are you using a custom script?

N/A; discovered via unittest.
4. Is there a reliable way to reproduce the behavior? If so, please provide detailed instructions.

See this unittest

[andrewmains12]

Hey @gibbscullen I'm going to reopen this; I think it's actually still an issue (unittest still fails post rebasing my branch). We happened to have a customer run into it as well (I think).

[abliqo]

See repro steps in #4125