Elasticsearch: Fix removing of empty settings from query in backend implementation (#59704)

* Elasticsearch: Fix removing of empty settings from query in backend implementation

* Update

* Update

* Update pkg/tsdb/elasticsearch/time_series_query.go

Co-authored-by: Gábor Farkas <gabor.farkas@gmail.com>

Co-authored-by: Gábor Farkas <gabor.farkas@gmail.com>
This commit is contained in:
Ivana Huckova 2022-12-05 16:06:55 +01:00 committed by GitHub
parent 3544250baf
commit e356a56741
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 12 additions and 5 deletions

View File

@ -454,7 +454,16 @@ func (p *timeSeriesQueryParser) parseMetrics(model *simplejson.Json) ([]*MetricA
metric.Hide = metricJSON.Get("hide").MustBool(false)
metric.ID = metricJSON.Get("id").MustString()
metric.PipelineAggregate = metricJSON.Get("pipelineAgg").MustString()
metric.Settings = simplejson.NewFromAny(metricJSON.Get("settings").MustMap())
// In legacy editors, we were storing empty settings values as "null"
// The new editor doesn't store empty strings at all
// We need to ensures backward compatibility with old queries and remove empty fields
settings := metricJSON.Get("settings").MustMap()
for k, v := range settings {
if v == "null" {
delete(settings, k)
}
}
metric.Settings = simplejson.NewFromAny(settings)
metric.Meta = simplejson.NewFromAny(metricJSON.Get("meta").MustMap())
metric.Type, err = metricJSON.Get("type").String()
if err != nil {

View File

@ -39,7 +39,6 @@ func TestExecuteTimeSeriesQuery(t *testing.T) {
require.Equal(t, dateHistogramAgg.ExtendedBounds.Min, fromMs)
require.Equal(t, dateHistogramAgg.ExtendedBounds.Max, toMs)
})
t.Run("Should clean settings from null values (from frontend tests)", func(t *testing.T) {
c := newFakeClient()
_, err := executeTsdbQuery(c, `{
@ -52,8 +51,7 @@ func TestExecuteTimeSeriesQuery(t *testing.T) {
firstLevel := sr.Aggs[0]
secondLevel := firstLevel.Aggregation.Aggs[0]
require.Equal(t, secondLevel.Aggregation.Aggregation.(*es.MetricAggregation).Settings["script"], "1")
// FIXME: This is a bug in implementation, missing is set to "null" instead of being removed
// require.Equal(t, secondLevel.Aggregation.Aggregation.(*es.MetricAggregation).Settings["missing"], nil)
require.NotContains(t, secondLevel.Aggregation.Aggregation.(*es.MetricAggregation).Settings, "missing")
})
t.Run("With multiple bucket aggs", func(t *testing.T) {

View File

@ -22,7 +22,7 @@ describe('ElasticQueryBuilder', () => {
// The following `missing: null as any` is because previous versions of the DS where
// storing null in the query model when inputting an empty string,
// which were then removed in the query builder.
// The new version doesn't store empty strings at all. This tests ensures backward compatinility.
// The new version doesn't store empty strings at all. This tests ensures backward compatibility.
metrics: [{ type: 'avg', id: '0', settings: { missing: null as any, script: '1' } }],
timeField: '@timestamp',
bucketAggs: [{ type: 'date_histogram', field: '@timestamp', id: '1' }],