From 49e70f1b7932d146fecd991be04f8e1ad235452c Mon Sep 17 00:00:00 2001 From: Jake Kaufman Date: Thu, 16 May 2024 14:49:16 -0400 Subject: [PATCH] Add reserved_words counter (#168) We've run into an issue internally where certain applications are emitting stats with reserved words. We don't want to panic or otherwise harm the app, but we do want to be able to alert / notify owners when this is happening. We create an additional counter here to track how often reserved_tags are being sent to statsd without blocking them. --- .github/workflows/actions.yml | 3 +-- .golangci.yaml | 1 - stat_handler_wrapper_test.go | 2 +- stats.go | 22 ++++++++++++++++++++++ stats_test.go | 27 +++++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 4 deletions(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index f96f502..a6b2ca5 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -8,8 +8,7 @@ jobs: strategy: matrix: go-version: - - 1.19.x - - 1.20.x + - 1.22.x name: run-tests runs-on: ubuntu-latest diff --git a/.golangci.yaml b/.golangci.yaml index c80ff33..6cea8ad 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -6,7 +6,6 @@ linters: - gosimple - govet - ineffassign - - megacheck - misspell - nakedret - revive diff --git a/stat_handler_wrapper_test.go b/stat_handler_wrapper_test.go index 571a934..29d9dce 100644 --- a/stat_handler_wrapper_test.go +++ b/stat_handler_wrapper_test.go @@ -92,7 +92,7 @@ func TestHTTPHandler_WrapResponse(t *testing.T) { h := NewStatHandler( NewStore(NewNullSink(), false), - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})).(*httpHandler) + http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})).(*httpHandler) for i, test := range tests { tc := test diff --git a/stats.go b/stats.go index 4143dc6..9f167bd 100644 --- a/stats.go +++ b/stats.go @@ -346,6 +346,17 @@ type statStore struct { sink Sink } +var ReservedTagWords = map[string]bool{"asg": true, "az": true, "backend": true, "canary": true, "host": true, "period": true, "region": true, "shard": true, "window": true, "source": true, "project": true, "facet": true, "envoyservice": true} + +func (s *statStore) validateTags(tags map[string]string) { + for k := range tags { + if _, ok := ReservedTagWords[k]; ok { + // Keep track of how many times a reserved tag is used + s.NewCounter("reserved_tag").Inc() + } + } +} + func (s *statStore) StartContext(ctx context.Context, ticker *time.Ticker) { for { select { @@ -403,6 +414,7 @@ func (s *statStore) Scope(name string) Scope { } func (s *statStore) ScopeWithTags(name string, tags map[string]string) Scope { + s.validateTags(tags) return newSubScope(s, name, tags) } @@ -422,6 +434,7 @@ func (s *statStore) NewCounter(name string) Counter { } func (s *statStore) NewCounterWithTags(name string, tags map[string]string) Counter { + s.validateTags(tags) return s.newCounter(tagspkg.SerializeTags(name, tags)) } @@ -438,6 +451,7 @@ func (s *statStore) NewPerInstanceCounter(name string, tags map[string]string) C if _, found := tags["_f"]; found { return s.NewCounterWithTags(name, tags) } + s.validateTags(tags) return s.newCounterWithTagSet(name, tagspkg.TagSet(nil).MergePerInstanceTags(tags)) } @@ -457,6 +471,7 @@ func (s *statStore) NewGauge(name string) Gauge { } func (s *statStore) NewGaugeWithTags(name string, tags map[string]string) Gauge { + s.validateTags(tags) return s.newGauge(tagspkg.SerializeTags(name, tags)) } @@ -471,6 +486,7 @@ func (s *statStore) NewPerInstanceGauge(name string, tags map[string]string) Gau if _, found := tags["_f"]; found { return s.NewGaugeWithTags(name, tags) } + s.validateTags(tags) return s.newGaugeWithTagSet(name, tagspkg.TagSet(nil).MergePerInstanceTags(tags)) } @@ -490,6 +506,7 @@ func (s *statStore) NewMilliTimer(name string) Timer { } func (s *statStore) NewMilliTimerWithTags(name string, tags map[string]string) Timer { + s.validateTags(tags) return s.newTimer(tagspkg.SerializeTags(name, tags), time.Millisecond) } @@ -498,6 +515,7 @@ func (s *statStore) NewTimer(name string) Timer { } func (s *statStore) NewTimerWithTags(name string, tags map[string]string) Timer { + s.validateTags(tags) return s.newTimer(tagspkg.SerializeTags(name, tags), time.Microsecond) } @@ -512,6 +530,7 @@ func (s *statStore) NewPerInstanceTimer(name string, tags map[string]string) Tim if _, found := tags["_f"]; found { return s.NewTimerWithTags(name, tags) } + s.validateTags(tags) return s.newTimerWithTagSet(name, tagspkg.TagSet(nil).MergePerInstanceTags(tags), time.Microsecond) } @@ -522,6 +541,7 @@ func (s *statStore) NewPerInstanceMilliTimer(name string, tags map[string]string if _, found := tags["_f"]; found { return s.NewMilliTimerWithTags(name, tags) } + s.validateTags(tags) return s.newTimerWithTagSet(name, tagspkg.TagSet(nil).MergePerInstanceTags(tags), time.Millisecond) } @@ -540,6 +560,7 @@ func (s *subScope) Scope(name string) Scope { } func (s *subScope) ScopeWithTags(name string, tags map[string]string) Scope { + s.registry.validateTags(tags) return &subScope{ registry: s.registry, name: joinScopes(s.name, name), @@ -599,6 +620,7 @@ func (s *subScope) NewMilliTimerWithTags(name string, tags map[string]string) Ti } func (s *subScope) NewPerInstanceMilliTimer(name string, tags map[string]string) Timer { + s.registry.validateTags(tags) return s.registry.newTimerWithTagSet(joinScopes(s.name, name), s.tags.MergePerInstanceTags(tags), time.Millisecond) } diff --git a/stats_test.go b/stats_test.go index 66ca14f..37dbbed 100644 --- a/stats_test.go +++ b/stats_test.go @@ -69,6 +69,33 @@ func TestStatsStartContext(_ *testing.T) { wg.Wait() } +// Ensure we create a counter and increment it for reserved tags +func TestValidateTags(t *testing.T) { + // Ensure we don't create a counter without reserved tags + sink := &testStatSink{} + store := NewStore(sink, true) + store.NewCounter("test").Inc() + store.Flush() + + expected := "test:1|c" + counter := sink.record + if !strings.Contains(counter, expected) { + t.Error("wanted counter value of test:1|c, got", counter) + } + + // A reserved tag should trigger adding the reserved_tag counter + sink = &testStatSink{} + store = NewStore(sink, true) + store.NewCounterWithTags("test", map[string]string{"host": "i"}).Inc() + store.Flush() + + expected = "reserved_tag:1|c\ntest.__host=i:1|c" + counter = sink.record + if !strings.Contains(counter, expected) { + t.Error("wanted counter value of test.___f=i:1|c, got", counter) + } +} + // Ensure timers and timespans are working func TestTimer(t *testing.T) { testDuration := time.Duration(9800000)