[coord] Guard against duplicate __rollup__ tags #3950
Conversation
Previously the tag merge logic could result in duplicate __rollup__ tags if the provided tags already had __rollup__. Additionally refactored to lazily merge the 2 sorted slices when calling Next(). This made it much easier to add deduping logic and I believe makes it easier to follow the code.
| } | ||
|
|
||
| func (p *rollupIDProvider) CurrentIndex() int { | ||
| if p.index >= 0 { |
There was a problem hiding this comment.
this was actually broken before, so it's intentionally not backwards compatible. it would produce 0, 0, 1, 2, ...
the addition to the test proves this.
| tags: []id.TagPair{ | ||
| { | ||
| Name: []byte("__name__"), | ||
| Value: []byte("http_requests"), |
There was a problem hiding this comment.
should we have a different value here to demonstrate which value is chosen?
Codecov Report
@@ Coverage Diff @@
## master #3950 +/- ##
======================================
Coverage 56.8% 56.8%
======================================
Files 555 555
Lines 63427 63427
======================================
Hits 36074 36074
Misses 24157 24157
Partials 3196 3196
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Previously the tag merge logic could result in duplicate rollup tags
if the provided tags already had rollup.
Additionally refactored to lazily merge the 2 sorted slices when calling
Next(). This made it much easier to add deduping logic and I believe
makes it easier to follow the code.
What this PR does / why we need it:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: