MQE: fix functions that could return multiple series with the same labels #10533
Conversation
2312545 to
526732d
Compare
| require.NoError(t, err) | ||
| defer q.Close() | ||
|
|
||
| mimirResult := q.Exec(ctx) |
There was a problem hiding this comment.
I think we should consider passing this into Prometheus too so that we can verify we merge in the same way (as I think we've found a case previous where we don't).
Additionally, I think we should consider seeing if we can make this part of the gauntlet. ie make sure we have cases represented in there along with the selectors that would cause this to happen.
There was a problem hiding this comment.
I think we should consider passing this into Prometheus too so that we can verify we merge in the same way (as I think we've found a case previous where we don't).
I originally had this, but unfortunately a bunch of these cases fail with Prometheus' engine due to prometheus/prometheus#14695.
Additionally, I think we should consider seeing if we can make this part of the gauntlet. ie make sure we have cases represented in there along with the selectors that would cause this to happen.
I think adding these cases to the gauntlet would be tricky, and I think having a dedicated test for this specific problem helps make the issue (and the solution) clear for any future functions we add that are missing the deduplication step.
What this PR does
This PR fixes an issue with the
clamp,clamp_min,clamp_max,roundandhistogram_fractionfunctions in MQE: they could return multiple series with the same labels if multiple series with the same labels but different metric names were passed to the function.This PR also adds a test to ensure we catch issues like these when new functions are added in the future.
Which issue(s) this PR fixes or relates to
#10067
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.