Update to latest mimir-prometheus #9508
Conversation
charleskorn
requested review from
stevesg,
grafanabot and
a team
as code owners
| // We currently omit checking the annotations due to a difference between the engines. | ||
| // This can be re-enabled once https://github.com/prometheus/prometheus/pull/14910 is vendored. | ||
| testutils.RequireEqualResults(t, expr, expectedResults, mimirResults, false) | ||
| testutils.RequireEqualResults(t, expr, expectedResults, mimirResults) |
There was a problem hiding this comment.
(I personally prefer to remove/tidy these kind of things in followups to keep vendor commits as close to just updating the vendored code as possible (of course there are times where we need to make changes to be compatible etc))
| // See https://github.com/prometheus/prometheus/pull/14821 for more discussion of this. | ||
| for i := range seriesData.Histograms { | ||
| if i > 0 && seriesData.Histograms[i].H == seriesData.Histograms[i-1].H { | ||
| // Previous point shares the same histogram instance, which we've already negated, so don't negate it again. |
There was a problem hiding this comment.
(Not for now, but I'm more and more thinking we should copy histograms for lookbacks. Seems so risky that we'll miss something 😬)
There was a problem hiding this comment.
Or rather than all lookbacks, perhaps copy histograms if they're used for successive points? Would be interesting to try and see the performance impact.
| @@ -15,16 +15,14 @@ import ( | |||
|
|
|||
| // Why do we do this rather than require.Equal(t, expected, actual)? | |||
| // It's possible that floating point values are slightly different due to imprecision, but require.Equal doesn't allow us to set an allowable difference. | |||
| func RequireEqualResults(t testing.TB, expr string, expected, actual *promql.Result, checkAnnotations bool) { | |||
| func RequireEqualResults(t testing.TB, expr string, expected, actual *promql.Result) { | |||
There was a problem hiding this comment.
I had intended to keep checkAnnotations in case we needed it for other cases, but it's easy enough to add back in.
Co-authored-by: George Krajcsovits <[email protected]>
charleskorn
force-pushed
the
charleskorn/update-prometheus
branch
from
e50f7fe to
ab018ff
Compare
What this PR does
This brings in five changes:
Which issue(s) this PR fixes or relates to
(none)
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.