fix(dogstatsd_sink): switch from pure map to list of key/value pairs (#…

…627)

The test for Two mogrifiers: First match has flaky results because the
underlying iteration order from the map is not guaranteed. Sometimes
matching first and sometimes matching second mogrifier.

Having a consistent mogrifier order is required for some types of
priority based manipulation, and that is how the keys are defined in the
environment, so changing the overall implementation to a slice to
maintain ordered iteration is the best option. And can be done by only
changing the internal private types.

Signed-off-by: Josh Jaques <[email protected]>

src/godogstats/dogstatsd_sink.go

@@ -38,9 +38,15 @@ func WithStatsdPort(port int) goDogStatsSinkOption {
 	}
 }
 
+// WithMogrifier adds a mogrifier to the sink. Map iteration order is randomized, to control order call multiple times.
 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,
+			})
+		}
 	}
 }
 

src/godogstats/dogstatsd_sink_test.go

@@ -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"}
+				},
 			},
 		},
 	}

src/godogstats/mogrifier_map.go

@@ -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                                      // the matcher determines whether a mogrifier should run on a metric at all
+	handler func(matches []string) (name string, tags []string) // the handler takes the list of matches, and returns metric name and list of tags
+}
+
+// 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 {
@@ -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
 	}
 

src/godogstats/mogrifier_map_test.go

@@ -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
+			},
 		},
 	}
 }