Skip to content

Commit c43e1a7

Browse files
fridgepoetsunker
fridgepoet
and
sunker
authored
CloudWatch: fix custom namespace for listing dimension keys, refactor to non-pointer types, add test assertions, rename packages (grafana#59106)
Co-authored-by: Erik Sundell <erik.sundell87@gmail.com>
1 parent f62e55f commit c43e1a7

32 files changed

+194
-160
lines changed

‎pkg/tsdb/cloudwatch/annotation_query_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func TestQuery_AnnotationQuery(t *testing.T) {
3232
t.Run("DescribeAlarmsForMetric is called with minimum parameters", func(t *testing.T) {
3333
client = fakeCWAnnotationsClient{describeAlarmsForMetricOutput: &cloudwatch.DescribeAlarmsForMetricOutput{}}
3434
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
35-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
35+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
3636
})
3737

3838
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -66,7 +66,7 @@ func TestQuery_AnnotationQuery(t *testing.T) {
6666
t.Run("DescribeAlarms is called when prefixMatching is true", func(t *testing.T) {
6767
client = fakeCWAnnotationsClient{describeAlarmsOutput: &cloudwatch.DescribeAlarmsOutput{}}
6868
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
69-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
69+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
7070
})
7171

7272
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())

‎pkg/tsdb/cloudwatch/cloudwatch.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ type DataQueryJson struct {
5050
}
5151

5252
type DataSource struct {
53-
Settings *models.CloudWatchSettings
53+
Settings models.CloudWatchSettings
5454
HTTPClient *http.Client
5555
}
5656

‎pkg/tsdb/cloudwatch/cloudwatch_test.go

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/grafana/grafana/pkg/setting"
2424
"github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks"
2525
"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models"
26+
"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources"
2627
"github.com/stretchr/testify/assert"
2728
"github.com/stretchr/testify/require"
2829
)
@@ -52,7 +53,7 @@ func TestNewInstanceSettings(t *testing.T) {
5253
},
5354
},
5455
expectedDS: DataSource{
55-
Settings: &models.CloudWatchSettings{
56+
Settings: models.CloudWatchSettings{
5657
AWSDatasourceSettings: awsds.AWSDatasourceSettings{
5758
Profile: "foo",
5859
Region: "us-east2",
@@ -112,7 +113,7 @@ func Test_CheckHealth(t *testing.T) {
112113
t.Run("successfully query metrics and logs", func(t *testing.T) {
113114
client = fakeCheckHealthClient{}
114115
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
115-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
116+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
116117
})
117118
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
118119

@@ -133,7 +134,7 @@ func Test_CheckHealth(t *testing.T) {
133134
return nil, fmt.Errorf("some logs query error")
134135
}}
135136
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
136-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
137+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
137138
})
138139
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
139140

@@ -154,7 +155,7 @@ func Test_CheckHealth(t *testing.T) {
154155
return fmt.Errorf("some list metrics error")
155156
}}
156157
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
157-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
158+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
158159
})
159160
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
160161

@@ -172,7 +173,7 @@ func Test_CheckHealth(t *testing.T) {
172173
t.Run("fail to get clients", func(t *testing.T) {
173174
client = fakeCheckHealthClient{}
174175
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
175-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
176+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
176177
})
177178
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{getSession: func(c awsds.SessionConfig) (*session.Session, error) {
178179
return nil, fmt.Errorf("some sessions error")
@@ -203,7 +204,7 @@ func Test_executeLogAlertQuery(t *testing.T) {
203204
t.Run("getCWLogsClient is called with region from input JSON", func(t *testing.T) {
204205
cli = fakeCWLogsClient{queryResults: cloudwatchlogs.GetQueryResultsOutput{Status: aws.String("Complete")}}
205206
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
206-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
207+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
207208
})
208209
sess := fakeSessionCache{}
209210
executor := newExecutor(im, newTestConfig(), &sess, featuremgmt.WithFeatures())
@@ -229,7 +230,7 @@ func Test_executeLogAlertQuery(t *testing.T) {
229230
t.Run("getCWLogsClient is called with region from instance manager when region is default", func(t *testing.T) {
230231
cli = fakeCWLogsClient{queryResults: cloudwatchlogs.GetQueryResultsOutput{Status: aws.String("Complete")}}
231232
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
232-
return DataSource{Settings: &models.CloudWatchSettings{AWSDatasourceSettings: awsds.AWSDatasourceSettings{Region: "instance manager's region"}}}, nil
233+
return DataSource{Settings: models.CloudWatchSettings{AWSDatasourceSettings: awsds.AWSDatasourceSettings{Region: "instance manager's region"}}}, nil
233234
})
234235
sess := fakeSessionCache{}
235236

@@ -266,7 +267,7 @@ func TestQuery_ResourceRequest_DescribeAllLogGroups(t *testing.T) {
266267
}
267268

268269
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
269-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
270+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
270271
})
271272

272273
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -412,7 +413,7 @@ func TestQuery_ResourceRequest_DescribeLogGroups(t *testing.T) {
412413
}
413414

414415
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
415-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
416+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
416417
})
417418

418419
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -467,7 +468,7 @@ func TestQuery_ResourceRequest_DescribeLogGroups(t *testing.T) {
467468
}
468469

469470
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
470-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
471+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
471472
})
472473

473474
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -505,7 +506,7 @@ func TestQuery_ResourceRequest_DescribeLogGroups(t *testing.T) {
505506
}
506507

507508
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
508-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
509+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
509510
})
510511

511512
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -546,7 +547,7 @@ func Test_CloudWatch_CallResource_Integration_Test(t *testing.T) {
546547
return &api
547548
}
548549
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
549-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
550+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
550551
})
551552

552553
t.Run("Should handle dimension value request and return values from the api", func(t *testing.T) {
@@ -699,10 +700,10 @@ func Test_CloudWatch_CallResource_Integration_Test(t *testing.T) {
699700
sent := sender.Response
700701
require.NotNil(t, sent)
701702
require.Equal(t, http.StatusOK, sent.Status)
702-
res := []models.Metric{}
703+
res := []resources.Metric{}
703704
err = json.Unmarshal(sent.Body, &res)
704705
require.Nil(t, err)
705-
assert.Equal(t, []models.Metric{{Name: "Test_MetricName1", Namespace: "AWS/EC2"}, {Name: "Test_MetricName2", Namespace: "AWS/EC2"}, {Name: "Test_MetricName3", Namespace: "AWS/ECS"}, {Name: "Test_MetricName10", Namespace: "AWS/ECS"}, {Name: "Test_MetricName4", Namespace: "AWS/ECS"}, {Name: "Test_MetricName5", Namespace: "AWS/Redshift"}}, res)
706+
assert.Equal(t, []resources.Metric{{Name: "Test_MetricName1", Namespace: "AWS/EC2"}, {Name: "Test_MetricName2", Namespace: "AWS/EC2"}, {Name: "Test_MetricName3", Namespace: "AWS/ECS"}, {Name: "Test_MetricName10", Namespace: "AWS/ECS"}, {Name: "Test_MetricName4", Namespace: "AWS/ECS"}, {Name: "Test_MetricName5", Namespace: "AWS/Redshift"}}, res)
706707
})
707708
}
708709

‎pkg/tsdb/cloudwatch/log_actions_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func TestQuery_GetLogEvents(t *testing.T) {
8383
cli = fakeCWLogsClient{}
8484

8585
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
86-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
86+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
8787
})
8888

8989
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -141,7 +141,7 @@ func TestQuery_GetLogGroupFields(t *testing.T) {
141141
const refID = "A"
142142

143143
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
144-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
144+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
145145
})
146146

147147
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -222,7 +222,7 @@ func TestQuery_StartQuery(t *testing.T) {
222222
}
223223

224224
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
225-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
225+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
226226
})
227227

228228
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -275,7 +275,7 @@ func TestQuery_StartQuery(t *testing.T) {
275275
}
276276

277277
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
278-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
278+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
279279
})
280280

281281
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -333,7 +333,7 @@ func Test_executeStartQuery(t *testing.T) {
333333
t.Run("successfully parses information from JSON to StartQueryWithContext", func(t *testing.T) {
334334
cli = fakeCWLogsClient{}
335335
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
336-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
336+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
337337
})
338338
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
339339

@@ -369,7 +369,7 @@ func Test_executeStartQuery(t *testing.T) {
369369
t.Run("does not populate StartQueryInput.limit when no limit provided", func(t *testing.T) {
370370
cli = fakeCWLogsClient{}
371371
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
372-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
372+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
373373
})
374374
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
375375

@@ -425,7 +425,7 @@ func TestQuery_StopQuery(t *testing.T) {
425425
}
426426

427427
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
428-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
428+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
429429
})
430430

431431
timeRange := backend.TimeRange{
@@ -520,7 +520,7 @@ func TestQuery_GetQueryResults(t *testing.T) {
520520
}
521521

522522
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
523-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
523+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
524524
})
525525

526526
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())

‎pkg/tsdb/cloudwatch/metric_find_query_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func TestQuery_Regions(t *testing.T) {
4141
}
4242

4343
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
44-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
44+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
4545
})
4646

4747
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -108,7 +108,7 @@ func TestQuery_InstanceAttributes(t *testing.T) {
108108
}
109109

110110
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
111-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
111+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
112112
})
113113

114114
filterMap := map[string][]string{
@@ -191,7 +191,7 @@ func TestQuery_EBSVolumeIDs(t *testing.T) {
191191
}
192192

193193
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
194-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
194+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
195195
})
196196

197197
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
@@ -251,7 +251,7 @@ func TestQuery_ResourceARNs(t *testing.T) {
251251
}
252252

253253
im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
254-
return DataSource{Settings: &models.CloudWatchSettings{}}, nil
254+
return DataSource{Settings: models.CloudWatchSettings{}}, nil
255255
})
256256

257257
tagMap := map[string][]string{

‎pkg/tsdb/cloudwatch/mocks/list_metrics_service.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,34 @@
11
package mocks
22

33
import (
4-
"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models"
5-
"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/request"
4+
"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources"
65
"github.com/stretchr/testify/mock"
76
)
87

98
type ListMetricsServiceMock struct {
109
mock.Mock
1110
}
1211

13-
func (a *ListMetricsServiceMock) GetDimensionKeysByDimensionFilter(*request.DimensionKeysRequest) ([]string, error) {
14-
args := a.Called()
12+
func (a *ListMetricsServiceMock) GetDimensionKeysByDimensionFilter(r resources.DimensionKeysRequest) ([]string, error) {
13+
args := a.Called(r)
1514

1615
return args.Get(0).([]string), args.Error(1)
1716
}
1817

19-
func (a *ListMetricsServiceMock) GetDimensionValuesByDimensionFilter(r *request.DimensionValuesRequest) ([]string, error) {
20-
args := a.Called()
18+
func (a *ListMetricsServiceMock) GetDimensionValuesByDimensionFilter(r resources.DimensionValuesRequest) ([]string, error) {
19+
args := a.Called(r)
2120

2221
return args.Get(0).([]string), args.Error(1)
2322
}
2423

25-
func (a *ListMetricsServiceMock) GetDimensionKeysByNamespace(string) ([]string, error) {
26-
args := a.Called()
24+
func (a *ListMetricsServiceMock) GetDimensionKeysByNamespace(namespace string) ([]string, error) {
25+
args := a.Called(namespace)
2726

2827
return args.Get(0).([]string), args.Error(1)
2928
}
3029

31-
func (a *ListMetricsServiceMock) GetMetricsByNamespace(namespace string) ([]models.Metric, error) {
32-
args := a.Called()
30+
func (a *ListMetricsServiceMock) GetMetricsByNamespace(namespace string) ([]resources.Metric, error) {
31+
args := a.Called(namespace)
3332

34-
return args.Get(0).([]models.Metric), args.Error(1)
33+
return args.Get(0).([]resources.Metric), args.Error(1)
3534
}

‎pkg/tsdb/cloudwatch/models/metric_types.go renamed to ‎pkg/tsdb/cloudwatch/models/api.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ package models
22

33
import (
44
"github.com/aws/aws-sdk-go/service/cloudwatch"
5-
"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/request"
5+
"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources"
66
)
77

88
type ListMetricsProvider interface {
9-
GetDimensionKeysByDimensionFilter(*request.DimensionKeysRequest) ([]string, error)
9+
GetDimensionKeysByDimensionFilter(resources.DimensionKeysRequest) ([]string, error)
1010
GetDimensionKeysByNamespace(string) ([]string, error)
11-
GetDimensionValuesByDimensionFilter(*request.DimensionValuesRequest) ([]string, error)
12-
GetMetricsByNamespace(namespace string) ([]Metric, error)
11+
GetDimensionValuesByDimensionFilter(resources.DimensionValuesRequest) ([]string, error)
12+
GetMetricsByNamespace(namespace string) ([]resources.Metric, error)
1313
}
1414

1515
type MetricsClientProvider interface {

‎pkg/tsdb/cloudwatch/models/request/types.go

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎pkg/tsdb/cloudwatch/models/request/dimension_keys_request.go renamed to ‎pkg/tsdb/cloudwatch/models/resources/dimension_keys_request.go

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package request
1+
package resources
22

33
import (
44
"net/url"
@@ -9,7 +9,6 @@ type DimensionKeysRequestType uint32
99
const (
1010
StandardDimensionKeysRequest DimensionKeysRequestType = iota
1111
FilterDimensionKeysRequest
12-
CustomMetricDimensionKeysRequest
1312
)
1413

1514
type DimensionKeysRequest struct {
@@ -20,24 +19,20 @@ type DimensionKeysRequest struct {
2019
}
2120

2221
func (q *DimensionKeysRequest) Type() DimensionKeysRequestType {
23-
if isCustomNamespace(q.Namespace) {
24-
return CustomMetricDimensionKeysRequest
25-
}
26-
27-
if len(q.DimensionFilter) > 0 {
22+
if isCustomNamespace(q.Namespace) || len(q.DimensionFilter) > 0 {
2823
return FilterDimensionKeysRequest
2924
}
3025

3126
return StandardDimensionKeysRequest
3227
}
3328

34-
func GetDimensionKeysRequest(parameters url.Values) (*DimensionKeysRequest, error) {
29+
func GetDimensionKeysRequest(parameters url.Values) (DimensionKeysRequest, error) {
3530
resourceRequest, err := getResourceRequest(parameters)
3631
if err != nil {
37-
return nil, err
32+
return DimensionKeysRequest{}, err
3833
}
3934

40-
request := &DimensionKeysRequest{
35+
request := DimensionKeysRequest{
4136
ResourceRequest: resourceRequest,
4237
Namespace: parameters.Get("namespace"),
4338
MetricName: parameters.Get("metricName"),
@@ -46,7 +41,7 @@ func GetDimensionKeysRequest(parameters url.Values) (*DimensionKeysRequest, erro
4641

4742
dimensions, err := parseDimensionFilter(parameters.Get("dimensionFilters"))
4843
if err != nil {
49-
return nil, err
44+
return DimensionKeysRequest{}, err
5045
}
5146

5247
request.DimensionFilter = dimensions

0 commit comments

Comments
 (0)