feat(dogstatsd_sink): support EXTRA_TAGS

When using the godogstats sink, previously the EXTRA_TAGS would not be emitted as datadog tags. Signed-off-by: Josh Jaques <[email protected]>
envoyproxy · Jun 19, 2024 · 6f15451 · 6f15451
1 parent 0ddd444
commit 6f15451
Show file tree

Hide file tree

Showing 3 changed files with 148 additions and 4 deletions.
diff --git a/README.md b/README.md
@@ -896,7 +896,7 @@ First, enable an extra mogrifier:
 
 Then, declare additional rules for the `DESCRIPTOR` mogrifier
 
-1. `DOG_STATSD_MOGRIFIER_HITS_PATTERN`: `^ratelimit\.service\.rate_limit\.(.*)\.(.*)\.(.*)$`
+1. `DOG_STATSD_MOGRIFIER_HITS_PATTERN`: `^ratelimit\.service\.rate_limit\.([^.]+)\.(.*)\.([^.]+)$`
 2. `DOG_STATSD_MOGRIFIER_HITS_NAME`: `ratelimit.service.rate_limit.$3`
 3. `DOG_STATSD_MOGRIFIER_HITS_TAGS`: `domain:$1,descriptor:$2`
 

diff --git a/src/godogstats/dogstatsd_sink.go b/src/godogstats/dogstatsd_sink.go
@@ -3,10 +3,12 @@ package godogstats
 import (
 	"regexp"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/DataDog/datadog-go/v5/statsd"
 	gostats "github.com/lyft/gostats"
+	logger "github.com/sirupsen/logrus"
 )
 
 type godogStatsSink struct {
@@ -65,18 +67,67 @@ func NewSink(opts ...goDogStatsSinkOption) (*godogStatsSink, error) {
 	return sink, nil
 }
 
-func (g *godogStatsSink) FlushCounter(name string, value uint64) {
+// separateExtraTags separates the metric name and tags from the combined serialized metric name.
+// e.g. given input: "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits.__COMMIT=12345.__DEPLOY=67890"
+// this should produce output: "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits", ["COMMIT:12345", "DEPLOY:67890"]
+// Aligns to how tags are serialized here https://github.com/lyft/gostats/blob/49e70f1b7932d146fecd991be04f8e1ad235452c/internal/tags/tags.go#L335
+func (g *godogStatsSink) separateExtraTags(name string) (string, []string) {
+	const (
+		prefix = "__"
+		sep    = "="
+		tagsep = "."
+	)
+
+	// find the position of the first __ if any
+	prefixPos := strings.Index(name, prefix)
+	if prefixPos == -1 {
+		return name, nil // no extra tags
+	}
+
+	// split the name and tags
+	tagString := name[prefixPos:]
+	shortName := name[:prefixPos-1]
+
+	// split the tags
+	tagPairs := strings.Split(tagString, tagsep)
+	tags := make([]string, 0, len(tagPairs))
+	for _, tagPair := range tagPairs {
+		tag := strings.Split(tagPair, sep)
+		if len(tag) != 2 {
+			logger.Debugf("godogstats sink found malformed extra tag: %v, string: %v", tag, name)
+			continue
+		}
+		tagName := tag[0]
+		if tagName[0] != '_' || tagName[1] != '_' {
+			logger.Debugf("godogstats sink found malformed extra tag with no dunder: %v, pair: %v", tagName, tagPair)
+			continue
+		}
+		tagName = tagName[2:]
+		tagValue := tag[1]
+		tags = append(tags, tagName+":"+tagValue)
+	}
+
+	return shortName, tags
+}
+
+func (g *godogStatsSink) mogrify(name string) (string, []string) {
+	name, extraTags := g.separateExtraTags(name)
 	name, tags := g.mogrifier.mogrify(name)
+	return name, append(extraTags, tags...)
+}
+
+func (g *godogStatsSink) FlushCounter(name string, value uint64) {
+	name, tags := g.mogrify(name)
 	g.client.Count(name, int64(value), tags, 1.0)
 }
 
 func (g *godogStatsSink) FlushGauge(name string, value uint64) {
-	name, tags := g.mogrifier.mogrify(name)
+	name, tags := g.mogrify(name)
 	g.client.Gauge(name, float64(value), tags, 1.0)
 }
 
 func (g *godogStatsSink) FlushTimer(name string, milliseconds float64) {
-	name, tags := g.mogrifier.mogrify(name)
+	name, tags := g.mogrify(name)
 	duration := time.Duration(milliseconds) * time.Millisecond
 	g.client.Timing(name, duration, tags, 1.0)
 }
diff --git a/src/godogstats/dogstatsd_sink_test.go b/src/godogstats/dogstatsd_sink_test.go
@@ -0,0 +1,93 @@
+package godogstats
+
+import (
+	"regexp"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSeparateExtraTags(t *testing.T) {
+	tests := []struct {
+		name         string
+		givenMetric  string
+		expectOutput string
+		expectTags   []string
+	}{
+		{
+			name:         "has extra tags",
+			givenMetric:  "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits.__COMMIT=12345.__DEPLOY=6890",
+			expectOutput: "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits",
+			expectTags:   []string{"COMMIT:12345", "DEPLOY:6890"},
+		},
+		{
+			name:         "no extra tags",
+			givenMetric:  "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits",
+			expectOutput: "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits",
+			expectTags:   nil,
+		},
+		{
+			name:         "invalid extra tags",
+			givenMetric:  "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits.__COMMIT",
+			expectOutput: "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits",
+			expectTags:   []string{},
+		},
+	}
+
+	g := &godogStatsSink{}
+	for _, tt := range tests {
+		actualName, actualTags := g.separateExtraTags(tt.givenMetric)
+
+		assert.Equal(t, tt.expectOutput, actualName)
+		assert.Equal(t, tt.expectTags, actualTags)
+	}
+}
+
+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"}
+			},
+		},
+	}
+
+	tests := []struct {
+		name         string
+		input        string
+		expectedName string
+		expectedTags []string
+	}{
+		{
+			name:         "mogrify with match and extra tags",
+			input:        "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits.__COMMIT=12345.__DEPLOY=67890",
+			expectedName: "custom.service.rate_limit.mongo_cps.database_users.total_hits",
+			expectedTags: []string{"COMMIT:12345", "DEPLOY:67890", "tag1:value1", "tag2:value2"},
+		},
+		{
+			name:         "mogrify with match without extra tags",
+			input:        "ratelimit.service.rate_limit.mongo_cps.database_users.total_hits",
+			expectedName: "custom.service.rate_limit.mongo_cps.database_users.total_hits",
+			expectedTags: []string{"tag1:value1", "tag2:value2"},
+		},
+		{
+			name:         "extra tags with no match",
+			input:        "foo.service.rate_limit.mongo_cps.database_users.total_hits.__COMMIT=12345.__DEPLOY=67890",
+			expectedName: "foo.service.rate_limit.mongo_cps.database_users.total_hits",
+			expectedTags: []string{"COMMIT:12345", "DEPLOY:67890"},
+		},
+		{
+			name:         "no mogrification",
+			input:        "other.metric.name",
+			expectedName: "other.metric.name",
+			expectedTags: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		actualName, actualTags := g.mogrify(tt.input)
+
+		assert.Equal(t, tt.expectedName, actualName)
+		assert.Equal(t, tt.expectedTags, actualTags)
+	}
+}