Skip to content

Commit

Permalink
fix(dogstatsd_sink): switch from pure map to list of key/value pairs
Browse files Browse the repository at this point in the history
This fixes an issue where the test for Two mogrifiers: First match
has flaky results because the underlying match order in the map
is not guaranteed.

Having a consistent mogrifier order is required for some types of
manipulation, so changing the overall implementation to a list is the best
option.

Signed-off-by: Josh Jaques <[email protected]>
  • Loading branch information
JDeuce committed Jun 20, 2024
1 parent 00d7d6c commit 8fa135d
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 22 deletions.
7 changes: 6 additions & 1 deletion src/godogstats/dogstatsd_sink.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,12 @@ func WithStatsdPort(port int) goDogStatsSinkOption {

func WithMogrifier(mogrifiers map[*regexp.Regexp]func([]string) (string, []string)) goDogStatsSinkOption {
return func(g *godogStatsSink) {
g.mogrifier = mogrifiers
for m, h := range mogrifiers {
g.mogrifier = append(g.mogrifier, mogrifierEntry{
matcher: m,
handler: h,
})
}
}
}

Expand Down
7 changes: 5 additions & 2 deletions src/godogstats/dogstatsd_sink_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ func TestSeparateExtraTags(t *testing.T) {
func TestSinkMogrify(t *testing.T) {
g := &godogStatsSink{
mogrifier: mogrifierMap{
regexp.MustCompile(`^ratelimit\.(.*)$`): func(matches []string) (string, []string) {
return "custom." + matches[1], []string{"tag1:value1", "tag2:value2"}
{
matcher: regexp.MustCompile(`^ratelimit\.(.*)$`),
handler: func(matches []string) (string, []string) {
return "custom." + matches[1], []string{"tag1:value1", "tag2:value2"}
},
},
},
}
Expand Down
39 changes: 24 additions & 15 deletions src/godogstats/mogrifier_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@ var varFinder = regexp.MustCompile(`\$\d+`) // matches $0, $1, etc.

const envPrefix = "DOG_STATSD_MOGRIFIER" // prefix for environment variables

// mogrifierMap is a map of regular expressions to functions that mogrify a name and return tags
type mogrifierMap map[*regexp.Regexp]func([]string) (string, []string)
type mogrifierEntry struct {
matcher *regexp.Regexp
handler func([]string) (string, []string)
}

// mogrifierMap is an ordered map of regular expressions to functions that mogrify a name and return tags
type mogrifierMap []mogrifierEntry

// makePatternHandler returns a function that replaces $0, $1, etc. in the pattern with the corresponding match
func makePatternHandler(pattern string) func([]string) string {
Expand Down Expand Up @@ -73,32 +78,36 @@ func newMogrifierMapFromEnv(keys []string) (mogrifierMap, error) {
}
}

mogrifiers[re] = func(matches []string) (string, []string) {
name := nameHandler(matches)
tags := make([]string, 0, len(tagHandlers))
for tagKey, handler := range tagHandlers {
tagValue := handler(matches)
tags = append(tags, tagKey+":"+tagValue)
}
return name, tags
}
mogrifiers = append(mogrifiers, mogrifierEntry{
matcher: re,
handler: func(matches []string) (string, []string) {
name := nameHandler(matches)
tags := make([]string, 0, len(tagHandlers))
for tagKey, handler := range tagHandlers {
tagValue := handler(matches)
tags = append(tags, tagKey+":"+tagValue)
}
return name, tags
},
},
)

}
return mogrifiers, nil
}

// mogrify applies the first mogrifier in the map that matches the name
func (m mogrifierMap) mogrify(name string) (string, []string) {
func (m *mogrifierMap) mogrify(name string) (string, []string) {
if m == nil {
return name, nil
}
for matcher, mogrifier := range m {
matches := matcher.FindStringSubmatch(name)
for _, mogrifier := range *m {
matches := mogrifier.matcher.FindStringSubmatch(name)
if len(matches) == 0 {
continue
}

mogrifiedName, tags := mogrifier(matches)
mogrifiedName, tags := mogrifier.handler(matches)
return mogrifiedName, tags
}

Expand Down
11 changes: 7 additions & 4 deletions src/godogstats/mogrifier_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ import (

func testMogrifier() mogrifierMap {
return mogrifierMap{
regexp.MustCompile(`^ratelimit\.service\.rate_limit\.(.*)\.(.*)\.(.*)$`): func(matches []string) (string, []string) {
name := "ratelimit.service.rate_limit." + matches[3]
tags := []string{"domain:" + matches[1], "descriptor:" + matches[2]}
return name, tags
{
matcher: regexp.MustCompile(`^ratelimit\.service\.rate_limit\.(.*)\.(.*)\.(.*)$`),
handler: func(matches []string) (string, []string) {
name := "ratelimit.service.rate_limit." + matches[3]
tags := []string{"domain:" + matches[1], "descriptor:" + matches[2]}
return name, tags
},
},
}
}
Expand Down

0 comments on commit 8fa135d

Please sign in to comment.