Fix TraceQL results caching bug for floats ending in .0 #4539
Conversation
carles-grafana
requested review from
joe-elliott,
mdisibio,
mapno,
yvrhdn,
zalegrala,
electron0zero,
ie-pham,
stoewer and
javiermolinar
as code owners
carles-grafana
force-pushed
the
fix-traceql-float-caching
branch
4 times, most recently
from
4444827 to
60eb3c6
Compare
| @@ -352,7 +352,7 @@ func TestQuantileOverTime(t *testing.T) { | |||
| // Output series with quantiles per foo | |||
| // Prom labels are sorted alphabetically, traceql labels maintain original order. | |||
| out := SeriesSet{ | |||
| `{p="0", span.foo="bar"}`: TimeSeries{ | |||
| `{p="0.0", span.foo="bar"}`: TimeSeries{ | |||
There was a problem hiding this comment.
not sure if this is wanted @mdisibio
There was a problem hiding this comment.
Ah interesting side effect. It's ok. The actual label value is the same NewStaticFloat(0)
| @@ -352,7 +352,7 @@ func TestQuantileOverTime(t *testing.T) { | |||
| // Output series with quantiles per foo | |||
| // Prom labels are sorted alphabetically, traceql labels maintain original order. | |||
| out := SeriesSet{ | |||
| `{p="0", span.foo="bar"}`: TimeSeries{ | |||
| `{p="0.0", span.foo="bar"}`: TimeSeries{ | |||
There was a problem hiding this comment.
Ah interesting side effect. It's ok. The actual label value is the same NewStaticFloat(0)
| {arg: 10.0, want: "10"}, | ||
| {arg: -1.0, want: "-1.0"}, | ||
| {arg: 0.0, want: "0.0"}, | ||
| {arg: 10.0, want: "10.0"}, |
There was a problem hiding this comment.
Can you add a few more cases: not ending in .0, and something with the e scientific notation?
There was a problem hiding this comment.
Sorry my comment wasn't clear. The cases I meant were a value like 12.34 to test the ., and something huge that outputs the e to test that case. 12.34 is halfway covered by the second case, but let's get clear cases where input == output. The 0 case isn't needed cause that's testing the int path which is already covered.
There was a problem hiding this comment.
I've added the 2 test cases and another one I think wasn't covered
carles-grafana
force-pushed
the
fix-traceql-float-caching
branch
2 times, most recently
from
26ff094 to
4bf18b9
Compare
carles-grafana
force-pushed
the
fix-traceql-float-caching
branch
from
4bf18b9 to
9def255
Compare
What this PR does:
Which issue(s) this PR fixes:
Fixes #4528
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]