Elasticsearch: Always use fixed_interval #50297
Conversation
gabor
added
add to changelog
breaking change
gabor
requested review from
a team and
Elfo404
and removed request for
a team and
Elfo404
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/21367 |
gabor
force-pushed
the
gabor/elastic-fixed-interval
branch
from
f6ae506
to
1d09b29
Compare
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/21371 |
| @@ -763,16 +763,12 @@ describe('ElasticQueryBuilder', () => { | |||
| extended_bounds: { max: '$timeTo', min: '$timeFrom' }, | |||
There was a problem hiding this comment.
since you are touching this file and we removed support for anything <7.10, i think it would be nice to change (at the top of this file this:
const builder = new ElasticQueryBuilder({ timeField: '@timestamp', esVersion: '2.0.0' });
const builder5x = new ElasticQueryBuilder({ timeField: '@timestamp', esVersion: '5.0.0' });
const builder56 = new ElasticQueryBuilder({ timeField: '@timestamp', esVersion: '5.6.0' });
const builder6x = new ElasticQueryBuilder({ timeField: '@timestamp', esVersion: '6.0.0' });
const builder7x = new ElasticQueryBuilder({ timeField: '@timestamp', esVersion: '7.0.0' });
const builder77 = new ElasticQueryBuilder({ timeField: '@timestamp', esVersion: '7.7.0' });
const builder8 = new ElasticQueryBuilder({ timeField: '@timestamp', esVersion: '8.0.0' });
const allBuilders = [builder, builder5x, builder56, builder6x, builder7x, builder77, builder8];to
const builder710 = new ElasticQueryBuilder({ timeField: '@timestamp', esVersion: '7.10.0' });
const builder8 = new ElasticQueryBuilder({ timeField: '@timestamp', esVersion: '8.0.0' });
const allBuilders = [builder710, builder8];
wdyt?
There was a problem hiding this comment.
in general i agree, but i did the change and now got a bunch of errors below :) (some tests are done on builder, which is now gone, same for builder7x... i don't know right now if i should just delete the builder7x tests or adjust them etc.)
in short ,it seems some more work is needed in query_builder.test.ts, so if you're ok with it, i'd let it be for a later time.
gabor
force-pushed
the
gabor/elastic-fixed-interval
branch
from
1d09b29
to
688199b
Compare
|
Hi, |
|
hi, the fix will be released in grafana9.1.0 . |
|
Hi, When setting a "Min Interval" value less than 365days, it is working as expected. However if you sent a longer duration to "Min Interval" (e.g. 365d), the Interval is automatically converted to years.
Thanks in advance. |
|
hi @alimli , could you please create this report as a separate github issue? it will make it easier for us to handle it. thanks! |
|
Hi, This broke one of my workflows, where I relied on I 'd be fine with trying to write the code to support it (hopefully with a nice UI/UX), but before I dive in that rabbit hole (which per #19911 (comment) could be a deep one), it would be nice to know if the result would have any chance of being merged. Thanks! |
|
hi @akosiaris , thank you for your feedback. before code starts being written, we should find out what exactly needs to be done, and how exactly the user-interface should behave. i created a feature-request discussion for it in #55034, if you could add your use-case, that would be great. |
Fixes #19911
in previous grafana versions, for Elasticsearch7.x we used the
intervalattribute to specify the interval-value. the problem is, this can becomefixed_intervalorcalendar_intervalbased on the interval-value, so for example, when the user resized the dashboard-window, this might trigger a different interval-handling. this is not good, so we are switching to always usefixed_intervalto provide consistent results. this is technically a breaking change, but it is a change to a better situation.(NOTE: in elastic8, we already always use
fixed_interval.)how to test:
make devenv sources=elastic elastic_verison=7.10.07.10. (simply copy the values from thegdev-elasticdatasource, and change the elastic-version select-box). let's name itelastictestelastictestdatasource"fixed_interval"there, and you should not see"interval"thereelastictestdatasource"fixed_interval"there, and you should not see"interval"thereRelease notice breaking change
In Elasticsearch versions 7.x, to specify the interval-value we used the
intervalproperty. In Grafana 9.1.0 we switched to use thefixed_intervalproperty. This makes it to be the same as in Elasticsearch versions 8.x, also this provides a more consistent experience,fixed_intervalis a better match to Grafana's time invervals. For most situations this will not cause any visible change to query results.