Skip to content

Commit

Permalink
Fixing substring lookup, added tests (stripe#1012)
Browse files Browse the repository at this point in the history
When metrics are going through the globalization pipeline metrics from veneur local will have tags added to them by the `add_tags` functionality when these same metrics arrive at the globalization pipeline if veneur global has its own `add_tags` specified that overlap with those from veneur local we would end up with duplicate tags. See the original fix [here](stripe#1009).

As to why this PR: when accessing a substring of ft where `len(k)` is longer than `ft` itself we'll receive an out of bounds error, I was operating under the assumption they'd behave a lot like python strings where this behavior is valid:
```
Python 3.9.11 (main, Sep 13 2022, 16:05:34)
[Clang 13.1.6 (clang-1316.0.21.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> foo = "test"
>>> foo[0:20]
'test'
```
  • Loading branch information
andresgalindo-stripe authored Nov 30, 2022
1 parent 0f8f107 commit 2cc4ce6
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
2 changes: 1 addition & 1 deletion flusher.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func (s *Server) flushSink(

replaced := false
for i, ft := range filteredTags {
if ft[0:len(k)] == k {
if len(ft) >= len(k) && ft[0:len(k)] == k {
filteredTags[i] = tag
replaced = true
break
Expand Down
54 changes: 54 additions & 0 deletions flusher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,60 @@ func TestFlushWithAddTagsDedupes(t *testing.T) {
}
})

t.Run("WithAddTagsFilteredTagsShorterThanAddTags", func(t *testing.T) {
mockStatsd.EXPECT().Count("flushed_metrics", int64(0), []string{
"sink_name:channel",
"sink_kind:channel",
"status:skipped",
"veneurglobalonly:true",
}, 1.0)
mockStatsd.EXPECT().Count("flushed_metrics", int64(0), []string{
"sink_name:channel",
"sink_kind:channel",
"status:max_name_length",
"veneurglobalonly:true",
}, 1.0)
mockStatsd.EXPECT().Count("flushed_metrics", int64(0), []string{
"sink_name:channel",
"sink_kind:channel",
"status:max_tags",
"veneurglobalonly:true",
}, 1.0)
mockStatsd.EXPECT().Count("flushed_metrics", int64(0), []string{
"sink_name:channel",
"sink_kind:channel",
"status:max_tag_length",
"veneurglobalonly:true",
}, 1.0)
mockStatsd.EXPECT().Count("flushed_metrics", int64(1), []string{
"sink_name:channel",
"sink_kind:channel",
"status:flushed",
"veneurglobalonly:true",
}, 1.0)

server.Workers[0].PacketChan <- samplers.UDPMetric{
MetricKey: samplers.MetricKey{
Name: "test.metric",
Type: "counter",
},
Digest: 0,
Scope: samplers.LocalOnly,
Tags: []string{"foo:baz", "f:a"},
Value: 1.0,
SampleRate: 1.0,
}

result := <-channel
if assert.Len(t, result, 1) {
assert.Equal(t, "test.metric", result[0].Name)
if assert.Len(t, result[0].Tags, 3) {
assert.Equal(t, "foo:bar", result[0].Tags[0])
assert.Equal(t, "f:a", result[0].Tags[1])
}
}
})

t.Run("WithAddTagsMultiple", func(t *testing.T) {
mockStatsd.EXPECT().Count("flushed_metrics", int64(0), []string{
"sink_name:channel",
Expand Down

0 comments on commit 2cc4ce6

Please sign in to comment.