From f4e108c34f211c915d10ad6550a44c31f77ce18e Mon Sep 17 00:00:00 2001
From: Isabella Siu <Isabella.siu@grafana.com>
Date: Tue, 3 Dec 2024 15:02:52 -0500
Subject: [PATCH] Cloudwatch: Accept empty string for logstimeout and mark
 errors downstream (#96947)

---
 pkg/tsdb/cloudwatch/annotation_query.go     |  3 ++-
 pkg/tsdb/cloudwatch/log_sync_query.go       |  3 ++-
 pkg/tsdb/cloudwatch/models/settings.go      |  8 +++++--
 pkg/tsdb/cloudwatch/models/settings_test.go | 24 +++++++++++++++++++++
 pkg/tsdb/cloudwatch/time_series_query.go    |  3 ++-
 5 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/pkg/tsdb/cloudwatch/annotation_query.go b/pkg/tsdb/cloudwatch/annotation_query.go
index 074f30d5096..d2621419852 100644
--- a/pkg/tsdb/cloudwatch/annotation_query.go
+++ b/pkg/tsdb/cloudwatch/annotation_query.go
@@ -59,7 +59,8 @@ func (e *cloudWatchExecutor) executeAnnotationQuery(ctx context.Context, pluginC
 
 	cli, err := e.getCWClient(ctx, pluginCtx, region)
 	if err != nil {
-		return nil, err
+		result = errorsource.AddDownstreamErrorToResponse(query.RefID, result, fmt.Errorf("%v: %w", "failed to get client", err))
+		return result, nil
 	}
 
 	var alarmNames []*string
diff --git a/pkg/tsdb/cloudwatch/log_sync_query.go b/pkg/tsdb/cloudwatch/log_sync_query.go
index 3baaac9c076..f5193863e45 100644
--- a/pkg/tsdb/cloudwatch/log_sync_query.go
+++ b/pkg/tsdb/cloudwatch/log_sync_query.go
@@ -24,7 +24,8 @@ var executeSyncLogQuery = func(ctx context.Context, e *cloudWatchExecutor, req *
 
 	instance, err := e.getInstance(ctx, req.PluginContext)
 	if err != nil {
-		return nil, err
+		errorsource.AddErrorToResponse(req.Queries[0].RefID, resp, err)
+		return resp, nil
 	}
 
 	for _, q := range req.Queries {
diff --git a/pkg/tsdb/cloudwatch/models/settings.go b/pkg/tsdb/cloudwatch/models/settings.go
index 58e25d736e4..aa341f4d073 100644
--- a/pkg/tsdb/cloudwatch/models/settings.go
+++ b/pkg/tsdb/cloudwatch/models/settings.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/grafana/grafana-aws-sdk/pkg/awsds"
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
+	"github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource"
 )
 
 type Duration struct {
@@ -61,13 +62,16 @@ func (duration *Duration) UnmarshalJSON(b []byte) error {
 	case float64:
 		*duration = Duration{time.Duration(value)}
 	case string:
+		if value == "" {
+			return nil
+		}
 		dur, err := time.ParseDuration(value)
 		if err != nil {
-			return err
+			return errorsource.DownstreamError(err, false)
 		}
 		*duration = Duration{dur}
 	default:
-		return fmt.Errorf("invalid duration: %#v", unmarshalledJson)
+		return errorsource.DownstreamError(fmt.Errorf("invalid duration: %#v", unmarshalledJson), false)
 	}
 
 	return nil
diff --git a/pkg/tsdb/cloudwatch/models/settings_test.go b/pkg/tsdb/cloudwatch/models/settings_test.go
index 8b6579ac1ea..ed51c54fb05 100644
--- a/pkg/tsdb/cloudwatch/models/settings_test.go
+++ b/pkg/tsdb/cloudwatch/models/settings_test.go
@@ -2,12 +2,14 @@ package models
 
 import (
 	"context"
+	"errors"
 	"testing"
 	"time"
 
 	"github.com/grafana/grafana-aws-sdk/pkg/awsds"
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana-plugin-sdk-go/backend/proxy"
+	"github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -114,6 +116,24 @@ func Test_Settings_LoadCloudWatchSettings(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, time.Minute*30, s.LogsTimeout.Duration)
 	})
+	t.Run("Should set logsTimeout to default duration if it is empty string", func(t *testing.T) {
+		settings := backend.DataSourceInstanceSettings{
+			ID: 33,
+			JSONData: []byte(`{
+			"authType": "arn",
+			"assumeRoleArn": "arn:aws:iam::123456789012:role/grafana",
+			"logsTimeout": ""
+		  }`),
+			DecryptedSecureJSONData: map[string]string{
+				"accessKey": "AKIAIOSFODNN7EXAMPLE",
+				"secretKey": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
+			},
+		}
+
+		s, err := LoadCloudWatchSettings(settingCtx, settings)
+		require.NoError(t, err)
+		assert.Equal(t, time.Minute*30, s.LogsTimeout.Duration)
+	})
 	t.Run("Should correctly parse logsTimeout duration string", func(t *testing.T) {
 		settings := backend.DataSourceInstanceSettings{
 			ID: 33,
@@ -184,6 +204,10 @@ func Test_Settings_LoadCloudWatchSettings(t *testing.T) {
 
 		_, err := LoadCloudWatchSettings(context.Background(), settings)
 		require.Error(t, err)
+		var sourceErr errorsource.Error
+		ok := errors.As(err, &sourceErr)
+		require.True(t, ok)
+		require.Equal(t, sourceErr.ErrorSource().String(), "downstream")
 	})
 	t.Run("Should throw error if logsTimeout is an invalid type", func(t *testing.T) {
 		settings := backend.DataSourceInstanceSettings{
diff --git a/pkg/tsdb/cloudwatch/time_series_query.go b/pkg/tsdb/cloudwatch/time_series_query.go
index e872c319d76..4025aeba77c 100644
--- a/pkg/tsdb/cloudwatch/time_series_query.go
+++ b/pkg/tsdb/cloudwatch/time_series_query.go
@@ -35,7 +35,8 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, req *ba
 
 	instance, err := e.getInstance(ctx, req.PluginContext)
 	if err != nil {
-		return nil, err
+		errorsource.AddErrorToResponse(req.Queries[0].RefID, resp, err)
+		return resp, nil
 	}
 
 	requestQueries, err := models.ParseMetricDataQueries(req.Queries, startTime, endTime, instance.Settings.Region, e.logger.FromContext(ctx),