Fixing tagging on submetrics #609
Conversation
Codecov Report
@@ Coverage Diff @@
## master #609 +/- ##
==========================================
- Coverage 62.04% 62.03% -0.01%
==========================================
Files 97 97
Lines 7501 7510 +9
==========================================
+ Hits 4654 4659 +5
- Misses 2589 2593 +4
Partials 258 258
Continue to review full report at Codecov.
|
stats/stats.go
Outdated
| if st == other { | ||
| return true | ||
| } | ||
| if st == nil || other == nil || len(st.tags) < len(other.tags) { |
There was a problem hiding this comment.
Ah, actually, I'm not sure, but I think that if other == nil || len(other.tags) == 0, then we should always return true, since every set contains the empty set.
There was a problem hiding this comment.
actually, nil is not an empty set, but if you indeed pass an empty set you will get a true response.
I questioned myself that when implementing, then I went to a Ruby repl and tried something like [1,2,3].includes?(nil) and got false, so I decided to follow the same approach.
There was a problem hiding this comment.
I have no idea what ruby does, but in the case of SampleTags, a nil pointer is basically the same as an empty tag set. I'm not very familiar with how tagging of submetrics is supposed to work and if this is the desired behaviour though, just that mathematically the empty set is a subset of all sets.
There was a problem hiding this comment.
I don't really care about that. Let's make that return true then.
Fixes #590 by validating that sub metric tags are included in the sample tags.