Merge branch 'main' into andreas/graphite-alias-fix

pkg/tsdb/graphite/healthcheck.go

@@ -39,7 +39,7 @@ func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthReque
 
 	_, span := tracing.DefaultTracer().Start(ctx, "graphite healthcheck")
 	defer span.End()
-	graphiteReq, formData, _, err := s.createGraphiteRequest(ctx, healthCheckQuery, dsInfo)
+	graphiteReq, formData, _, _, err := s.createGraphiteRequest(ctx, healthCheckQuery, dsInfo)
 	if err != nil {
 		span.RecordError(err)
 		span.SetStatus(codes.Error, err.Error())
@@ -81,7 +81,7 @@ func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthReque
 		}
 	}()
 
-	_, err = s.toDataFrames(res, healthCheckQuery.RefID, false)
+	_, err = s.toDataFrames(res, healthCheckQuery.RefID, false, targetStr)
 	if err != nil {
 		span.RecordError(err)
 		span.SetStatus(codes.Error, err.Error())

pkg/tsdb/graphite/query.go

@@ -24,15 +24,16 @@ import (
 func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, dsInfo *datasourceInfo) (*backend.QueryDataResponse, error) {
 	emptyQueries := []string{}
 	graphiteQueries := map[string]struct {
-		req      *http.Request
-		formData url.Values
+		req       *http.Request
+		formData  url.Values
+		rawTarget string
 	}{}
 	// FromAlert header is defined in pkg/services/ngalert/models/constants.go
 	fromAlert := req.Headers["FromAlert"] == "true"
 	result := backend.NewQueryDataResponse()
 
 	for _, query := range req.Queries {
-		graphiteReq, formData, emptyQuery, err := s.createGraphiteRequest(ctx, query, dsInfo)
+		graphiteReq, formData, emptyQuery, target, err := s.createGraphiteRequest(ctx, query, dsInfo)
 		if err != nil {
 			result.Responses[query.RefID] = backend.ErrorResponseWithErrorSource(err)
 			return result, nil
@@ -44,11 +45,13 @@ func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, d
 		}
 
 		graphiteQueries[query.RefID] = struct {
-			req      *http.Request
-			formData url.Values
+			req       *http.Request
+			formData  url.Values
+			rawTarget string
 		}{
-			req:      graphiteReq,
-			formData: formData,
+			req:       graphiteReq,
+			formData:  formData,
+			rawTarget: target,
 		}
 	}
 
@@ -99,7 +102,7 @@ func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, d
 			}
 		}()
 
-		queryFrames, err := s.toDataFrames(res, refId, fromAlert)
+		queryFrames, err := s.toDataFrames(res, refId, fromAlert, graphiteReq.rawTarget)
 		if err != nil {
 			span.RecordError(err)
 			span.SetStatus(codes.Error, err.Error())
@@ -147,7 +150,7 @@ func (s *Service) processQuery(query backend.DataQuery) (string, *GraphiteQuery,
 	return target, nil, queryJSON.IsMetricTank, nil
 }
 
-func (s *Service) createGraphiteRequest(ctx context.Context, query backend.DataQuery, dsInfo *datasourceInfo) (*http.Request, url.Values, *GraphiteQuery, error) {
+func (s *Service) createGraphiteRequest(ctx context.Context, query backend.DataQuery, dsInfo *datasourceInfo) (*http.Request, url.Values, *GraphiteQuery, string, error) {
 	/*
 		graphite doc about from and until, with sdk we are getting absolute instead of relative time
 		https://graphite-api.readthedocs.io/en/latest/api.html#from-until
@@ -163,12 +166,12 @@ func (s *Service) createGraphiteRequest(ctx context.Context, query backend.DataQ
 
 	target, emptyQuery, isMetricTank, err := s.processQuery(query)
 	if err != nil {
-		return nil, formData, nil, err
+		return nil, formData, nil, "", err
 	}
 
 	if emptyQuery != nil {
 		s.logger.Debug("Graphite", "empty query target", emptyQuery)
-		return nil, formData, emptyQuery, nil
+		return nil, formData, emptyQuery, "", nil
 	}
 
 	formData["target"] = []string{target}
@@ -188,20 +191,23 @@ func (s *Service) createGraphiteRequest(ctx context.Context, query backend.DataQ
 		QueryParams: params,
 	})
 	if err != nil {
-		return nil, formData, nil, err
+		return nil, formData, nil, "", err
 	}
 
-	return graphiteReq, formData, emptyQuery, nil
+	return graphiteReq, formData, emptyQuery, target, nil
 }
 
-func (s *Service) toDataFrames(response *http.Response, refId string, fromAlert bool) (frames data.Frames, error error) {
+func (s *Service) toDataFrames(response *http.Response, refId string, fromAlert bool, rawTarget string) (frames data.Frames, error error) {
 	responseData, err := s.parseResponse(response)
 	if err != nil {
 		return nil, err
 	}
 
+	aliasRegex := regexp.MustCompile(`(alias\(|aliasByMetric|aliasByNode|aliasByTags|aliasQuery|aliasSub)`)
+
 	frames = data.Frames{}
 	for _, series := range responseData {
+		aliasMatch := aliasRegex.MatchString(rawTarget)
 		timeVector := make([]time.Time, 0, len(series.DataPoints))
 		values := make([]*float64, 0, len(series.DataPoints))
 
@@ -217,7 +223,9 @@ func (s *Service) toDataFrames(response *http.Response, refId string, fromAlert
 		tags := make(map[string]string)
 		for name, value := range series.Tags {
 			if name == "name" {
-				if fromAlert {
+				// Queries with aliases should use the target as the name
+				// to ensure multi-dimensional queries are distinguishable from each other
+				if fromAlert || aliasMatch {
 					value = series.Target
 				}
 			}

pkg/tsdb/graphite/query_test.go

@@ -182,7 +182,7 @@ func TestConvertResponses(t *testing.T) {
 		expectedFrames := data.Frames{expectedFrame}
 
 		httpResponse := &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body))}
-		dataFrames, err := service.toDataFrames(httpResponse, refId, false)
+		dataFrames, err := service.toDataFrames(httpResponse, refId, false, "target")
 
 		require.NoError(t, err)
 		if !reflect.DeepEqual(expectedFrames, dataFrames) {
@@ -196,7 +196,7 @@ func TestConvertResponses(t *testing.T) {
 		body := `
 		[
 			{
-				"target": "aliasedTarget(target)",
+				"target": "alias(target)",
 				"tags": { "name": "target", "fooTag": "fooValue", "barTag": "barValue", "int": 100, "float": 3.14 },
 				"datapoints": [[50, 1], [null, 2], [100, 3]]
 			}
@@ -211,13 +211,13 @@ func TestConvertResponses(t *testing.T) {
 				"barTag": "barValue",
 				"int":    "100",
 				"float":  "3.14",
-				"name":   "target",
-			}, []*float64{&a, nil, &b}).SetConfig(&data.FieldConfig{DisplayNameFromDS: "aliasedTarget(target)"}),
+				"name":   "alias(target)",
+			}, []*float64{&a, nil, &b}).SetConfig(&data.FieldConfig{DisplayNameFromDS: "alias(target)"}),
 		).SetMeta(&data.FrameMeta{Type: data.FrameTypeTimeSeriesMulti})
 		expectedFrames := data.Frames{expectedFrame}
 
 		httpResponse := &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body))}
-		dataFrames, err := service.toDataFrames(httpResponse, refId, false)
+		dataFrames, err := service.toDataFrames(httpResponse, refId, false, "alias(target)")
 
 		require.NoError(t, err)
 		if !reflect.DeepEqual(expectedFrames, dataFrames) {
@@ -240,7 +240,7 @@ func TestConvertResponses(t *testing.T) {
 		expectedFrames := data.Frames{}
 
 		httpResponse := &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body))}
-		dataFrames, err := service.toDataFrames(httpResponse, refId, false)
+		dataFrames, err := service.toDataFrames(httpResponse, refId, false, "")
 
 		require.NoError(t, err)
 		if !reflect.DeepEqual(expectedFrames, dataFrames) {
@@ -281,7 +281,7 @@ func TestConvertResponses(t *testing.T) {
 		expectedFrames := data.Frames{expectedFrame}
 
 		httpResponse := &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body))}
-		dataFrames, err := service.toDataFrames(httpResponse, refId, false)
+		dataFrames, err := service.toDataFrames(httpResponse, refId, false, "target")
 
 		require.NoError(t, err)
 		if !reflect.DeepEqual(expectedFrames, dataFrames) {
@@ -295,7 +295,7 @@ func TestConvertResponses(t *testing.T) {
 		body := `
 		[
 			{
-				"target": "aliasedTarget(target)",
+				"target": "alias(target)",
 				"tags": { "name": "target", "fooTag": "fooValue", "barTag": "barValue", "int": 100, "float": 3.14 },
 				"datapoints": [[50, 1], [null, 2], [100, 3]]
 			}
@@ -310,13 +310,13 @@ func TestConvertResponses(t *testing.T) {
 				"barTag": "barValue",
 				"int":    "100",
 				"float":  "3.14",
-				"name":   "aliasedTarget(target)",
-			}, []*float64{&a, nil, &b}).SetConfig(&data.FieldConfig{DisplayNameFromDS: "aliasedTarget(target)"}),
+				"name":   "alias(target)",
+			}, []*float64{&a, nil, &b}).SetConfig(&data.FieldConfig{DisplayNameFromDS: "alias(target)"}),
 		).SetMeta(&data.FrameMeta{Type: data.FrameTypeTimeSeriesMulti})
 		expectedFrames := data.Frames{expectedFrame}
 
 		httpResponse := &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body))}
-		dataFrames, err := service.toDataFrames(httpResponse, refId, true)
+		dataFrames, err := service.toDataFrames(httpResponse, refId, true, "alias(target)")
 
 		require.NoError(t, err)
 		if !reflect.DeepEqual(expectedFrames, dataFrames) {
@@ -738,6 +738,119 @@ Error: Target not found
 	}
 }
 
+func TestAliasMatching(t *testing.T) {
+	service := &Service{
+		logger: backend.Logger,
+	}
+
+	testCases := []struct {
+		name              string
+		target            string
+		tagsName          string
+		fromAlert         bool
+		expectedLabelName string
+	}{
+		{
+			name:              "alias() function sets name tag to target",
+			target:            "alias(stats.counters.web.hits, 'Web Hits')",
+			tagsName:          "stats.counters.web.hits",
+			fromAlert:         false,
+			expectedLabelName: "alias(stats.counters.web.hits, 'Web Hits')",
+		},
+		{
+			name:              "aliasByNode() function sets name tag to target",
+			target:            "aliasByNode(stats.counters.web.hits, 2)",
+			tagsName:          "stats.counters.web.hits",
+			fromAlert:         false,
+			expectedLabelName: "aliasByNode(stats.counters.web.hits, 2)",
+		},
+		{
+			name:              "aliasByMetric() function sets name tag to target",
+			target:            "aliasByMetric(stats.counters.web.hits)",
+			tagsName:          "stats.counters.web.hits",
+			fromAlert:         false,
+			expectedLabelName: "aliasByMetric(stats.counters.web.hits)",
+		},
+		{
+			name:              "aliasByTags() function sets name tag to target",
+			target:            "aliasByTags(stats.counters.web.hits, 'host')",
+			tagsName:          "stats.counters.web.hits",
+			fromAlert:         false,
+			expectedLabelName: "aliasByTags(stats.counters.web.hits, 'host')",
+		},
+		{
+			name:              "aliasSub() function sets name tag to target",
+			target:            "aliasSub(stats.counters.web.hits, 'stats', 'metrics')",
+			tagsName:          "stats.counters.web.hits",
+			fromAlert:         false,
+			expectedLabelName: "aliasSub(stats.counters.web.hits, 'stats', 'metrics')",
+		},
+		{
+			name:              "aliasQuery() function sets name tag to target",
+			target:            "aliasQuery(stats.counters.web.hits, 'SELECT name FROM hosts')",
+			tagsName:          "stats.counters.web.hits",
+			fromAlert:         false,
+			expectedLabelName: "aliasQuery(stats.counters.web.hits, 'SELECT name FROM hosts')",
+		},
+		{
+			name:              "no alias function keeps original name tag",
+			target:            "stats.counters.web.hits",
+			tagsName:          "stats.counters.web.hits",
+			fromAlert:         false,
+			expectedLabelName: "stats.counters.web.hits",
+		},
+		{
+			name:              "fromAlert overrides name tag even without alias",
+			target:            "stats.counters.web.hits",
+			tagsName:          "original.name",
+			fromAlert:         true,
+			expectedLabelName: "stats.counters.web.hits",
+		},
+		{
+			name:              "nested alias function matches",
+			target:            "sumSeries(alias(stats.counters.*.hits, 'Hits'))",
+			tagsName:          "stats.counters.web.hits",
+			fromAlert:         false,
+			expectedLabelName: "sumSeries(alias(stats.counters.*.hits, 'Hits'))",
+		},
+		{
+			name:              "alias in metric path should not match",
+			target:            "stats.alias.web.hits",
+			tagsName:          "stats.alias.web.hits",
+			fromAlert:         false,
+			expectedLabelName: "stats.alias.web.hits",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			body := fmt.Sprintf(`[
+				{
+					"target": %q,
+					"tags": { "name": %q, "host": "server1" },
+					"datapoints": [[100, 1609459200]]
+				}
+			]`, tc.target, tc.tagsName)
+
+			httpResponse := &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body))}
+			dataFrames, err := service.toDataFrames(httpResponse, "A", tc.fromAlert, tc.target)
+
+			require.NoError(t, err)
+			require.Len(t, dataFrames, 1)
+
+			frame := dataFrames[0]
+			require.GreaterOrEqual(t, len(frame.Fields), 2)
+
+			valueField := frame.Fields[1]
+			require.NotNil(t, valueField.Labels)
+
+			actualName, ok := valueField.Labels["name"]
+			require.True(t, ok, "name label should exist")
+			assert.Equal(t, tc.expectedLabelName, actualName, "name label should match expected value")
+		})
+	}
+}
+
 func TestParseGraphiteError(t *testing.T) {
 	tests := []struct {
 		name     string