diff --git a/pkg/tsdb/cloudwatch/cloudwatch.go b/pkg/tsdb/cloudwatch/cloudwatch.go index 85a33992369..cd3fb5c501f 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch.go +++ b/pkg/tsdb/cloudwatch/cloudwatch.go @@ -151,6 +151,7 @@ func (e *cloudWatchExecutor) getRequestContext(ctx context.Context, pluginCtx ba EC2APIProvider: ec2Client, Settings: instance.Settings, Features: e.features, + Logger: logger, }, nil } diff --git a/pkg/tsdb/cloudwatch/models/api.go b/pkg/tsdb/cloudwatch/models/api.go index bcda4d2df1f..4abcb4346ed 100644 --- a/pkg/tsdb/cloudwatch/models/api.go +++ b/pkg/tsdb/cloudwatch/models/api.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/oam" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" ) @@ -24,6 +25,7 @@ type RequestContext struct { EC2APIProvider EC2APIProvider Settings CloudWatchSettings Features featuremgmt.FeatureToggles + Logger log.Logger } // Services diff --git a/pkg/tsdb/cloudwatch/routes/regions.go b/pkg/tsdb/cloudwatch/routes/regions.go index 4fd9fbfd57f..ff7a0aad232 100644 --- a/pkg/tsdb/cloudwatch/routes/regions.go +++ b/pkg/tsdb/cloudwatch/routes/regions.go @@ -44,5 +44,5 @@ var newRegionsService = func(ctx context.Context, pluginCtx backend.PluginContex return nil, err } - return services.NewRegionsService(reqCtx.EC2APIProvider), nil + return services.NewRegionsService(reqCtx.EC2APIProvider, reqCtx.Logger), nil } diff --git a/pkg/tsdb/cloudwatch/services/regions.go b/pkg/tsdb/cloudwatch/services/regions.go index 8662afba988..37bbac3172d 100644 --- a/pkg/tsdb/cloudwatch/services/regions.go +++ b/pkg/tsdb/cloudwatch/services/regions.go @@ -3,6 +3,8 @@ package services import ( "sort" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/aws/aws-sdk-go/service/ec2" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/constants" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" @@ -11,11 +13,13 @@ import ( type RegionsService struct { models.EC2APIProvider + log.Logger } -func NewRegionsService(ec2client models.EC2APIProvider) models.RegionsAPIProvider { +func NewRegionsService(ec2client models.EC2APIProvider, logger log.Logger) models.RegionsAPIProvider { return &RegionsService{ ec2client, + logger, } } @@ -33,8 +37,11 @@ func (r *RegionsService) GetRegions() ([]resources.ResourceResponse[resources.Re result := make([]resources.ResourceResponse[resources.Region], 0) ec2Regions, err := r.DescribeRegions(&ec2.DescribeRegionsInput{}) + // we ignore this error and always send default regions + // we only fetch incase a user has enabled additional regions + // but we still log it in case the user is expecting to fetch regions specific to their account and are unable to if err != nil { - return nil, err + r.Error("Failed to get regions: ", "error", err) } mergeEC2RegionsAndConstantRegions(regions, ec2Regions.Regions) diff --git a/pkg/tsdb/cloudwatch/services/regions_test.go b/pkg/tsdb/cloudwatch/services/regions_test.go index 20334d58cfc..9778a43b135 100644 --- a/pkg/tsdb/cloudwatch/services/regions_test.go +++ b/pkg/tsdb/cloudwatch/services/regions_test.go @@ -4,12 +4,15 @@ import ( "testing" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils" "github.com/stretchr/testify/assert" ) +var testLogger = log.New("test logger") + func TestRegions(t *testing.T) { t.Run("returns regions from the api and merges them with default regions", func(t *testing.T) { mockRegions := &ec2.DescribeRegionsOutput{ @@ -21,7 +24,7 @@ func TestRegions(t *testing.T) { } ec2Mock := &mocks.EC2Mock{} ec2Mock.On("DescribeRegions").Return(mockRegions, nil) - regions, err := NewRegionsService(ec2Mock).GetRegions() + regions, err := NewRegionsService(ec2Mock, testLogger).GetRegions() assert.NoError(t, err) assert.Contains(t, regions, resources.ResourceResponse[resources.Region]{ Value: resources.Region{ @@ -35,10 +38,18 @@ func TestRegions(t *testing.T) { }) }) - t.Run("forwards error if DescribeRegions errors out", func(t *testing.T) { + t.Run("always returns default regions, even if fetch fails", func(t *testing.T) { ec2Mock := &mocks.EC2Mock{} - ec2Mock.On("DescribeRegions").Return((*ec2.DescribeRegionsOutput)(nil), assert.AnError) - _, err := NewRegionsService(ec2Mock).GetRegions() - assert.Error(t, err) + mockRegions := &ec2.DescribeRegionsOutput{ + Regions: []*ec2.Region{}, + } + ec2Mock.On("DescribeRegions").Return(mockRegions, assert.AnError) + regions, err := NewRegionsService(ec2Mock, testLogger).GetRegions() + assert.NoError(t, err) + assert.Contains(t, regions, resources.ResourceResponse[resources.Region]{ + Value: resources.Region{ + Name: "us-east-2", + }, + }) }) }