CloudMonitoring: Use response unit instead of metric descriptor unit (#32928)

* improve unit detection * Update docs/sources/datasources/google-cloud-monitoring/_index.md Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com> * goimports * fix typo * golint fixes * Update docs/sources/datasources/google-cloud-monitoring/_index.md Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com> * Update docs/sources/datasources/google-cloud-monitoring/_index.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Update docs/sources/datasources/google-cloud-monitoring/_index.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Update docs/sources/datasources/google-cloud-monitoring/_index.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Update docs/sources/datasources/google-cloud-monitoring/_index.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * pr feedback * Update docs/sources/datasources/google-cloud-monitoring/_index.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com> Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com> Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
2025-02-25 18:55:37 -06:00 · 2021-04-14 16:41:02 +02:00
parent 9f82eac833
commit dac9393061
8 changed files with 50 additions and 127 deletions
--- a/pkg/tsdb/cloudmonitoring/cloudmonitoring.go
+++ b/pkg/tsdb/cloudmonitoring/cloudmonitoring.go
@@ -48,6 +48,7 @@ var (
 		"us":      "µs",
 		"ms":      "ms",
 		"ns":      "ns",
+		"%":       "percent",
 		"percent": "percent",
 		"MiBy":    "mbytes",
 		"By/s":    "Bps",
@@ -157,8 +158,6 @@ func (e *Executor) executeTimeSeriesQuery(ctx context.Context, tsdbQuery plugins
 		return plugins.DataResponse{}, err
 	}

-	unit := e.resolvePanelUnitFromQueries(queryExecutors)
-
 	for _, queryExecutor := range queryExecutors {
 		queryRes, resp, executedQueryString, err := queryExecutor.run(ctx, tsdbQuery, e)
 		if err != nil {
@@ -170,43 +169,11 @@ func (e *Executor) executeTimeSeriesQuery(ctx context.Context, tsdbQuery plugins
 		}

 		result.Results[queryExecutor.getRefID()] = queryRes
-
-		if len(unit) > 0 {
-			frames, _ := queryRes.Dataframes.Decoded()
-			for i := range frames {
-				if frames[i].Fields[1].Config == nil {
-					frames[i].Fields[1].Config = &data.FieldConfig{}
-				}
-				frames[i].Fields[1].Config.Unit = unit
-			}
-			queryRes.Dataframes = plugins.NewDecodedDataFrames(frames)
-		}
-		result.Results[queryExecutor.getRefID()] = queryRes
 	}

 	return result, nil
 }

-func (e *Executor) resolvePanelUnitFromQueries(executors []cloudMonitoringQueryExecutor) string {
-	if len(executors) == 0 {
-		return ""
-	}
-	unit := executors[0].getUnit()
-	if len(executors) > 1 {
-		for _, query := range executors[1:] {
-			if query.getUnit() != unit {
-				return ""
-			}
-		}
-	}
-	if len(unit) > 0 {
-		if val, ok := cloudMonitoringUnitMappings[unit]; ok {
-			return val
-		}
-	}
-	return ""
-}
-
 func (e *Executor) buildQueryExecutors(tsdbQuery plugins.DataQuery) ([]cloudMonitoringQueryExecutor, error) {
 	cloudMonitoringQueryExecutors := []cloudMonitoringQueryExecutor{}

@@ -282,7 +249,6 @@ func (e *Executor) buildQueryExecutors(tsdbQuery plugins.DataQuery) ([]cloudMoni
 		target = params.Encode()
 		cmtsf.Target = target
 		cmtsf.Params = params
-		cmtsf.Unit = q.MetricQuery.Unit

 		if setting.Env == setting.Dev {
 			slog.Debug("CloudMonitoring request", "params", params)
@@ -601,7 +567,7 @@ func unmarshalResponse(res *http.Response) (cloudMonitoringResponse, error) {
 	return data, nil
 }

-func addConfigData(frames data.Frames, dl string) data.Frames {
+func addConfigData(frames data.Frames, dl string, unit string) data.Frames {
 	for i := range frames {
 		if frames[i].Fields[1].Config == nil {
 			frames[i].Fields[1].Config = &data.FieldConfig{}
@@ -612,6 +578,11 @@ func addConfigData(frames data.Frames, dl string) data.Frames {
 			URL:         dl,
 		}
 		frames[i].Fields[1].Config.Links = append(frames[i].Fields[1].Config.Links, deepLink)
+		if len(unit) > 0 {
+			if val, ok := cloudMonitoringUnitMappings[unit]; ok {
+				frames[i].Fields[1].Config.Unit = val
+			}
+		}
 	}
 	return frames
 }
--- a/pkg/tsdb/cloudmonitoring/cloudmonitoring_test.go
+++ b/pkg/tsdb/cloudmonitoring/cloudmonitoring_test.go
@@ -893,77 +893,34 @@ func TestCloudMonitoring(t *testing.T) {
 				assert.Equal(t, "select_slo_compliance(\"projects/test-proj/services/test-service/serviceLevelObjectives/test-slo\")", frames[0].Fields[1].Name)
 			})
 		})
-	})

-	t.Run("Parse cloud monitoring unit", func(t *testing.T) {
-		t.Run("when there is only one query", func(t *testing.T) {
-			t.Run("and cloud monitoring unit does not have a corresponding grafana unit", func(t *testing.T) {
-				executors := []cloudMonitoringQueryExecutor{
-					&cloudMonitoringTimeSeriesFilter{Params: url.Values{}, ProjectName: "test-proj", Selector: "select_slo_compliance", Service: "test-service",
-						Slo: "test-slo", Unit: "megaseconds"},
-				}
-				unit := executor.resolvePanelUnitFromQueries(executors)
-				assert.Equal(t, "", unit)
+		t.Run("Parse cloud monitoring unit", func(t *testing.T) {
+			t.Run("when mapping is found a unit should be specified on the field config", func(t *testing.T) {
+				data, err := loadTestFile("./test-data/1-series-response-agg-one-metric.json")
+				require.NoError(t, err)
+				assert.Equal(t, 1, len(data.TimeSeries))
+
+				res := &plugins.DataQueryResult{Meta: simplejson.New(), RefID: "A"}
+				query := &cloudMonitoringTimeSeriesFilter{Params: url.Values{}}
+				err = query.parseResponse(res, data, "")
+				require.NoError(t, err)
+				frames, err := res.Dataframes.Decoded()
+				require.NoError(t, err)
+				assert.Equal(t, "Bps", frames[0].Fields[1].Config.Unit)
 			})

-			t.Run("and cloud monitoring unit has a corresponding grafana unit", func(t *testing.T) {
-				for key, element := range cloudMonitoringUnitMappings {
-					queries := []cloudMonitoringQueryExecutor{
-						&cloudMonitoringTimeSeriesFilter{Params: url.Values{}, ProjectName: "test-proj", Selector: "select_slo_compliance", Service: "test-service",
-							Slo: "test-slo", Unit: key},
-					}
-					unit := executor.resolvePanelUnitFromQueries(queries)
-					assert.Equal(t, element, unit)
-				}
-			})
-		})
+			t.Run("when mapping is found a unit should be specified on the field config", func(t *testing.T) {
+				data, err := loadTestFile("./test-data/2-series-response-no-agg.json")
+				require.NoError(t, err)
+				assert.Equal(t, 3, len(data.TimeSeries))

-		t.Run("when there are more than one query", func(t *testing.T) {
-			t.Run("and all target units are the same", func(t *testing.T) {
-				for key, element := range cloudMonitoringUnitMappings {
-					queries := []cloudMonitoringQueryExecutor{
-						&cloudMonitoringTimeSeriesFilter{
-							Params: url.Values{}, ProjectName: "test-proj", Selector: "select_slo_compliance",
-							Service: "test-service1", Slo: "test-slo", Unit: key,
-						},
-						&cloudMonitoringTimeSeriesFilter{
-							Params: url.Values{}, ProjectName: "test-proj", Selector: "select_slo_compliance",
-							Service: "test-service2", Slo: "test-slo", Unit: key,
-						},
-					}
-					unit := executor.resolvePanelUnitFromQueries(queries)
-					assert.Equal(t, element, unit)
-				}
-			})
-
-			t.Run("and all target units are the same but does not have grafana mappings", func(t *testing.T) {
-				queries := []cloudMonitoringQueryExecutor{
-					&cloudMonitoringTimeSeriesFilter{
-						Params: url.Values{}, ProjectName: "test-proj", Selector: "select_slo_compliance",
-						Service: "test-service1", Slo: "test-slo", Unit: "megaseconds",
-					},
-					&cloudMonitoringTimeSeriesFilter{
-						Params: url.Values{}, ProjectName: "test-proj", Selector: "select_slo_compliance",
-						Service: "test-service2", Slo: "test-slo", Unit: "megaseconds",
-					},
-				}
-				unit := executor.resolvePanelUnitFromQueries(queries)
-				assert.Equal(t, "", unit)
-			})
-
-			t.Run("and all target units are not the same", func(t *testing.T) {
-				queries := []cloudMonitoringQueryExecutor{
-					&cloudMonitoringTimeSeriesFilter{
-						Params: url.Values{}, ProjectName: "test-proj", Selector: "select_slo_compliance",
-						Service: "test-service1", Slo: "test-slo", Unit: "bit",
-					},
-					&cloudMonitoringTimeSeriesFilter{
-						Params: url.Values{}, ProjectName: "test-proj", Selector: "select_slo_compliance",
-						Service: "test-service2", Slo: "test-slo", Unit: "min",
-					},
-				}
-				unit := executor.resolvePanelUnitFromQueries(queries)
-				assert.Equal(t, "", unit)
+				res := &plugins.DataQueryResult{Meta: simplejson.New(), RefID: "A"}
+				query := &cloudMonitoringTimeSeriesFilter{Params: url.Values{}}
+				err = query.parseResponse(res, data, "")
+				require.NoError(t, err)
+				frames, err := res.Dataframes.Decoded()
+				require.NoError(t, err)
+				assert.Equal(t, "", frames[0].Fields[1].Config.Unit)
 			})
 		})

--- a/pkg/tsdb/cloudmonitoring/test-data/1-series-response-agg-one-metric.json
+++ b/pkg/tsdb/cloudmonitoring/test-data/1-series-response-agg-one-metric.json
@@ -42,5 +42,6 @@
        }
      ]
    }
-  ]
+  ],
+  "unit": "By/s"
 }
--- a/pkg/tsdb/cloudmonitoring/test-data/2-series-response-no-agg.json
+++ b/pkg/tsdb/cloudmonitoring/test-data/2-series-response-no-agg.json
@@ -141,5 +141,6 @@
        }
      ]
    }
-  ]
+  ],
+  "unit": "10^2.%"
 }
--- a/pkg/tsdb/cloudmonitoring/time_series_filter.go
+++ b/pkg/tsdb/cloudmonitoring/time_series_filter.go
@@ -224,7 +224,7 @@ func (timeSeriesFilter *cloudMonitoringTimeSeriesFilter) parseResponse(queryRes
 	}
 	if len(response.TimeSeries) > 0 {
 		dl := timeSeriesFilter.buildDeepLink()
-		frames = addConfigData(frames, dl)
+		frames = addConfigData(frames, dl, response.Unit)
 	}

 	queryRes.Dataframes = plugins.NewDecodedDataFrames(frames)
@@ -392,7 +392,3 @@ func setDisplayNameAsFieldName(f *data.Field) {
 func (timeSeriesFilter *cloudMonitoringTimeSeriesFilter) getRefID() string {
 	return timeSeriesFilter.RefID
 }
-
-func (timeSeriesFilter *cloudMonitoringTimeSeriesFilter) getUnit() string {
-	return timeSeriesFilter.Unit
-}
--- a/pkg/tsdb/cloudmonitoring/time_series_query.go
+++ b/pkg/tsdb/cloudmonitoring/time_series_query.go
@@ -247,7 +247,7 @@ func (timeSeriesQuery cloudMonitoringTimeSeriesQuery) parseResponse(queryRes *pl
 	}
 	if len(response.TimeSeriesData) > 0 {
 		dl := timeSeriesQuery.buildDeepLink()
-		frames = addConfigData(frames, dl)
+		frames = addConfigData(frames, dl, response.Unit)
 	}

 	queryRes.Dataframes = plugins.NewDecodedDataFrames(frames)
@@ -375,7 +375,3 @@ func (timeSeriesQuery cloudMonitoringTimeSeriesQuery) buildDeepLink() string {
 func (timeSeriesQuery cloudMonitoringTimeSeriesQuery) getRefID() string {
 	return timeSeriesQuery.RefID
 }
-
-func (timeSeriesQuery *cloudMonitoringTimeSeriesQuery) getUnit() string {
-	return timeSeriesQuery.Unit
-}
--- a/pkg/tsdb/cloudmonitoring/types.go
+++ b/pkg/tsdb/cloudmonitoring/types.go
@@ -16,7 +16,6 @@ type (
 		parseToAnnotations(queryRes *plugins.DataQueryResult, data cloudMonitoringResponse, title string, text string, tags string) error
 		buildDeepLink() string
 		getRefID() string
-		getUnit() string
 	}

 	// Used to build time series filters
@@ -30,7 +29,6 @@ type (
 		Selector    string
 		Service     string
 		Slo         string
-		Unit        string
 	}

 	// Used to build MQL queries
@@ -41,7 +39,6 @@ type (
 		IntervalMS  int64
 		AliasBy     string
 		timeRange   plugins.DataTimeRange
-		Unit        string
 	}

 	metricQuery struct {
@@ -56,7 +53,6 @@ type (
 		View               string
 		EditorMode         string
 		Query              string
-		Unit               string
 	}

 	sloQuery struct {
@@ -97,6 +93,7 @@ type (
 		TimeSeries           []timeSeries         `json:"timeSeries"`
 		TimeSeriesDescriptor timeSeriesDescriptor `json:"timeSeriesDescriptor"`
 		TimeSeriesData       timeSeriesData       `json:"timeSeriesData"`
+		Unit                 string               `json:"unit"`
 	}
 )