From 04f4454974c3f517dcbe39917ae19025937c9f91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 2 Oct 2015 09:04:46 +0200 Subject: [PATCH] feat(cloudwatch): lots of code refactoring and cleanup, #684, dimension values lookup works but seems to return all dimension values no matter what dimension key you select, removed strange formating of template dimension values query, should not return key value pair but only the dimension value --- pkg/api/dataproxy_cloudwatch.go | 24 +-- .../app/features/dashboard/dashboardCtrl.js | 2 +- public/app/features/templating/editorCtrl.js | 8 +- .../datasource/cloudwatch/datasource.js | 139 ++++++------------ .../datasource/cloudwatch/query_ctrl.js | 8 +- .../cloudwatch/specs/datasource_specs.ts | 37 ++--- 6 files changed, 82 insertions(+), 136 deletions(-) diff --git a/pkg/api/dataproxy_cloudwatch.go b/pkg/api/dataproxy_cloudwatch.go index 3dc43c4e780..2e833c1caf4 100644 --- a/pkg/api/dataproxy_cloudwatch.go +++ b/pkg/api/dataproxy_cloudwatch.go @@ -31,13 +31,13 @@ func ProxyCloudWatchDataSourceRequest(c *middleware.Context) { case "GetMetricStatistics": reqParam := &struct { Parameters struct { - Namespace string `json:"Namespace"` - MetricName string `json:"MetricName"` - Dimensions []*cloudwatch.Dimension `json:"Dimensions"` - Statistics []*string `json:"Statistics"` - StartTime int64 `json:"StartTime"` - EndTime int64 `json:"EndTime"` - Period int64 `json:"Period"` + Namespace string `json:"namespace"` + MetricName string `json:"metricName"` + Dimensions []*cloudwatch.Dimension `json:"dimensions"` + Statistics []*string `json:"statistics"` + StartTime int64 `json:"startTime"` + EndTime int64 `json:"endTime"` + Period int64 `json:"period"` } `json:"parameters"` }{} json.Unmarshal([]byte(body), reqParam) @@ -63,9 +63,9 @@ func ProxyCloudWatchDataSourceRequest(c *middleware.Context) { case "ListMetrics": reqParam := &struct { Parameters struct { - Namespace string `json:"Namespace"` - MetricName string `json:"MetricName"` - Dimensions []*cloudwatch.DimensionFilter `json:"Dimensions"` + Namespace string `json:"namespace"` + MetricName string `json:"metricName"` + Dimensions []*cloudwatch.DimensionFilter `json:"dimensions"` } `json:"parameters"` }{} json.Unmarshal([]byte(body), reqParam) @@ -94,8 +94,8 @@ func ProxyCloudWatchDataSourceRequest(c *middleware.Context) { case "DescribeInstances": reqParam := &struct { Parameters struct { - Filters []*ec2.Filter `json:"Filters"` - InstanceIds []*string `json:"InstanceIds"` + Filters []*ec2.Filter `json:"filters"` + InstanceIds []*string `json:"instanceIds"` } `json:"parameters"` }{} json.Unmarshal([]byte(body), reqParam) diff --git a/public/app/features/dashboard/dashboardCtrl.js b/public/app/features/dashboard/dashboardCtrl.js index f25fea26afe..30dda154e94 100644 --- a/public/app/features/dashboard/dashboardCtrl.js +++ b/public/app/features/dashboard/dashboardCtrl.js @@ -64,7 +64,7 @@ function (angular, $, config) { $scope.appEvent("dashboard-loaded", $scope.dashboard); }).catch(function(err) { - console.log('Failed to initialize dashboard', err); + if (err.data && err.data.message) { err.message = err.data.message; } $scope.appEvent("alert-error", ['Dashboard init failed', 'Template variables could not be initialized: ' + err.message]); }); }; diff --git a/public/app/features/templating/editorCtrl.js b/public/app/features/templating/editorCtrl.js index 74157ac3dd8..2f30129eedb 100644 --- a/public/app/features/templating/editorCtrl.js +++ b/public/app/features/templating/editorCtrl.js @@ -7,7 +7,7 @@ function (angular, _) { var module = angular.module('grafana.controllers'); - module.controller('TemplateEditorCtrl', function($scope, datasourceSrv, templateSrv, templateValuesSrv, alertSrv) { + module.controller('TemplateEditorCtrl', function($scope, datasourceSrv, templateSrv, templateValuesSrv) { var replacementDefaults = { type: 'query', @@ -78,9 +78,9 @@ function (angular, _) { }; $scope.runQuery = function() { - return templateValuesSrv.updateOptions($scope.current).then(function() { - }, function(err) { - alertSrv.set('Templating', 'Failed to run query for variable values: ' + err.message, 'error'); + return templateValuesSrv.updateOptions($scope.current).then(null, function(err) { + if (err.data && err.data.message) { err.message = err.data.message; } + $scope.appEvent("alert-error", ['Templating', 'Template variables could not be initialized: ' + err.message]); }); }; diff --git a/public/app/plugins/datasource/cloudwatch/datasource.js b/public/app/plugins/datasource/cloudwatch/datasource.js index 03b81ddc428..55657ee2071 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.js +++ b/public/app/plugins/datasource/cloudwatch/datasource.js @@ -10,7 +10,7 @@ function (angular, _) { var module = angular.module('grafana.services'); - module.factory('CloudWatchDatasource', function($q, $http, templateSrv) { + module.factory('CloudWatchDatasource', function($q, backendSrv, templateSrv) { function CloudWatchDatasource(datasource) { this.type = 'cloudwatch'; @@ -173,7 +173,7 @@ function (angular, _) { return _.isEmpty(u); }) .map(function(u) { - return $http({ method: 'GET', url: u }); + return backendSrv.datasourceRequest({ method: 'GET', url: u }); }) ) .then(function(allResponse) { @@ -218,7 +218,7 @@ function (angular, _) { var queries = []; _.each(options.targets, _.bind(function(target) { - if (!target.namespace || !target.metricName || _.isEmpty(target.statistics)) { + if (target.hide || !target.namespace || !target.metricName || _.isEmpty(target.statistics)) { return; } @@ -263,19 +263,20 @@ function (angular, _) { }; CloudWatchDatasource.prototype.performTimeSeriesQuery = function(query, start, end) { - var cloudwatch = this.getAwsClient(query.region, 'CloudWatch'); - - var params = { - Namespace: query.namespace, - MetricName: query.metricName, - Dimensions: query.dimensions, - Statistics: query.statistics, - StartTime: start, - EndTime: end, - Period: query.period - }; - - return cloudwatch.getMetricStatistics(params); + return this.awsRequest({ + region: query.region, + service: 'CloudWatch', + action: 'GetMetricStatistics', + parameters: { + namespace: query.namespace, + metricName: query.metricName, + dimensions: query.dimensions, + statistics: query.statistics, + startTime: start, + endTime: end, + period: query.period + } + }); }; CloudWatchDatasource.prototype.getRegions = function() { @@ -297,38 +298,36 @@ function (angular, _) { }; CloudWatchDatasource.prototype.getDimensionValues = function(region, namespace, metricName, dimensions) { - region = templateSrv.replace(region); - namespace = templateSrv.replace(namespace); - metricName = templateSrv.replace(metricName); + var request = { + region: templateSrv.replace(region), + service: 'CloudWatch', + action: 'ListMetrics', + parameters: { + namespace: templateSrv.replace(namespace), + metricName: templateSrv.replace(metricName), + dimensions: convertDimensionFormat(dimensions), + } + }; - var cloudwatch = this.getAwsClient(region, 'CloudWatch'); - var params = {Namespace: namespace, MetricName: metricName}; - - if (!_.isEmpty(dimensions)) { - params.Dimensions = convertDimensionFormat(dimensions); - } - - return cloudwatch.listMetrics(params).then(function(result) { + return this.awsRequest(request).then(function(result) { return _.chain(result.Metrics).map(function(metric) { - return metric.Dimensions; - }).reject(function(metric) { - return _.isEmpty(metric); + return _.pluck(metric.Dimensions, 'Value'); + }).flatten().uniq().sortBy(function(name) { + return name; }).value(); }); }; CloudWatchDatasource.prototype.performEC2DescribeInstances = function(region, filters, instanceIds) { - var ec2 = this.getAwsClient(region, 'EC2'); - - var params = {}; - if (filters.length > 0) { - params.Filters = filters; - } - if (instanceIds.length > 0) { - params.InstanceIds = instanceIds; - } - - return ec2.describeInstances(params); + return this.awsRequest({ + region: region, + service: 'EC2', + action: 'DescribeInstances', + parameters: { + filter: filters, + instanceIds: instanceIds + } + }); }; CloudWatchDatasource.prototype.metricFindQuery = function(query) { @@ -382,21 +381,7 @@ function (angular, _) { }); } - return this.getDimensionValues(region, namespace, metricName, dimensions) - .then(function(suggestData) { - return _.map(suggestData, function(dimensions) { - var result = _.chain(dimensions) - .sortBy(function(dimension) { - return dimension.Name; - }) - .map(function(dimension) { - return dimension.Name + '=' + dimension.Value; - }) - .value().join(','); - - return { text: result }; - }); - }); + return this.getDimensionValues(region, namespace, metricName, dimensions).then(transformSuggestData); } var ebsVolumeIdsQuery = query.match(/^ebs_volume_ids\(([^,]+?),\s?([^,]+?)\)/); @@ -431,42 +416,16 @@ function (angular, _) { }); }; - CloudWatchDatasource.prototype.getAwsClient = function(region, service) { - var self = this; - var generateRequestProxy = function(service, action) { - return function(params) { - var data = { - region: region, - service: service, - action: action, - parameters: params - }; - - var options = { - method: 'POST', - url: self.proxyUrl, - data: data - }; - - return $http(options).then(function(result) { - return result.data; - }); - }; + CloudWatchDatasource.prototype.awsRequest = function(data) { + var options = { + method: 'POST', + url: this.proxyUrl, + data: data }; - switch (service) { - case 'CloudWatch': { - return { - getMetricStatistics: generateRequestProxy('CloudWatch', 'GetMetricStatistics'), - listMetrics: generateRequestProxy('CloudWatch', 'ListMetrics') - }; - } - case 'EC2': { - return { - describeInstances: generateRequestProxy('EC2', 'DescribeInstances') - }; - } - } + return backendSrv.datasourceRequest(options).then(function(result) { + return result.data; + }); }; CloudWatchDatasource.prototype.getDefaultRegion = function() { diff --git a/public/app/plugins/datasource/cloudwatch/query_ctrl.js b/public/app/plugins/datasource/cloudwatch/query_ctrl.js index cc34eaa2dd6..7c8b44b7f2a 100644 --- a/public/app/plugins/datasource/cloudwatch/query_ctrl.js +++ b/public/app/plugins/datasource/cloudwatch/query_ctrl.js @@ -99,13 +99,7 @@ function (angular, _) { $scope.target.metricName, $scope.target.dimensions ).then(function(result) { - var suggestData = _.chain(result) - .flatten(true) - .pluck('Value') - .uniq() - .value(); - - callback(suggestData); + callback(result); }, function() { callback([]); }); diff --git a/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts b/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts index 9672c7147c7..807b3b010b6 100644 --- a/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts +++ b/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts @@ -10,7 +10,7 @@ describe('CloudWatchDatasource', function() { beforeEach(angularMocks.module('grafana.services')); beforeEach(angularMocks.module('grafana.controllers')); - beforeEach(ctx.providePhase(['templateSrv'])); + beforeEach(ctx.providePhase(['templateSrv', 'backendSrv'])); beforeEach(ctx.createService('CloudWatchDatasource')); beforeEach(function() { ctx.ds = new ctx.service({ @@ -53,24 +53,21 @@ describe('CloudWatchDatasource', function() { }; beforeEach(function() { - ctx.ds.getAwsClient = function() { - return { - getMetricStatistics: function(params) { - requestParams = params; - return ctx.$q.when(response); - } - }; + ctx.backendSrv.datasourceRequest = function(params) { + requestParams = params; + return ctx.$q.when({data: response}); }; }); it('should generate the correct query', function(done) { ctx.ds.query(query).then(function() { - expect(requestParams.Namespace).to.be(query.targets[0].namespace); - expect(requestParams.MetricName).to.be(query.targets[0].metricName); - expect(requestParams.Dimensions[0].Name).to.be(Object.keys(query.targets[0].dimensions)[0]); - expect(requestParams.Dimensions[0].Value).to.be(query.targets[0].dimensions[Object.keys(query.targets[0].dimensions)[0]]); - expect(requestParams.Statistics).to.eql(Object.keys(query.targets[0].statistics)); - expect(requestParams.Period).to.be(query.targets[0].period); + var params = requestParams.data.parameters; + expect(params.namespace).to.be(query.targets[0].namespace); + expect(params.metricName).to.be(query.targets[0].metricName); + expect(params.dimensions[0].Name).to.be(Object.keys(query.targets[0].dimensions)[0]); + expect(params.dimensions[0].Value).to.be(query.targets[0].dimensions[Object.keys(query.targets[0].dimensions)[0]]); + expect(params.statistics).to.eql(Object.keys(query.targets[0].statistics)); + expect(params.period).to.be(query.targets[0].period); done(); }); ctx.$rootScope.$apply(); @@ -105,13 +102,9 @@ describe('CloudWatchDatasource', function() { }; beforeEach(function() { - ctx.ds.getAwsClient = function() { - return { - listMetrics: function(params) { - requestParams = params; - return ctx.$q.when(response); - } - }; + ctx.backendSrv.datasourceRequest = function(params) { + requestParams = params; + return ctx.$q.when({data: response}); }; }); @@ -158,7 +151,7 @@ describe('CloudWatchDatasource', function() { var query = 'dimension_values(us-east-1,AWS/EC2,CPUUtilization)'; ctx.ds.metricFindQuery(query).then(function(result) { result = result.map(function(v) { return v.text; }); - expect(result).to.eql(['InstanceId=i-12345678']); + expect(result).to.eql(['i-12345678']); done(); }); ctx.$rootScope.$apply();