diff --git a/pkg/expr/nodes.go b/pkg/expr/nodes.go index 0846eee6ea0..de664501944 100644 --- a/pkg/expr/nodes.go +++ b/pkg/expr/nodes.go @@ -20,6 +20,9 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" ) +// label that is used when all mathexp.Series have 0 labels to make them identifiable by labels. The value of this label is extracted from value field names +const nameLabelName = "__name__" + var ( logger = log.New("expr") ) @@ -295,7 +298,6 @@ func convertDataFramesToResults(ctx context.Context, frames data.Frames, datasou return "no-data", mathexp.Results{Values: mathexp.Values{mathexp.NewNoData()}}, nil } - vals := make([]mathexp.Value, 0) var dt data.FrameType dt, useDataplane, _ := shouldUseDataplane(frames, logger, s.features.IsEnabled(featuremgmt.FlagDisableSSEDataplane)) if useDataplane { @@ -325,6 +327,7 @@ func convertDataFramesToResults(ctx context.Context, frames data.Frames, datasou if err != nil { return "", mathexp.Results{}, err } + vals := make([]mathexp.Value, 0, len(numberSet)) for _, n := range numberSet { vals = append(vals, n) } @@ -334,16 +337,33 @@ func convertDataFramesToResults(ctx context.Context, frames data.Frames, datasou } } + filtered := make([]*data.Frame, 0, len(frames)) + totalLen := 0 for _, frame := range frames { + schema := frame.TimeSeriesSchema() // Check for TimeSeriesTypeNot in InfluxDB queries. A data frame of this type will cause // the WideToMany() function to error out, which results in unhealthy alerts. // This check should be removed once inconsistencies in data source responses are solved. - if frame.TimeSeriesSchema().Type == data.TimeSeriesTypeNot && datasourceType == datasources.DS_INFLUXDB { + if schema.Type == data.TimeSeriesTypeNot && datasourceType == datasources.DS_INFLUXDB { logger.Warn("Ignoring InfluxDB data frame due to missing numeric fields") continue } - var series []mathexp.Series - series, err := WideToMany(frame) + if schema.Type != data.TimeSeriesTypeWide { + return "", mathexp.Results{}, fmt.Errorf("input data must be a wide series but got type %s (input refid)", schema.Type) + } + filtered = append(filtered, frame) + totalLen += len(schema.ValueIndices) + } + + if len(filtered) == 0 { + return "no data", mathexp.Results{Values: mathexp.Values{mathexp.NoData{Frame: frames[0]}}}, nil + } + + maybeFixerFn := checkIfSeriesNeedToBeFixed(filtered, datasourceType) + + vals := make([]mathexp.Value, 0, totalLen) + for _, frame := range filtered { + series, err := WideToMany(frame, maybeFixerFn) if err != nil { return "", mathexp.Results{}, err } @@ -351,14 +371,17 @@ func convertDataFramesToResults(ctx context.Context, frames data.Frames, datasou vals = append(vals, ser) } } - - return "series set", mathexp.Results{ - Values: vals, // TODO vals can be empty. Should we replace with no-data? + dataType := "single frame series" + if len(filtered) > 1 { + dataType = "multi frame series" + } + return dataType, mathexp.Results{ + Values: vals, }, nil } func isAllFrameVectors(datasourceType string, frames data.Frames) bool { - if datasourceType != "prometheus" { + if datasourceType != datasources.DS_PROMETHEUS { return false } allVector := false @@ -466,7 +489,7 @@ func extractNumberSet(frame *data.Frame) ([]mathexp.Number, error) { // is created for each value type column of wide frame. // // This might not be a good idea long term, but works now as an adapter/shim. -func WideToMany(frame *data.Frame) ([]mathexp.Series, error) { +func WideToMany(frame *data.Frame, fixSeries func(series mathexp.Series, valueField *data.Field)) ([]mathexp.Series, error) { tsSchema := frame.TimeSeriesSchema() if tsSchema.Type != data.TimeSeriesTypeWide { return nil, fmt.Errorf("input data must be a wide series but got type %s (input refid)", tsSchema.Type) @@ -477,10 +500,13 @@ func WideToMany(frame *data.Frame) ([]mathexp.Series, error) { if err != nil { return nil, err } + if fixSeries != nil { + fixSeries(s, frame.Fields[tsSchema.ValueIndices[0]]) + } return []mathexp.Series{s}, nil } - series := []mathexp.Series{} + series := make([]mathexp.Series, 0, len(tsSchema.ValueIndices)) for _, valIdx := range tsSchema.ValueIndices { l := frame.Rows() f := data.NewFrameOfFieldTypes(frame.Name, l, frame.Fields[tsSchema.TimeIndex].Type(), frame.Fields[valIdx].Type()) @@ -500,8 +526,79 @@ func WideToMany(frame *data.Frame) ([]mathexp.Series, error) { if err != nil { return nil, err } + if fixSeries != nil { + fixSeries(s, frame.Fields[valIdx]) + } series = append(series, s) } return series, nil } + +// checkIfSeriesNeedToBeFixed scans all value fields of all provided frames and determines whether the resulting mathexp.Series +// needs to be updated so each series could be identifiable by labels. +// NOTE: applicable only to only datasources.DS_GRAPHITE and datasources.DS_TESTDATA data sources +// returns a function that patches the mathexp.Series with information from data.Field from which it was created if the all series need to be fixed. Otherwise, returns nil +func checkIfSeriesNeedToBeFixed(frames []*data.Frame, datasourceType string) func(series mathexp.Series, valueField *data.Field) { + if !(datasourceType == datasources.DS_GRAPHITE || datasourceType == datasources.DS_TESTDATA) { + return nil + } + + // get all value fields + var valueFields []*data.Field + for _, frame := range frames { + tsSchema := frame.TimeSeriesSchema() + for _, index := range tsSchema.ValueIndices { + field := frame.Fields[index] + // if at least one value field contains labels, the result does not need to be fixed. + if len(field.Labels) > 0 { + return nil + } + if valueFields == nil { + valueFields = make([]*data.Field, 0, len(frames)*len(tsSchema.ValueIndices)) + } + valueFields = append(valueFields, field) + } + } + + // selectors are in precedence order. + nameSelectors := []func(f *data.Field) string{ + func(f *data.Field) string { + if f == nil || f.Config == nil { + return "" + } + return f.Config.DisplayNameFromDS + }, + func(f *data.Field) string { + if f == nil || f.Config == nil { + return "" + } + return f.Config.DisplayName + }, + func(f *data.Field) string { + return f.Name + }, + } + + // now look for the first selector that would make all value fields be unique + for _, selector := range nameSelectors { + names := make(map[string]struct{}, len(valueFields)) + good := true + for _, field := range valueFields { + name := selector(field) + if _, ok := names[name]; ok || name == "" { + good = false + break + } + names[name] = struct{}{} + } + if good { + return func(series mathexp.Series, valueField *data.Field) { + series.SetLabels(data.Labels{ + nameLabelName: selector(valueField), + }) + } + } + } + return nil +} diff --git a/pkg/expr/nodes_test.go b/pkg/expr/nodes_test.go index 9661a663302..58ea03b60bb 100644 --- a/pkg/expr/nodes_test.go +++ b/pkg/expr/nodes_test.go @@ -1,10 +1,22 @@ package expr import ( + "context" "errors" + "fmt" "testing" + "time" + "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/expr/mathexp" + "github.com/grafana/grafana/pkg/infra/log/logtest" + "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/setting" ) type expectedError struct{} @@ -40,3 +52,232 @@ func TestQueryError_Unwrap(t *testing.T) { assert.True(t, errors.As(e, &expectedAsError)) }) } + +func TestCheckIfSeriesNeedToBeFixed(t *testing.T) { + createFrame := func(m ...func(field *data.Field)) []*data.Frame { + f := data.NewFrame("", + data.NewField("Time", nil, []time.Time{})) + for i := 0; i < 100; i++ { + fld := data.NewField(fmt.Sprintf("fld-%d", i), nil, []*float64{}) + fld.Config = &data.FieldConfig{} + for _, change := range m { + change(fld) + } + f.Fields = append(f.Fields, fld) + } + return []*data.Frame{f} + } + withLabels := func(field *data.Field) { + field.Labels = map[string]string{ + "field": field.Name, + } + } + withDisplayNameFromDS := func(field *data.Field) { + field.Config.DisplayNameFromDS = fmt.Sprintf("dnds-%s", field.Name) + } + withDisplayName := func(field *data.Field) { + field.Config.DisplayName = fmt.Sprintf("dn-%s", field.Name) + } + withoutName := func(field *data.Field) { + field.Name = "" + } + + getLabelName := func(f func(series mathexp.Series, valueField *data.Field)) string { + s := mathexp.NewSeries("A", nil, 0) + field := &data.Field{ + Name: "Name", + Config: &data.FieldConfig{ + DisplayNameFromDS: "DisplayNameFromDS", + DisplayName: "DisplayName", + }, + } + f(s, field) + return s.GetLabels()[nameLabelName] + } + + testCases := []struct { + name string + frames []*data.Frame + expectedName string + }{ + { + name: "should return nil if at least one value field has labels", + frames: createFrame(withLabels, withDisplayNameFromDS, withDisplayName), + expectedName: "", + }, + { + name: "should return nil if names are empty", + frames: createFrame(withoutName), + expectedName: "", + }, + { + name: "should return patcher with DisplayNameFromDS first", + frames: createFrame(withDisplayNameFromDS, withDisplayName), + expectedName: "DisplayNameFromDS", + }, + { + name: "should return patcher with DisplayName if DisplayNameFromDS is not unique", + frames: func() []*data.Frame { + frames := createFrame(withDisplayNameFromDS, withDisplayName) + f := frames[0] + f.Fields[2].Config.DisplayNameFromDS = "test" + f.Fields[3].Config.DisplayNameFromDS = "test" + return frames + }(), + expectedName: "DisplayName", + }, + { + name: "should return patcher with DisplayName if is empty", + frames: createFrame(withDisplayName), + expectedName: "DisplayName", + }, + { + name: "should return patcher with Name if DisplayName and DisplayNameFromDS are not unique", + frames: func() []*data.Frame { + frames := createFrame(withDisplayNameFromDS, withDisplayName) + f := frames[0] + f.Fields[1].Config.DisplayNameFromDS = f.Fields[2].Config.DisplayNameFromDS + f.Fields[1].Config.DisplayName = f.Fields[2].Config.DisplayName + return frames + }(), + expectedName: "Name", + }, + { + name: "should return patcher with Name if DisplayName and DisplayNameFromDS are empty", + frames: createFrame(), + expectedName: "Name", + }, + { + name: "should return nil if all fields are not unique", + frames: func() []*data.Frame { + frames := createFrame(withDisplayNameFromDS, withDisplayName) + f := frames[0] + f.Fields[1].Config.DisplayNameFromDS = f.Fields[2].Config.DisplayNameFromDS + f.Fields[1].Config.DisplayName = f.Fields[2].Config.DisplayName + f.Fields[1].Name = f.Fields[2].Name + return frames + }(), + expectedName: "", + }, + } + + supportedDatasources := []string{ + datasources.DS_GRAPHITE, + datasources.DS_TESTDATA, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + for _, datasource := range supportedDatasources { + fixer := checkIfSeriesNeedToBeFixed(tc.frames, datasource) + if tc.expectedName == "" { + require.Nil(t, fixer) + } else { + require.Equal(t, tc.expectedName, getLabelName(fixer)) + } + } + }) + } +} + +func TestConvertDataFramesToResults(t *testing.T) { + s := &Service{ + cfg: setting.NewCfg(), + features: &featuremgmt.FeatureManager{}, + tracer: tracing.InitializeTracerForTest(), + metrics: newMetrics(nil), + } + + t.Run("should add name label if no labels and specific data source", func(t *testing.T) { + supported := []string{datasources.DS_GRAPHITE, datasources.DS_TESTDATA} + t.Run("when only field name is specified", func(t *testing.T) { + t.Run("use value field names if one frame - many series", func(t *testing.T) { + supported := []string{datasources.DS_GRAPHITE, datasources.DS_TESTDATA} + + frames := []*data.Frame{ + data.NewFrame("test", + data.NewField("time", nil, []time.Time{time.Unix(1, 0)}), + data.NewField("test-value1", nil, []*float64{fp(2)}), + data.NewField("test-value2", nil, []*float64{fp(2)})), + } + + for _, dtype := range supported { + t.Run(dtype, func(t *testing.T) { + resultType, res, err := convertDataFramesToResults(context.Background(), frames, dtype, s, &logtest.Fake{}) + require.NoError(t, err) + assert.Equal(t, "single frame series", resultType) + require.Len(t, res.Values, 2) + + var names []string + for _, value := range res.Values { + require.IsType(t, mathexp.Series{}, value) + lbls := value.GetLabels() + require.Contains(t, lbls, nameLabelName) + names = append(names, lbls[nameLabelName]) + } + require.EqualValues(t, []string{"test-value1", "test-value2"}, names) + }) + } + }) + t.Run("should use frame name if one frame - one series", func(t *testing.T) { + frames := []*data.Frame{ + data.NewFrame("test-frame1", + data.NewField("time", nil, []time.Time{time.Unix(1, 0)}), + data.NewField("test-value1", nil, []*float64{fp(2)})), + data.NewFrame("test-frame2", + data.NewField("time", nil, []time.Time{time.Unix(1, 0)}), + data.NewField("test-value2", nil, []*float64{fp(2)})), + } + + for _, dtype := range supported { + t.Run(dtype, func(t *testing.T) { + resultType, res, err := convertDataFramesToResults(context.Background(), frames, dtype, s, &logtest.Fake{}) + require.NoError(t, err) + assert.Equal(t, "multi frame series", resultType) + require.Len(t, res.Values, 2) + + var names []string + for _, value := range res.Values { + require.IsType(t, mathexp.Series{}, value) + lbls := value.GetLabels() + require.Contains(t, lbls, nameLabelName) + names = append(names, lbls[nameLabelName]) + } + require.EqualValues(t, []string{"test-frame1", "test-frame2"}, names) + }) + } + }) + }) + t.Run("should use fields DisplayNameFromDS when it is unique", func(t *testing.T) { + f1 := data.NewField("test-value1", nil, []*float64{fp(2)}) + f1.Config = &data.FieldConfig{DisplayNameFromDS: "test-value1"} + f2 := data.NewField("test-value2", nil, []*float64{fp(2)}) + f2.Config = &data.FieldConfig{DisplayNameFromDS: "test-value2"} + frames := []*data.Frame{ + data.NewFrame("test-frame1", + data.NewField("time", nil, []time.Time{time.Unix(1, 0)}), + f1), + data.NewFrame("test-frame2", + data.NewField("time", nil, []time.Time{time.Unix(1, 0)}), + f2), + } + + for _, dtype := range supported { + t.Run(dtype, func(t *testing.T) { + resultType, res, err := convertDataFramesToResults(context.Background(), frames, dtype, s, &logtest.Fake{}) + require.NoError(t, err) + assert.Equal(t, "multi frame series", resultType) + require.Len(t, res.Values, 2) + + var names []string + for _, value := range res.Values { + require.IsType(t, mathexp.Series{}, value) + lbls := value.GetLabels() + require.Contains(t, lbls, nameLabelName) + names = append(names, lbls[nameLabelName]) + } + require.EqualValues(t, []string{"test-value1", "test-value2"}, names) + }) + } + }) + }) +} diff --git a/pkg/expr/service_test.go b/pkg/expr/service_test.go index cce41cf3ef8..6129a6bd89c 100644 --- a/pkg/expr/service_test.go +++ b/pkg/expr/service_test.go @@ -26,7 +26,7 @@ import ( func TestService(t *testing.T) { dsDF := data.NewFrame("test", data.NewField("time", nil, []time.Time{time.Unix(1, 0)}), - data.NewField("value", nil, []*float64{fp(2)})) + data.NewField("value", data.Labels{"test": "label"}, []*float64{fp(2)})) me := &mockEndpoint{ Frames: []*data.Frame{dsDF}, @@ -78,7 +78,7 @@ func TestService(t *testing.T) { bDF := data.NewFrame("", data.NewField("Time", nil, []time.Time{time.Unix(1, 0)}), - data.NewField("B", nil, []*float64{fp(4)})) + data.NewField("B", data.Labels{"test": "label"}, []*float64{fp(4)})) bDF.RefID = "B" bDF.SetMeta(&data.FrameMeta{ Type: data.FrameTypeTimeSeriesMulti, diff --git a/pkg/services/datasources/models.go b/pkg/services/datasources/models.go index 49ef0e03fa5..b472b0f597d 100644 --- a/pkg/services/datasources/models.go +++ b/pkg/services/datasources/models.go @@ -28,6 +28,7 @@ const ( DS_ES_OPEN_DISTRO = "grafana-es-open-distro-datasource" DS_ES_OPENSEARCH = "grafana-opensearch-datasource" DS_AZURE_MONITOR = "grafana-azure-monitor-datasource" + DS_TESTDATA = "testdata" // CustomHeaderName is the prefix that is used to store the name of a custom header. CustomHeaderName = "httpHeaderName" // CustomHeaderValue is the prefix that is used to store the value of a custom header. diff --git a/pkg/services/ngalert/backtesting/eval_data.go b/pkg/services/ngalert/backtesting/eval_data.go index 97a98c9e856..cd0fecdccff 100644 --- a/pkg/services/ngalert/backtesting/eval_data.go +++ b/pkg/services/ngalert/backtesting/eval_data.go @@ -21,7 +21,7 @@ type dataEvaluator struct { } func newDataEvaluator(refID string, frame *data.Frame) (*dataEvaluator, error) { - series, err := expr.WideToMany(frame) + series, err := expr.WideToMany(frame, nil) if err != nil { return nil, err }