From 8d64ea930d371e4b676d49b2884f073f99fbd869 Mon Sep 17 00:00:00 2001 From: Jake Kaufman Date: Thu, 16 May 2024 14:14:56 -0400 Subject: [PATCH 1/4] Add reserved_words counter 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. --- stats.go | 22 ++++++++++++++++++++++ stats_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/stats.go b/stats.go index 4143dc6..90fd189 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..0296bbb 100644 --- a/stats_test.go +++ b/stats_test.go @@ -69,6 +69,35 @@ 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) From 859f8dbc5eb03d3aebdf7e9ba598cd866dbc0e93 Mon Sep 17 00:00:00 2001 From: Jake Kaufman Date: Thu, 16 May 2024 14:28:47 -0400 Subject: [PATCH 2/4] FUMPT --- stats.go | 2 +- stats_test.go | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/stats.go b/stats.go index 90fd189..9f167bd 100644 --- a/stats.go +++ b/stats.go @@ -349,7 +349,7 @@ type statStore struct { 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 { + 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() diff --git a/stats_test.go b/stats_test.go index 0296bbb..37dbbed 100644 --- a/stats_test.go +++ b/stats_test.go @@ -71,7 +71,6 @@ func TestStatsStartContext(_ *testing.T) { // 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) @@ -95,7 +94,6 @@ func TestValidateTags(t *testing.T) { if !strings.Contains(counter, expected) { t.Error("wanted counter value of test.___f=i:1|c, got", counter) } - } // Ensure timers and timespans are working From 0eec15abf3010b7af03b945d2ce46daa54211796 Mon Sep 17 00:00:00 2001 From: Jake Kaufman Date: Thu, 16 May 2024 14:35:19 -0400 Subject: [PATCH 3/4] Update go, fix golangci-lint config --- .github/workflows/actions.yml | 3 +-- .golangci.yaml | 1 - 2 files changed, 1 insertion(+), 3 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 From eb5752f6a0640cfe8a1ca52762c0a483963b9985 Mon Sep 17 00:00:00 2001 From: Jake Kaufman Date: Thu, 16 May 2024 14:41:01 -0400 Subject: [PATCH 4/4] Fix lint --- stat_handler_wrapper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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