Skip to content

Commit

Permalink
Cloudwatch: Fix deeplink with default region (#60260) (#60273)
Browse files Browse the repository at this point in the history 
Cloudwatch: fix deeplink with default region
(cherry picked from commit d6bb2a7)
  • Loading branch information
@iwysiu
iwysiu committed Dec 13, 2022
1 parent d7dcea7 commit 796e2e0
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 27 deletions.
10 changes: 7 additions & 3 deletions pkg/tsdb/cloudwatch/request_parser.go
View file
Expand Up @@ -43,7 +43,7 @@ type QueryJson struct {
}

// parseQueries parses the json queries and returns a map of cloudWatchQueries by region. The cloudWatchQuery has a 1 to 1 mapping to a query editor row
func (e *cloudWatchExecutor) parseQueries(queries []backend.DataQuery, startTime time.Time, endTime time.Time) (map[string][]*cloudWatchQuery, error) {
func (e *cloudWatchExecutor) parseQueries(queries []backend.DataQuery, startTime time.Time, endTime time.Time, defaultRegionValue string) (map[string][]*cloudWatchQuery, error) {
requestQueries := make(map[string][]*cloudWatchQuery)
migratedQueries, err := migrateLegacyQuery(queries, e.features.IsEnabled(featuremgmt.FlagCloudWatchDynamicLabels))
if err != nil {
Expand All @@ -68,7 +68,7 @@ func (e *cloudWatchExecutor) parseQueries(queries []backend.DataQuery, startTime
}

refID := query.RefID
query, err := parseRequestQuery(model, refID, startTime, endTime)
query, err := parseRequestQuery(model, refID, startTime, endTime, defaultRegionValue)
if err != nil {
return nil, &queryError{err: err, RefID: refID}
}
Expand Down Expand Up @@ -159,7 +159,7 @@ func migrateAliasToDynamicLabel(queryJson *QueryJson) {
queryJson.Label = &fullAliasField
}

func parseRequestQuery(model QueryJson, refId string, startTime time.Time, endTime time.Time) (*cloudWatchQuery, error) {
func parseRequestQuery(model QueryJson, refId string, startTime time.Time, endTime time.Time, defaultRegionValue string) (*cloudWatchQuery, error) {
plog.Debug("Parsing request query", "query", model)
cloudWatchQuery := cloudWatchQuery{
Alias: "",
Expand Down Expand Up @@ -264,6 +264,10 @@ func parseRequestQuery(model QueryJson, refId string, startTime time.Time, endTi
cloudWatchQuery.Label = *model.Label
}

if model.Region == defaultRegion {
cloudWatchQuery.Region = defaultRegionValue
}

return &cloudWatchQuery, nil
}

Expand Down
55 changes: 32 additions & 23 deletions pkg/tsdb/cloudwatch/request_parser_test.go
View file
Expand Up @@ -73,7 +73,7 @@ func TestRequestParser(t *testing.T) {
Hide: &false,
}

res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
assert.Equal(t, "us-east-1", res.Region)
assert.Equal(t, "ref1", res.RefId)
Expand Down Expand Up @@ -107,7 +107,7 @@ func TestRequestParser(t *testing.T) {
Hide: &false,
}

res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
assert.Equal(t, "us-east-1", res.Region)
assert.Equal(t, "ref1", res.RefId)
Expand Down Expand Up @@ -141,7 +141,7 @@ func TestRequestParser(t *testing.T) {
Hide: &false,
}

res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
assert.Equal(t, 900, res.Period)
})
Expand All @@ -168,7 +168,7 @@ func TestRequestParser(t *testing.T) {
to := time.Now()
from := to.Local().Add(time.Minute * time.Duration(5))

res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 60, res.Period)
})
Expand All @@ -178,7 +178,7 @@ func TestRequestParser(t *testing.T) {
to := time.Now()
from := to.AddDate(0, 0, -1)

res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 60, res.Period)
})
Expand All @@ -187,7 +187,7 @@ func TestRequestParser(t *testing.T) {
query.Period = "auto"
to := time.Now()
from := to.AddDate(0, 0, -2)
res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 300, res.Period)
})
Expand All @@ -197,7 +197,7 @@ func TestRequestParser(t *testing.T) {
to := time.Now()
from := to.AddDate(0, 0, -7)

res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 900, res.Period)
})
Expand All @@ -207,7 +207,7 @@ func TestRequestParser(t *testing.T) {
to := time.Now()
from := to.AddDate(0, 0, -30)

res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 3600, res.Period)
})
Expand All @@ -217,7 +217,7 @@ func TestRequestParser(t *testing.T) {
to := time.Now()
from := to.AddDate(0, 0, -90)

res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 21600, res.Period)
})
Expand All @@ -227,7 +227,7 @@ func TestRequestParser(t *testing.T) {
to := time.Now()
from := to.AddDate(-1, 0, 0)

res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.Nil(t, err)
assert.Equal(t, 21600, res.Period)
})
Expand All @@ -237,7 +237,7 @@ func TestRequestParser(t *testing.T) {
to := time.Now()
from := to.AddDate(-2, 0, 0)

res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 86400, res.Period)
})
Expand All @@ -246,7 +246,7 @@ func TestRequestParser(t *testing.T) {
query.Period = "auto"
to := time.Now().AddDate(0, 0, -14)
from := to.AddDate(0, 0, -2)
res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 300, res.Period)
})
Expand All @@ -255,7 +255,7 @@ func TestRequestParser(t *testing.T) {
query.Period = "auto"
to := time.Now().AddDate(0, 0, -88)
from := to.AddDate(0, 0, -2)
res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 3600, res.Period)
})
Expand All @@ -264,7 +264,7 @@ func TestRequestParser(t *testing.T) {
query.Period = "auto"
to := time.Now().AddDate(0, 0, -454)
from := to.AddDate(0, 0, -2)
res, err := parseRequestQuery(query, "ref1", from, to)
res, err := parseRequestQuery(query, "ref1", from, to, "us-east-2")
require.NoError(t, err)
assert.Equal(t, 21600, res.Period)
})
Expand All @@ -274,7 +274,7 @@ func TestRequestParser(t *testing.T) {
t.Run("when metric query type and metric editor mode is not specified", func(t *testing.T) {
t.Run("it should be metric search builder", func(t *testing.T) {
query := getBaseJsonQuery()
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
assert.Equal(t, MetricQueryTypeSearch, res.MetricQueryType)
assert.Equal(t, MetricEditorModeBuilder, res.MetricEditorMode)
Expand All @@ -284,7 +284,7 @@ func TestRequestParser(t *testing.T) {
t.Run("and an expression is specified it should be metric search builder", func(t *testing.T) {
query := getBaseJsonQuery()
query.Expression = "SUM(a)"
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
assert.Equal(t, MetricQueryTypeSearch, res.MetricQueryType)
assert.Equal(t, MetricEditorModeRaw, res.MetricEditorMode)
Expand All @@ -295,7 +295,7 @@ func TestRequestParser(t *testing.T) {
t.Run("and an expression is specified it should be metric search builder", func(t *testing.T) {
query := getBaseJsonQuery()
query.Expression = "SUM(a)"
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
assert.Equal(t, MetricQueryTypeSearch, res.MetricQueryType)
assert.Equal(t, MetricEditorModeRaw, res.MetricEditorMode)
Expand All @@ -307,7 +307,7 @@ func TestRequestParser(t *testing.T) {
t.Run("default", func(t *testing.T) {
query := getBaseJsonQuery()
query.QueryType = "timeSeriesQuery"
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
require.True(t, res.ReturnData)
})
Expand All @@ -316,7 +316,7 @@ func TestRequestParser(t *testing.T) {
query.QueryType = "timeSeriesQuery"
true := true
query.Hide = &true
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
require.False(t, res.ReturnData)
})
Expand All @@ -325,15 +325,15 @@ func TestRequestParser(t *testing.T) {
query.QueryType = "timeSeriesQuery"
false := false
query.Hide = &false
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
require.True(t, res.ReturnData)
})
})

t.Run("ID is the string `query` appended with refId if refId is a valid MetricData ID", func(t *testing.T) {
query := getBaseJsonQuery()
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
assert.Equal(t, "ref1", res.RefId)
assert.Equal(t, "queryref1", res.Id)
Expand All @@ -342,7 +342,7 @@ func TestRequestParser(t *testing.T) {
t.Run("Valid id is generated if ID is not provided and refId is not a valid MetricData ID", func(t *testing.T) {
query := getBaseJsonQuery()
query.RefId = "$$"
res, err := parseRequestQuery(query, "$$", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "$$", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")
require.NoError(t, err)
assert.Equal(t, "$$", res.RefId)
assert.Regexp(t, validMetricDataID, res.Id)
Expand All @@ -356,12 +356,21 @@ func TestRequestParser(t *testing.T) {
label := "some label"
query.Label = &label

res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2")

assert.NoError(t, err)
assert.Equal(t, "some alias", res.Alias) // alias is unmodified
assert.Equal(t, "some label", res.Label)
})

t.Run("default region is used when when region not set", func(t *testing.T) {
query := getBaseJsonQuery()
query.Region = defaultRegion
region := "us-east-2"
res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), region)
assert.NoError(t, err)
assert.Equal(t, region, res.Region)
})
}

func getBaseJsonQuery() QueryJson {
Expand Down
7 changes: 6 additions & 1 deletion pkg/tsdb/cloudwatch/time_series_query.go
View file
Expand Up @@ -28,7 +28,12 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, req *ba
return nil, fmt.Errorf("invalid time range: start time must be before end time")
}

requestQueriesByRegion, err := e.parseQueries(req.Queries, startTime, endTime)
dsInfo, err := e.getDSInfo(req.PluginContext)
if err != nil {
return nil, err
}

requestQueriesByRegion, err := e.parseQueries(req.Queries, startTime, endTime, dsInfo.region)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit 796e2e0

Please sign in to comment.