From 64d600f971c5d4ddf0fc66d23a2fe5ea39a475a5 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Wed, 13 Jan 2021 15:30:09 +0100 Subject: [PATCH] Cloudwatch: Move deep link creation to the backend (#30206) * get meta data from dataframe meta * gmdMeta is not used anymore * wip: move deep link creation to backend * Refactor backend. Remove not used front end code * Unit test deep links. Remove not used frontend tests * remove reference to gmdmeta * fix goimports error * fixed after pr feedback * more changes according to pr feedback * fix lint issue * fix bad math expression check and fix bad test * Decrease nesting * put link on first data field only --- packages/grafana-data/src/types/data.ts | 1 - pkg/tsdb/cloudwatch/query_transformer.go | 135 ++++++++++++++++-- pkg/tsdb/cloudwatch/query_transformer_test.go | 95 ++++++++++++ pkg/tsdb/cloudwatch/time_series_query.go | 12 +- pkg/tsdb/cloudwatch/types.go | 24 ++++ .../components/MetricsQueryEditor.tsx | 14 +- .../datasource/cloudwatch/datasource.test.ts | 6 +- .../datasource/cloudwatch/datasource.ts | 90 +----------- .../cloudwatch/specs/datasource.test.ts | 72 +--------- 9 files changed, 270 insertions(+), 179 deletions(-) diff --git a/packages/grafana-data/src/types/data.ts b/packages/grafana-data/src/types/data.ts index fbf0dbfb0c6..5c8702b1cc0 100644 --- a/packages/grafana-data/src/types/data.ts +++ b/packages/grafana-data/src/types/data.ts @@ -66,7 +66,6 @@ export interface QueryResultMeta { /** * Legacy data source specific, should be moved to custom * */ - gmdMeta?: any[]; // used by cloudwatch alignmentPeriod?: number; // used by cloud monitoring searchWords?: string[]; // used by log models and loki limit?: number; // used by log models and loki diff --git a/pkg/tsdb/cloudwatch/query_transformer.go b/pkg/tsdb/cloudwatch/query_transformer.go index 0ce7c1e8be5..2966899bda0 100644 --- a/pkg/tsdb/cloudwatch/query_transformer.go +++ b/pkg/tsdb/cloudwatch/query_transformer.go @@ -1,12 +1,14 @@ package cloudwatch import ( + "encoding/json" "fmt" + "net/url" "sort" "strings" + "time" "github.com/grafana/grafana-plugin-sdk-go/data" - "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/tsdb" ) @@ -53,7 +55,7 @@ func (e *cloudWatchExecutor) transformRequestQueriesToCloudWatchQueries(requestQ return cloudwatchQueries, nil } -func (e *cloudWatchExecutor) transformQueryResponsesToQueryResult(cloudwatchResponses []*cloudwatchResponse) map[string]*tsdb.QueryResult { +func (e *cloudWatchExecutor) transformQueryResponsesToQueryResult(cloudwatchResponses []*cloudwatchResponse, requestQueries []*requestQuery, startTime time.Time, endTime time.Time) (map[string]*tsdb.QueryResult, error) { responsesByRefID := make(map[string][]*cloudwatchResponse) refIDs := sort.StringSlice{} for _, res := range cloudwatchResponses { @@ -68,25 +70,18 @@ func (e *cloudWatchExecutor) transformQueryResponsesToQueryResult(cloudwatchResp responses := responsesByRefID[refID] queryResult := tsdb.NewQueryResult() queryResult.RefId = refID - queryResult.Meta = simplejson.New() queryResult.Series = tsdb.TimeSeriesSlice{} frames := make(data.Frames, 0, len(responses)) requestExceededMaxLimit := false partialData := false - queryMeta := []struct { - Expression, ID string - Period int - }{} + executedQueries := []executedQuery{} for _, response := range responses { frames = append(frames, response.DataFrames...) requestExceededMaxLimit = requestExceededMaxLimit || response.RequestExceededMaxLimit partialData = partialData || response.PartialData - queryMeta = append(queryMeta, struct { - Expression, ID string - Period int - }{ + executedQueries = append(executedQueries, executedQuery{ Expression: response.Expression, ID: response.Id, Period: response.Period, @@ -104,10 +99,124 @@ func (e *cloudWatchExecutor) transformQueryResponsesToQueryResult(cloudwatchResp queryResult.ErrorString = "Cloudwatch GetMetricData error: Too many datapoints requested - your search has been limited. Please try to reduce the time range" } + eq, err := json.Marshal(executedQueries) + if err != nil { + return nil, fmt.Errorf("could not marshal executedString struct: %w", err) + } + + link, err := buildDeepLink(refID, requestQueries, executedQueries, startTime, endTime) + if err != nil { + return nil, fmt.Errorf("could not build deep link: %w", err) + } + + createDataLinks := func(link string) []data.DataLink { + return []data.DataLink{{ + Title: "View in CloudWatch console", + TargetBlank: true, + URL: link, + }} + } + + for _, frame := range frames { + frame.Meta = &data.FrameMeta{ + ExecutedQueryString: string(eq), + } + + if link == "" || len(frame.Fields) < 2 { + continue + } + + frame.Fields[1].SetConfig(&data.FieldConfig{ + Links: createDataLinks(link), + }) + } + queryResult.Dataframes = tsdb.NewDecodedDataFrames(frames) - queryResult.Meta.Set("gmdMeta", queryMeta) results[refID] = queryResult } - return results + return results, nil +} + +// buildDeepLink generates a deep link from Grafana to the CloudWatch console. The link params are based on metric(s) for a given query row in the Query Editor. +func buildDeepLink(refID string, requestQueries []*requestQuery, executedQueries []executedQuery, startTime time.Time, endTime time.Time) (string, error) { + if isMathExpression(executedQueries) { + return "", nil + } + + requestQuery := &requestQuery{} + for _, rq := range requestQueries { + if rq.RefId == refID { + requestQuery = rq + break + } + } + + metricItems := []interface{}{} + cloudWatchLinkProps := &cloudWatchLink{ + Title: refID, + View: "timeSeries", + Stacked: false, + Region: requestQuery.Region, + Start: startTime.UTC().Format(time.RFC3339), + End: endTime.UTC().Format(time.RFC3339), + } + + expressions := []interface{}{} + for _, meta := range executedQueries { + if strings.Contains(meta.Expression, "SEARCH(") { + expressions = append(expressions, &metricExpression{Expression: meta.Expression}) + } + } + + if len(expressions) != 0 { + cloudWatchLinkProps.Metrics = expressions + } else { + for _, stat := range requestQuery.Statistics { + metricStat := []interface{}{requestQuery.Namespace, requestQuery.MetricName} + for dimensionKey, dimensionValues := range requestQuery.Dimensions { + metricStat = append(metricStat, dimensionKey, dimensionValues[0]) + } + metricStat = append(metricStat, &metricStatMeta{ + Stat: *stat, + Period: requestQuery.Period, + }) + metricItems = append(metricItems, metricStat) + } + cloudWatchLinkProps.Metrics = metricItems + } + + linkProps, err := json.Marshal(cloudWatchLinkProps) + if err != nil { + return "", fmt.Errorf("could not marshal link: %w", err) + } + + url, err := url.Parse(fmt.Sprintf(`https://%s.console.aws.amazon.com/cloudwatch/deeplink.js`, requestQuery.Region)) + if err != nil { + return "", fmt.Errorf("unable to parse CloudWatch console deep link") + } + + fragment := url.Query() + fragment.Set("", string(linkProps)) + + q := url.Query() + q.Set("region", requestQuery.Region) + url.RawQuery = q.Encode() + + link := fmt.Sprintf(`%s#metricsV2:graph%s`, url.String(), fragment.Encode()) + + return link, nil +} + +func isMathExpression(executedQueries []executedQuery) bool { + isMathExpression := false + for _, query := range executedQueries { + if strings.Contains(query.Expression, "SEARCH(") { + return false + } else if query.Expression != "" { + isMathExpression = true + } + } + + return isMathExpression } diff --git a/pkg/tsdb/cloudwatch/query_transformer_test.go b/pkg/tsdb/cloudwatch/query_transformer_test.go index 7fd9054a57f..90f64498042 100644 --- a/pkg/tsdb/cloudwatch/query_transformer_test.go +++ b/pkg/tsdb/cloudwatch/query_transformer_test.go @@ -1,7 +1,9 @@ package cloudwatch import ( + "net/url" "testing" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/stretchr/testify/assert" @@ -150,4 +152,97 @@ func TestQueryTransformer(t *testing.T) { require.Nil(t, res) assert.Error(t, err) }) + + requestQueries := []*requestQuery{ + { + RefId: "D", + Region: "us-east-1", + Namespace: "ec2", + MetricName: "CPUUtilization", + Statistics: aws.StringSlice([]string{"Sum"}), + Period: 600, + Id: "myId", + }, + { + RefId: "E", + Region: "us-east-1", + Namespace: "ec2", + MetricName: "CPUUtilization", + Statistics: aws.StringSlice([]string{"Average", "p46.32"}), + Period: 600, + Id: "myId", + }, + } + + t.Run("A deep link that reference two metric stat metrics is created based on a request query with two stats", func(t *testing.T) { + start, err := time.Parse(time.RFC3339, "2018-03-15T13:00:00Z") + require.NoError(t, err) + end, err := time.Parse(time.RFC3339, "2018-03-18T13:34:00Z") + require.NoError(t, err) + + executedQueries := []executedQuery{{ + Expression: ``, + ID: "D", + Period: 600, + }} + + link, err := buildDeepLink("E", requestQueries, executedQueries, start, end) + require.NoError(t, err) + + parsedURL, err := url.Parse(link) + require.NoError(t, err) + + decodedLink, err := url.PathUnescape(parsedURL.String()) + require.NoError(t, err) + expected := `https://us-east-1.console.aws.amazon.com/cloudwatch/deeplink.js?region=us-east-1#metricsV2:graph={"view":"timeSeries","stacked":false,"title":"E","start":"2018-03-15T13:00:00Z","end":"2018-03-18T13:34:00Z","region":"us-east-1","metrics":[["ec2","CPUUtilization",{"stat":"Average","period":600}],["ec2","CPUUtilization",{"stat":"p46.32","period":600}]]}` + assert.Equal(t, expected, decodedLink) + }) + + t.Run("A deep link that reference an expression based metric is created based on a request query with one stat", func(t *testing.T) { + start, err := time.Parse(time.RFC3339, "2018-03-15T13:00:00Z") + require.NoError(t, err) + end, err := time.Parse(time.RFC3339, "2018-03-18T13:34:00Z") + require.NoError(t, err) + + executedQueries := []executedQuery{{ + Expression: `REMOVE_EMPTY(SEARCH('Namespace="AWS/EC2" MetricName="CPUUtilization"', 'Sum', 600))`, + ID: "D", + Period: 600, + }} + + link, err := buildDeepLink("E", requestQueries, executedQueries, start, end) + require.NoError(t, err) + + parsedURL, err := url.Parse(link) + require.NoError(t, err) + + decodedLink, err := url.PathUnescape(parsedURL.String()) + require.NoError(t, err) + + expected := `https://us-east-1.console.aws.amazon.com/cloudwatch/deeplink.js?region=us-east-1#metricsV2:graph={"view":"timeSeries","stacked":false,"title":"E","start":"2018-03-15T13:00:00Z","end":"2018-03-18T13:34:00Z","region":"us-east-1","metrics":[{"expression":"REMOVE_EMPTY(SEARCH('Namespace=\"AWS/EC2\"+MetricName=\"CPUUtilization\"',+'Sum',+600))"}]}` + assert.Equal(t, expected, decodedLink) + }) + + t.Run("A deep link is not built in case any of the executedQueries are math expressions", func(t *testing.T) { + start, err := time.Parse(time.RFC3339, "2018-03-15T13:00:00Z") + require.NoError(t, err) + end, err := time.Parse(time.RFC3339, "2018-03-18T13:34:00Z") + require.NoError(t, err) + + executedQueries := []executedQuery{{ + Expression: `a * 2`, + ID: "D", + Period: 600, + }} + + link, err := buildDeepLink("E", requestQueries, executedQueries, start, end) + require.NoError(t, err) + + parsedURL, err := url.Parse(link) + require.NoError(t, err) + + decodedLink, err := url.PathUnescape(parsedURL.String()) + require.NoError(t, err) + assert.Equal(t, "", decodedLink) + }) } diff --git a/pkg/tsdb/cloudwatch/time_series_query.go b/pkg/tsdb/cloudwatch/time_series_query.go index 1385cc90df7..bb29df431a4 100644 --- a/pkg/tsdb/cloudwatch/time_series_query.go +++ b/pkg/tsdb/cloudwatch/time_series_query.go @@ -97,7 +97,17 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, queryCo } cloudwatchResponses = append(cloudwatchResponses, responses...) - res := e.transformQueryResponsesToQueryResult(cloudwatchResponses) + res, err := e.transformQueryResponsesToQueryResult(cloudwatchResponses, requestQueries, startTime, endTime) + if err != nil { + for _, query := range requestQueries { + resultChan <- &tsdb.QueryResult{ + RefId: query.RefId, + Error: err, + } + } + return nil + } + for _, queryRes := range res { resultChan <- queryRes } diff --git a/pkg/tsdb/cloudwatch/types.go b/pkg/tsdb/cloudwatch/types.go index fb360c52bb8..0aa6939bc6f 100644 --- a/pkg/tsdb/cloudwatch/types.go +++ b/pkg/tsdb/cloudwatch/types.go @@ -41,3 +41,27 @@ type queryError struct { func (e *queryError) Error() string { return fmt.Sprintf("error parsing query %q, %s", e.RefID, e.err) } + +type executedQuery struct { + Expression, ID string + Period int +} + +type cloudWatchLink struct { + View string `json:"view"` + Stacked bool `json:"stacked"` + Title string `json:"title"` + Start string `json:"start"` + End string `json:"end"` + Region string `json:"region"` + Metrics []interface{} `json:"metrics"` +} + +type metricExpression struct { + Expression string `json:"expression"` +} + +type metricStatMeta struct { + Stat string `json:"stat"` + Period int `json:"period"` +} diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor.tsx index 114e0603b6e..4675512d9a3 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor.tsx @@ -70,7 +70,10 @@ export class MetricsQueryEditor extends PureComponent { const metricsQuery = this.props.query as CloudWatchMetricsQuery; const { showMeta } = this.state; const query = normalizeQuery(metricsQuery); - const metaDataExist = data && Object.values(data).length && data.state === 'Done'; + const executedQueries = + data && data.series.length && data.series[0].meta && data.state === 'Done' + ? data.series[0].meta.executedQueryString + : null; return ( <> @@ -150,20 +153,21 @@ export class MetricsQueryEditor extends PureComponent {
- {showMeta && metaDataExist && ( + {showMeta && executedQueries && ( @@ -174,7 +178,7 @@ export class MetricsQueryEditor extends PureComponent { - {data?.series?.[0]?.meta?.gmdMeta?.map(({ ID, Expression, Period }: any) => ( + {JSON.parse(executedQueries).map(({ ID, Expression, Period }: any) => ( diff --git a/public/app/plugins/datasource/cloudwatch/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/datasource.test.ts index 8dc3eb41292..855c09541f2 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.test.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.test.ts @@ -46,13 +46,11 @@ describe('datasource', () => { const { datasource } = setup({ data: { results: { - a: { refId: 'a', series: [{ name: 'cpu', points: [1, 1] }], meta: { gmdMeta: '' } }, - b: { refId: 'b', series: [{ name: 'memory', points: [2, 2] }], meta: { gmdMeta: '' } }, + a: { refId: 'a', series: [{ name: 'cpu', points: [1, 1] }], meta: {} }, + b: { refId: 'b', series: [{ name: 'memory', points: [2, 2] }], meta: {} }, }, }, }); - const buildCloudwatchConsoleUrlMock = jest.spyOn(datasource, 'buildCloudwatchConsoleUrl'); - buildCloudwatchConsoleUrlMock.mockImplementation(() => ''); const observable = datasource.performTimeSeriesQuery( { diff --git a/public/app/plugins/datasource/cloudwatch/datasource.ts b/public/app/plugins/datasource/cloudwatch/datasource.ts index 2d3827338db..04f15e6c121 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.ts @@ -542,57 +542,6 @@ export class CloudWatchDatasource extends DataSourceApi - ) { - region = this.getActualRegion(region); - let conf = { - view: 'timeSeries', - stacked: false, - title, - start, - end, - region, - } as any; - - const isSearchExpression = - gmdMeta && gmdMeta.length && gmdMeta.every(({ Expression: expression }) => /SEARCH().*/.test(expression)); - const isMathExpression = !isSearchExpression && expression; - - if (isMathExpression) { - return ''; - } - - if (isSearchExpression) { - const metrics: any = - gmdMeta && gmdMeta.length ? gmdMeta.map(({ Expression: expression }) => ({ expression })) : [{ expression }]; - conf = { ...conf, metrics }; - } else { - conf = { - ...conf, - metrics: [ - ...statistics.map(stat => [ - namespace, - metricName, - ...Object.entries(dimensions).reduce((acc, [key, value]) => [...acc, key, value[0]], []), - { - stat, - period: gmdMeta.length ? gmdMeta[0].Period : 60, - }, - ]), - ], - }; - } - - return `https://${region}.console.aws.amazon.com/cloudwatch/deeplink.js?region=${region}#metricsV2:graph=${encodeURIComponent( - JSON.stringify(conf) - )}`; - } - performTimeSeriesQuery(request: MetricRequest, { from, to }: TimeRange): Observable { return this.awsRequest(TSDB_QUERY_ENDPOINT, request).pipe( map(res => { @@ -601,41 +550,12 @@ export class CloudWatchDatasource extends DataSourceApi { - const queryResult = res.results[frame.refId!]; - const error = queryResult.error ? { message: queryResult.error } : null; - if (!queryResult) { - return { frame, error }; - } - - const requestQuery = request.queries.find(q => q.refId === frame.refId!) as any; - - const link = this.buildCloudwatchConsoleUrl( - requestQuery!, - from.toISOString(), - to.toISOString(), - frame.refId!, - queryResult.meta.gmdMeta - ); - - if (link) { - for (const field of frame.fields) { - field.config.links = [ - { - url: link, - title: 'View in CloudWatch console', - targetBlank: true, - }, - ]; - } - } - return { frame, error }; - }); - return { - data: data.map(o => o.frame), - error: data - .map(o => o.error) + data: dataframes, + error: Object.values(res.results) + .map(o => ({ + message: o.error, + })) .reduce((err, error) => { return err || error; }, null), diff --git a/public/app/plugins/datasource/cloudwatch/specs/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/specs/datasource.test.ts index faa0eb0249c..4d8d7c21616 100644 --- a/public/app/plugins/datasource/cloudwatch/specs/datasource.test.ts +++ b/public/app/plugins/datasource/cloudwatch/specs/datasource.test.ts @@ -339,7 +339,7 @@ describe('CloudWatchDatasource', () => { type: 'Metrics', error: '', refId: 'A', - meta: { gmdMeta: [] }, + meta: {}, series: [ { name: 'CPUUtilization_Average', @@ -452,68 +452,6 @@ describe('CloudWatchDatasource', () => { }); }); - describe('a correct cloudwatch url should be built for each time series in the response', () => { - it('should be built correctly if theres one search expressions returned in meta for a given query row', async () => { - const { ds } = getTestContext({ response }); - - response.results['A'].meta.gmdMeta = [{ Expression: `REMOVE_EMPTY(SEARCH('some expression'))`, Period: '300' }]; - - await expect(ds.query(query)).toEmitValuesWith(received => { - const result = received[0]; - expect(getFrameDisplayName(result.data[0])).toBe(response.results.A.series[0].name); - expect(result.data[0].fields[1].config.links[0].title).toBe('View in CloudWatch console'); - expect(decodeURIComponent(result.data[0].fields[1].config.links[0].url)).toContain( - `region=us-east-1#metricsV2:graph={"view":"timeSeries","stacked":false,"title":"A","start":"2016-12-31T15:00:00.000Z","end":"2016-12-31T16:00:00.000Z","region":"us-east-1","metrics":[{"expression":"REMOVE_EMPTY(SEARCH(\'some expression\'))"}]}` - ); - }); - }); - - it('should be built correctly if theres two search expressions returned in meta for a given query row', async () => { - const { ds } = getTestContext({ response }); - - response.results['A'].meta.gmdMeta = [ - { Expression: `REMOVE_EMPTY(SEARCH('first expression'))` }, - { Expression: `REMOVE_EMPTY(SEARCH('second expression'))` }, - ]; - - await expect(ds.query(query)).toEmitValuesWith(received => { - const result = received[0]; - expect(getFrameDisplayName(result.data[0])).toBe(response.results.A.series[0].name); - expect(result.data[0].fields[1].config.links[0].title).toBe('View in CloudWatch console'); - expect(decodeURIComponent(result.data[0].fields[0].config.links[0].url)).toContain( - `region=us-east-1#metricsV2:graph={"view":"timeSeries","stacked":false,"title":"A","start":"2016-12-31T15:00:00.000Z","end":"2016-12-31T16:00:00.000Z","region":"us-east-1","metrics":[{"expression":"REMOVE_EMPTY(SEARCH(\'first expression\'))"},{"expression":"REMOVE_EMPTY(SEARCH(\'second expression\'))"}]}` - ); - }); - }); - - it('should be built correctly if the query is a metric stat query', async () => { - const { ds } = getTestContext({ response }); - - response.results['A'].meta.gmdMeta = [{ Period: '300' }]; - - await expect(ds.query(query)).toEmitValuesWith(received => { - const result = received[0]; - expect(getFrameDisplayName(result.data[0])).toBe(response.results.A.series[0].name); - expect(result.data[0].fields[1].config.links[0].title).toBe('View in CloudWatch console'); - expect(decodeURIComponent(result.data[0].fields[0].config.links[0].url)).toContain( - `region=us-east-1#metricsV2:graph={\"view\":\"timeSeries\",\"stacked\":false,\"title\":\"A\",\"start\":\"2016-12-31T15:00:00.000Z\",\"end\":\"2016-12-31T16:00:00.000Z\",\"region\":\"us-east-1\",\"metrics\":[[\"AWS/EC2\",\"CPUUtilization\",\"InstanceId\",\"i-12345678\",{\"stat\":\"Average\",\"period\":\"300\"}]]}` - ); - }); - }); - - it('should not be added at all if query is a math expression', async () => { - const { ds } = getTestContext({ response }); - - query.targets[0].expression = 'a * 2'; - response.results['A'].meta.searchExpressions = []; - - await expect(ds.query(query)).toEmitValuesWith(received => { - const result = received[0]; - expect(result.data[0].fields[1].config.links).toBeUndefined(); - }); - }); - }); - describe('and throttling exception is thrown', () => { const partialQuery = { type: 'Metrics', @@ -752,13 +690,7 @@ describe('CloudWatchDatasource', () => { A: { error: '', refId: 'A', - meta: { - gmdMeta: [ - { - Period: 300, - }, - ], - }, + meta: {}, series: [ { name: 'TargetResponseTime_p90.00',
{ID} {Expression}