From 94d0934e906f1cf7ff8c50fb42f695b0d343b8af Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Fri, 24 Jul 2020 12:34:56 +0200 Subject: [PATCH] CloudWatch: Fix a few API status codes (#26578) Signed-off-by: Arve Knudsen --- pkg/api/metrics.go | 17 ++++++++++++----- pkg/services/sqlstore/sqlstore.go | 2 +- pkg/tsdb/cloudwatch/metric_find_query.go | 24 +++++++++++++++++------- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/pkg/api/metrics.go b/pkg/api/metrics.go index 8f50bfadd76..1f9a976e403 100644 --- a/pkg/api/metrics.go +++ b/pkg/api/metrics.go @@ -2,6 +2,7 @@ package api import ( "context" + "errors" "sort" "github.com/grafana/grafana/pkg/models" @@ -20,7 +21,7 @@ import ( // POST /api/ds/query DataSource query w/ expressions func (hs *HTTPServer) QueryMetricsV2(c *models.ReqContext, reqDto dtos.MetricRequest) Response { if len(reqDto.Queries) == 0 { - return Error(500, "No queries found in query", nil) + return Error(400, "No queries found in query", nil) } request := &tsdb.TsdbQuery{ @@ -32,6 +33,7 @@ func (hs *HTTPServer) QueryMetricsV2(c *models.ReqContext, reqDto dtos.MetricReq expr := false var ds *models.DataSource for i, query := range reqDto.Queries { + hs.log.Debug("Processing metrics query", "query", query) name := query.Get("datasource").MustString("") if name == "__expr__" { expr = true @@ -39,16 +41,21 @@ func (hs *HTTPServer) QueryMetricsV2(c *models.ReqContext, reqDto dtos.MetricReq datasourceID, err := query.Get("datasourceId").Int64() if err != nil { - return Error(500, "datasource missing ID", nil) + hs.log.Debug("Can't process query since it's missing data source ID") + return Error(400, "Query missing data source ID", nil) } if i == 0 && !expr { ds, err = hs.DatasourceCache.GetDatasource(datasourceID, c.SignedInUser, c.SkipCache) if err != nil { - if err == models.ErrDataSourceAccessDenied { - return Error(403, "Access denied to datasource", err) + hs.log.Debug("Encountered error getting data source", "err", err) + if errors.Is(err, models.ErrDataSourceAccessDenied) { + return Error(403, "Access denied to data source", err) } - return Error(500, "Unable to load datasource meta data", err) + if errors.Is(err, models.ErrDataSourceNotFound) { + return Error(400, "Invalid data source ID", err) + } + return Error(500, "Unable to load data source metadata", err) } } diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index db7f9daf07a..c1f16eefb45 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -80,7 +80,7 @@ func (ss *SqlStore) Init() error { x = engine dialect = ss.Dialect - migrator := migrator.NewMigrator(x) + migrator := migrator.NewMigrator(engine) migrations.AddMigrations(migrator) for _, descriptor := range registry.GetServices() { diff --git a/pkg/tsdb/cloudwatch/metric_find_query.go b/pkg/tsdb/cloudwatch/metric_find_query.go index 0b487c52800..e4fd78b85e3 100644 --- a/pkg/tsdb/cloudwatch/metric_find_query.go +++ b/pkg/tsdb/cloudwatch/metric_find_query.go @@ -309,7 +309,8 @@ func parseMultiSelectValue(input string) []string { // Whenever this list is updated, the frontend list should also be updated. // Please update the region list in public/app/plugins/datasource/cloudwatch/partials/config.html -func (e *cloudWatchExecutor) handleGetRegions(ctx context.Context, parameters *simplejson.Json, queryContext *tsdb.TsdbQuery) ([]suggestData, error) { +func (e *cloudWatchExecutor) handleGetRegions(ctx context.Context, parameters *simplejson.Json, + queryContext *tsdb.TsdbQuery) ([]suggestData, error) { dsInfo := e.getDSInfo(defaultRegion) profile := dsInfo.Profile if cache, ok := regionCache.Load(profile); ok { @@ -479,7 +480,8 @@ func (e *cloudWatchExecutor) handleGetDimensionValues(ctx context.Context, param return result, nil } -func (e *cloudWatchExecutor) handleGetEbsVolumeIds(ctx context.Context, parameters *simplejson.Json, queryContext *tsdb.TsdbQuery) ([]suggestData, error) { +func (e *cloudWatchExecutor) handleGetEbsVolumeIds(ctx context.Context, parameters *simplejson.Json, + queryContext *tsdb.TsdbQuery) ([]suggestData, error) { region := parameters.Get("region").MustString() instanceId := parameters.Get("instanceId").MustString() @@ -501,7 +503,8 @@ func (e *cloudWatchExecutor) handleGetEbsVolumeIds(ctx context.Context, paramete return result, nil } -func (e *cloudWatchExecutor) handleGetEc2InstanceAttribute(ctx context.Context, parameters *simplejson.Json, queryContext *tsdb.TsdbQuery) ([]suggestData, error) { +func (e *cloudWatchExecutor) handleGetEc2InstanceAttribute(ctx context.Context, parameters *simplejson.Json, + queryContext *tsdb.TsdbQuery) ([]suggestData, error) { region := parameters.Get("region").MustString() attributeName := parameters.Get("attributeName").MustString() filterJson := parameters.Get("filters").MustMap() @@ -580,7 +583,8 @@ func (e *cloudWatchExecutor) handleGetEc2InstanceAttribute(ctx context.Context, return result, nil } -func (e *cloudWatchExecutor) handleGetResourceArns(ctx context.Context, parameters *simplejson.Json, queryContext *tsdb.TsdbQuery) ([]suggestData, error) { +func (e *cloudWatchExecutor) handleGetResourceArns(ctx context.Context, parameters *simplejson.Json, + queryContext *tsdb.TsdbQuery) ([]suggestData, error) { region := parameters.Get("region").MustString() resourceType := parameters.Get("resourceType").MustString() filterJson := parameters.Get("tags").MustMap() @@ -618,7 +622,8 @@ func (e *cloudWatchExecutor) handleGetResourceArns(ctx context.Context, paramete return result, nil } -func (e *cloudWatchExecutor) cloudwatchListMetrics(region string, namespace string, metricName string, dimensions []*cloudwatch.DimensionFilter) (*cloudwatch.ListMetricsOutput, error) { +func (e *cloudWatchExecutor) cloudwatchListMetrics(region string, namespace string, metricName string, + dimensions []*cloudwatch.DimensionFilter) (*cloudwatch.ListMetricsOutput, error) { svc, err := e.getCWClient(region) if err != nil { return nil, err @@ -706,6 +711,7 @@ func (e *cloudWatchExecutor) getAllMetrics(region string) (cloudwatch.ListMetric Namespace: aws.String(dsInfo.Namespace), } + plog.Debug("Listing metrics pages") var resp cloudwatch.ListMetricsOutput err = client.ListMetricsPages(params, func(page *cloudwatch.ListMetricsOutput, lastPage bool) bool { metrics.MAwsCloudWatchListMetrics.Inc() @@ -726,6 +732,7 @@ func (e *cloudWatchExecutor) getAllMetrics(region string) (cloudwatch.ListMetric var metricsCacheLock sync.Mutex func (e *cloudWatchExecutor) getMetricsForCustomMetrics(region, namespace string) ([]string, error) { + plog.Debug("Getting metrics for custom metrics", "region", region, "namespace", namespace) metricsCacheLock.Lock() defer metricsCacheLock.Unlock() @@ -750,6 +757,7 @@ func (e *cloudWatchExecutor) getMetricsForCustomMetrics(region, namespace string if err != nil { return []string{}, err } + customMetricsMetricsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache = make([]string, 0) customMetricsMetricsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Expire = time.Now().Add(5 * time.Minute) @@ -757,7 +765,8 @@ func (e *cloudWatchExecutor) getMetricsForCustomMetrics(region, namespace string if isDuplicate(customMetricsMetricsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache, *metric.MetricName) { continue } - customMetricsMetricsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache = append(customMetricsMetricsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache, *metric.MetricName) + customMetricsMetricsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache = append( + customMetricsMetricsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache, *metric.MetricName) } return customMetricsMetricsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache, nil @@ -797,7 +806,8 @@ func (e *cloudWatchExecutor) getDimensionsForCustomMetrics(region string) ([]str if isDuplicate(customMetricsDimensionsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache, *dimension.Name) { continue } - customMetricsDimensionsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache = append(customMetricsDimensionsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache, *dimension.Name) + customMetricsDimensionsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache = append( + customMetricsDimensionsMap[dsInfo.Profile][dsInfo.Region][dsInfo.Namespace].Cache, *dimension.Name) } }