Skip to content

N
nexpie-grafana-theme
Commit d40b66f1 by Erik Sundell Committed by GitHub
Options

CloudWatch: Calculate period based on time range (#21471) 

* Calculate min period based on time range and no of queries

* Use hardcoded array of periods if period is not defined actively by the user

* Fix broken tests

* Use a smaller max period for auto interval

* Fix broken tests

* Test period calculation

* Test min retention period

* Fix broken test
parent 5f3d913c
Showing with 197 additions and 172 deletions
pkg/tsdb/cloudwatch/metric_data_input_builder.go
package cloudwatch
import (
"fmt"
"time"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/cloudwatch"
"github.com/grafana/grafana/pkg/tsdb"
)
func (e *CloudWatchExecutor) buildMetricDataInput(queryContext *tsdb.TsdbQuery, queries map[string]*cloudWatchQuery) (*cloudwatch.GetMetricDataInput, error) {
startTime, err := queryContext.TimeRange.ParseFrom()
if err != nil {
return nil, err
}
endTime, err := queryContext.TimeRange.ParseTo()
if err != nil {
return nil, err
}
if !startTime.Before(endTime) {
return nil, fmt.Errorf("Invalid time range: Start time must be before end time")
}
func (e *CloudWatchExecutor) buildMetricDataInput(startTime time.Time, endTime time.Time, queries map[string]*cloudWatchQuery) (*cloudwatch.GetMetricDataInput, error) {
metricDataInput := &cloudwatch.GetMetricDataInput{
StartTime: aws.Time(startTime),
EndTime: aws.Time(endTime),
......
pkg/tsdb/cloudwatch/query_transformer.go
......@@ -74,6 +74,7 @@ func (e *CloudWatchExecutor) transformQueryResponseToQueryResult(cloudwatchRespo
partialData := false
queryMeta := []struct {
Expression, ID string
Period int
}{}
for _, response := range responses {
......@@ -82,9 +83,11 @@ func (e *CloudWatchExecutor) transformQueryResponseToQueryResult(cloudwatchRespo
partialData = partialData || response.PartialData
queryMeta = append(queryMeta, struct {
Expression, ID string
Period int
}{
Expression: response.Expression,
ID: response.Id,
Period: response.Period,
})
}
......
pkg/tsdb/cloudwatch/request_parser.go
......@@ -2,9 +2,11 @@ package cloudwatch
import (
"errors"
"math"
"regexp"
"sort"
"strconv"
"strings"
"time"
"github.com/aws/aws-sdk-go/aws"
......@@ -13,7 +15,7 @@ import (
)
// Parses the json queries and returns a requestQuery. The requstQuery has a 1 to 1 mapping to a query editor row
func (e *CloudWatchExecutor) parseQueries(queryContext *tsdb.TsdbQuery) (map[string][]*requestQuery, error) {
func (e *CloudWatchExecutor) parseQueries(queryContext *tsdb.TsdbQuery, startTime time.Time, endTime time.Time) (map[string][]*requestQuery, error) {
requestQueries := make(map[string][]*requestQuery)
for i, model := range queryContext.Queries {
......@@ -23,7 +25,7 @@ func (e *CloudWatchExecutor) parseQueries(queryContext *tsdb.TsdbQuery) (map[str
}
RefID := queryContext.Queries[i].RefId
query, err := parseRequestQuery(queryContext.Queries[i].Model, RefID)
query, err := parseRequestQuery(queryContext.Queries[i].Model, RefID, startTime, endTime)
if err != nil {
return nil, &queryError{err, RefID}
}
......@@ -36,7 +38,7 @@ func (e *CloudWatchExecutor) parseQueries(queryContext *tsdb.TsdbQuery) (map[str
return requestQueries, nil
}
func parseRequestQuery(model *simplejson.Json, refId string) (*requestQuery, error) {
func parseRequestQuery(model *simplejson.Json, refId string, startTime time.Time, endTime time.Time) (*requestQuery, error) {
region, err := model.Get("region").String()
if err != nil {
return nil, err
......@@ -63,15 +65,12 @@ func parseRequestQuery(model *simplejson.Json, refId string) (*requestQuery, err
}
p := model.Get("period").MustString("")
if p == "" {
if namespace == "AWS/EC2" {
p = "300"
} else {
p = "60"
}
}
var period int
if strings.ToLower(p) == "auto" || p == "" {
deltaInSeconds := endTime.Sub(startTime).Seconds()
periods := []int{60, 300, 900, 3600, 21600}
period = closest(periods, int(math.Ceil(deltaInSeconds/2000)))
} else {
if regexp.MustCompile(`^\d+$`).Match([]byte(p)) {
period, err = strconv.Atoi(p)
if err != nil {
......@@ -84,6 +83,7 @@ func parseRequestQuery(model *simplejson.Json, refId string) (*requestQuery, err
}
period = int(d.Seconds())
}
}
id := model.Get("id").MustString("")
expression := model.Get("expression").MustString("")
......@@ -158,3 +158,25 @@ func sortDimensions(dimensions map[string][]string) map[string][]string {
}
return sortedDimensions
}
func closest(array []int, num int) int {
minDiff := array[len(array)-1]
var closest int
if num <= array[0] {
return array[0]
}
if num >= array[len(array)-1] {
return array[len(array)-1]
}
for _, value := range array {
var m = int(math.Abs(float64(num - value)))
if m <= minDiff {
minDiff = m
closest = value
}
}
return closest
}
pkg/tsdb/cloudwatch/request_parser_test.go
......@@ -4,11 +4,15 @@ import (
"testing"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/tsdb"
. "github.com/smartystreets/goconvey/convey"
)
func TestRequestParser(t *testing.T) {
Convey("TestRequestParser", t, func() {
timeRange := tsdb.NewTimeRange("now-1h", "now-2h")
from, _ := timeRange.ParseFrom()
to, _ := timeRange.ParseTo()
Convey("when parsing query editor row json", func() {
Convey("using new dimensions structure", func() {
query := simplejson.NewFromAny(map[string]interface{}{
......@@ -27,7 +31,7 @@ func TestRequestParser(t *testing.T) {
"hide": false,
})
res, err := parseRequestQuery(query, "ref1")
res, err := parseRequestQuery(query, "ref1", from, to)
So(err, ShouldBeNil)
So(res.Region, ShouldEqual, "us-east-1")
So(res.RefId, ShouldEqual, "ref1")
......@@ -62,7 +66,7 @@ func TestRequestParser(t *testing.T) {
"hide": false,
})
res, err := parseRequestQuery(query, "ref1")
res, err := parseRequestQuery(query, "ref1", from, to)
So(err, ShouldBeNil)
So(res.Region, ShouldEqual, "us-east-1")
So(res.RefId, ShouldEqual, "ref1")
......@@ -78,6 +82,111 @@ func TestRequestParser(t *testing.T) {
So(res.Dimensions["InstanceType"][0], ShouldEqual, "test2")
So(*res.Statistics[0], ShouldEqual, "Average")
})
Convey("period defined in the editor by the user is being used", func() {
query := simplejson.NewFromAny(map[string]interface{}{
"refId": "ref1",
"region": "us-east-1",
"namespace": "ec2",
"metricName": "CPUUtilization",
"id": "",
"expression": "",
"dimensions": map[string]interface{}{
"InstanceId": "test",
"InstanceType": "test2",
},
"statistics": []interface{}{"Average"},
"hide": false,
})
Convey("when time range is short", func() {
query.Set("period", "900")
timeRange := tsdb.NewTimeRange("now-1h", "now-2h")
from, _ := timeRange.ParseFrom()
to, _ := timeRange.ParseTo()
res, err := parseRequestQuery(query, "ref1", from, to)
So(err, ShouldBeNil)
So(res.Period, ShouldEqual, 900)
})
})
Convey("period is parsed correctly if not defined by user", func() {
query := simplejson.NewFromAny(map[string]interface{}{
"refId": "ref1",
"region": "us-east-1",
"namespace": "ec2",
"metricName": "CPUUtilization",
"id": "",
"expression": "",
"dimensions": map[string]interface{}{
"InstanceId": "test",
"InstanceType": "test2",
},
"statistics": []interface{}{"Average"},
"hide": false,
"period": "auto",
})
Convey("when time range is short", func() {
query.Set("period", "auto")
timeRange := tsdb.NewTimeRange("now-2h", "now-1h")
from, _ := timeRange.ParseFrom()
to, _ := timeRange.ParseTo()
res, err := parseRequestQuery(query, "ref1", from, to)
So(err, ShouldBeNil)
So(res.Period, ShouldEqual, 60)
})
Convey("when time range is 5y", func() {
timeRange := tsdb.NewTimeRange("now-5y", "now")
from, _ := timeRange.ParseFrom()
to, _ := timeRange.ParseTo()
res, err := parseRequestQuery(query, "ref1", from, to)
So(err, ShouldBeNil)
So(res.Period, ShouldEqual, 21600)
})
})
Convey("closest works as expected", func() {
periods := []int{60, 300, 900, 3600, 21600}
Convey("and input is lower than 60", func() {
So(closest(periods, 6), ShouldEqual, 60)
})
Convey("and input is exactly 60", func() {
So(closest(periods, 60), ShouldEqual, 60)
})
Convey("and input is exactly between two steps", func() {
So(closest(periods, 180), ShouldEqual, 300)
})
Convey("and input is exactly 2000", func() {
So(closest(periods, 2000), ShouldEqual, 900)
})
Convey("and input is exactly 5000", func() {
So(closest(periods, 5000), ShouldEqual, 3600)
})
Convey("and input is exactly 50000", func() {
So(closest(periods, 50000), ShouldEqual, 21600)
})
Convey("and period isn't shorter than min retension for 15 days", func() {
So(closest(periods, (60*60*24*15)+1/2000), ShouldBeGreaterThanOrEqualTo, 300)
})
Convey("and period isn't shorter than min retension for 63 days", func() {
So(closest(periods, (60*60*24*63)+1/2000), ShouldBeGreaterThanOrEqualTo, 3600)
})
Convey("and period isn't shorter than min retension for 455 days", func() {
So(closest(periods, (60*60*24*455)+1/2000), ShouldBeGreaterThanOrEqualTo, 21600)
})
})
})
})
}
pkg/tsdb/cloudwatch/response_parser.go
......@@ -48,6 +48,7 @@ func (e *CloudWatchExecutor) parseResponse(metricDataOutputs []*cloudwatch.GetMe
}
response.series = series
response.Period = queries[id].Period
response.Expression = queries[id].UsedExpression
response.RefId = queries[id].RefId
response.Id = queries[id].Id
......@@ -65,7 +66,6 @@ func parseGetMetricDataTimeSeries(metricDataResults map[string]*cloudwatch.Metri
partialData := false
for label, metricDataResult := range metricDataResults {
if *metricDataResult.StatusCode != "Complete" {
// return nil, fmt.Errorf("too many datapoints requested in query %s. Please try to reduce the time range", query.RefId)
partialData = true
}
......
pkg/tsdb/cloudwatch/response_parser_test.go
......@@ -60,7 +60,7 @@ func TestCloudWatchResponseParser(t *testing.T) {
Period: 60,
Alias: "{{LoadBalancer}} Expanded",
}
series, err := parseGetMetricDataTimeSeries(resp, query)
series, _, err := parseGetMetricDataTimeSeries(resp, query)
timeSeries := (*series)[0]
So(err, ShouldBeNil)
......@@ -116,7 +116,7 @@ func TestCloudWatchResponseParser(t *testing.T) {
Period: 60,
Alias: "{{LoadBalancer}} Expanded",
}
series, err := parseGetMetricDataTimeSeries(resp, query)
series, _, err := parseGetMetricDataTimeSeries(resp, query)
timeSeries := (*series)[0]
So(err, ShouldBeNil)
......@@ -172,7 +172,7 @@ func TestCloudWatchResponseParser(t *testing.T) {
Period: 60,
Alias: "{{LoadBalancer}} Expanded",
}
series, err := parseGetMetricDataTimeSeries(resp, query)
series, _, err := parseGetMetricDataTimeSeries(resp, query)
So(err, ShouldBeNil)
So((*series)[0].Name, ShouldEqual, "lb3 Expanded")
......
pkg/tsdb/cloudwatch/time_series_query.go
......@@ -2,6 +2,7 @@ package cloudwatch
import (
"context"
"fmt"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/tsdb"
......@@ -13,7 +14,21 @@ func (e *CloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, queryCo
Results: make(map[string]*tsdb.QueryResult),
}
requestQueriesByRegion, err := e.parseQueries(queryContext)
startTime, err := queryContext.TimeRange.ParseFrom()
if err != nil {
return nil, err
}
endTime, err := queryContext.TimeRange.ParseTo()
if err != nil {
return nil, err
}
if !startTime.Before(endTime) {
return nil, fmt.Errorf("Invalid time range: Start time must be before end time")
}
requestQueriesByRegion, err := e.parseQueries(queryContext, startTime, endTime)
if err != nil {
return results, err
}
......@@ -52,7 +67,7 @@ func (e *CloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, queryCo
return nil
}
metricDataInput, err := e.buildMetricDataInput(queryContext, queries)
metricDataInput, err := e.buildMetricDataInput(startTime, endTime, queries)
if err != nil {
return err
}
......
pkg/tsdb/cloudwatch/metric_data_input_builder_test.go → pkg/tsdb/cloudwatch/time_series_query_test.go
package cloudwatch
import (
"context"
"testing"
"github.com/grafana/grafana/pkg/tsdb"
......@@ -8,19 +9,18 @@ import (
. "github.com/smartystreets/goconvey/convey"
)
func TestMetricDataInputBuilder(t *testing.T) {
Convey("TestMetricDataInputBuilder", t, func() {
func TestTimeSeriesQuery(t *testing.T) {
Convey("TestTimeSeriesQuery", t, func() {
executor := &CloudWatchExecutor{}
query := make(map[string]*cloudWatchQuery)
Convey("Time range is valid", func() {
Convey("End time before start time should result in error", func() {
_, err := executor.buildMetricDataInput(&tsdb.TsdbQuery{TimeRange: tsdb.NewTimeRange("now-1h", "now-2h")}, query)
_, err := executor.executeTimeSeriesQuery(context.TODO(), &tsdb.TsdbQuery{TimeRange: tsdb.NewTimeRange("now-1h", "now-2h")})
So(err.Error(), ShouldEqual, "Invalid time range: Start time must be before end time")
})
Convey("End time equals start time should result in error", func() {
_, err := executor.buildMetricDataInput(&tsdb.TsdbQuery{TimeRange: tsdb.NewTimeRange("now-1h", "now-1h")}, query)
_, err := executor.executeTimeSeriesQuery(context.TODO(), &tsdb.TsdbQuery{TimeRange: tsdb.NewTimeRange("now-1h", "now-1h")})
So(err.Error(), ShouldEqual, "Invalid time range: Start time must be before end time")
})
})
......
pkg/tsdb/cloudwatch/types.go
......@@ -37,6 +37,7 @@ type cloudwatchResponse struct {
Expression string
RequestExceededMaxLimit bool
PartialData bool
Period int
}
type queryError struct {
......
public/app/plugins/datasource/cloudwatch/components/QueryEditor.tsx
......@@ -165,14 +165,16 @@ export class QueryEditor extends PureComponent<Props, State> {
<tr>
<th>Metric Data Query ID</th>
<th>Metric Data Query Expression</th>
<th>Period</th>
<th />
</tr>
</thead>
<tbody>
{data.series[0].meta.gmdMeta.map(({ ID, Expression }: any) => (
{data.series[0].meta.gmdMeta.map(({ ID, Expression, Period }: any) => (
<tr key={ID}>
<td>{ID}</td>
<td>{Expression}</td>
<td>{Period}</td>
</tr>
))}
</tbody>
......
public/app/plugins/datasource/cloudwatch/components/Stats.tsx
......@@ -31,7 +31,6 @@ export const Stats: FunctionComponent<Props> = ({ stats, values, onChange, varia
}
/>
))}
{values.length !== stats.length && (
<Segment
Component={
<a className="gf-form-label query-part">
......@@ -42,6 +41,5 @@ export const Stats: FunctionComponent<Props> = ({ stats, values, onChange, varia
onChange={({ value }) => onChange([...values, value])}
options={[...stats.filter(({ value }) => !values.includes(value)), variableOptionGroup]}
/>
)}
</>
);
public/app/plugins/datasource/cloudwatch/datasource.ts
......@@ -125,52 +125,29 @@ export default class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery,
return this.templateSrv.variables.map(v => `$${v.name}`);
}
getPeriod(target: any, options: any, now?: number) {
const start = this.convertToCloudWatchTime(options.range.from, false);
now = Math.round((now || Date.now()) / 1000);
let period;
const hourSec = 60 * 60;
const daySec = hourSec * 24;
if (!target.period) {
if (now - start <= daySec * 15) {
// until 15 days ago
if (target.namespace === 'AWS/EC2') {
period = 300;
} else {
period = 60;
}
} else if (now - start <= daySec * 63) {
// until 63 days ago
period = 60 * 5;
} else if (now - start <= daySec * 455) {
// until 455 days ago
period = 60 * 60;
} else {
// over 455 days, should return error, but try to long period
period = 60 * 60;
}
} else {
period = this.templateSrv.replace(target.period, options.scopedVars);
getPeriod(target: any, options: any) {
let period = this.templateSrv.replace(target.period, options.scopedVars);
if (period && period.toLowerCase() !== 'auto') {
if (/^\d+$/.test(period)) {
period = parseInt(period, 10);
} else {
period = kbn.interval_to_seconds(period);
}
}
if (period < 1) {
period = 1;
}
}
return period;
}
buildCloudwatchConsoleUrl(
{ region, namespace, metricName, dimensions, statistics, period, expression }: CloudWatchQuery,
{ region, namespace, metricName, dimensions, statistics, expression }: CloudWatchQuery,
start: string,
end: string,
title: string,
gmdMeta: Array<{ Expression: string }>
gmdMeta: Array<{ Expression: string; Period: string }>
) {
region = this.getActualRegion(region);
let conf = {
......@@ -204,7 +181,7 @@ export default class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery,
...Object.entries(dimensions).reduce((acc, [key, value]) => [...acc, key, value[0]], []),
{
stat,
period,
period: gmdMeta.length ? gmdMeta[0].Period : 60,
},
]),
],
......
public/app/plugins/datasource/cloudwatch/specs/datasource.test.ts
......@@ -67,7 +67,7 @@ describe('CloudWatchDatasource', () => {
A: {
error: '',
refId: 'A',
meta: {},
meta: { gmdMeta: [] },
series: [
{
name: 'CPUUtilization_Average',
......@@ -181,7 +181,7 @@ describe('CloudWatchDatasource', () => {
});
it('should be built correctly if theres one search expressions returned in meta for a given query row', done => {
response.results['A'].meta.gmdMeta = [{ Expression: `REMOVE_EMPTY(SEARCH('some expression'))` }];
response.results['A'].meta.gmdMeta = [{ Expression: `REMOVE_EMPTY(SEARCH('some expression'))`, Period: '300' }];
ctx.ds.query(query).then((result: any) => {
expect(result.data[0].name).toBe(response.results.A.series[0].name);
expect(result.data[0].fields[0].config.links[0].title).toBe('View in CloudWatch console');
......@@ -208,7 +208,7 @@ describe('CloudWatchDatasource', () => {
});
it('should be built correctly if the query is a metric stat query', done => {
response.results['A'].meta.gmdMeta = [];
response.results['A'].meta.gmdMeta = [{ Period: '300' }];
ctx.ds.query(query).then((result: any) => {
expect(result.data[0].name).toBe(response.results.A.series[0].name);
expect(result.data[0].fields[0].config.links[0].title).toBe('View in CloudWatch console');
......@@ -415,7 +415,13 @@ describe('CloudWatchDatasource', () => {
A: {
error: '',
refId: 'A',
meta: {},
meta: {
gmdMeta: [
{
Period: 300,
},
],
},
series: [
{
name: 'TargetResponseTime_p90.00',
......@@ -789,97 +795,4 @@ describe('CloudWatchDatasource', () => {
});
}
);
it('should caclculate the correct period', () => {
const hourSec = 60 * 60;
const daySec = hourSec * 24;
const start = 1483196400 * 1000;
const testData: any[] = [
[
{ period: '60s', namespace: 'AWS/EC2' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
hourSec * 3,
60,
],
[
{ period: null, namespace: 'AWS/EC2' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
hourSec * 3,
300,
],
[
{ period: '60s', namespace: 'AWS/ELB' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
hourSec * 3,
60,
],
[
{ period: null, namespace: 'AWS/ELB' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
hourSec * 3,
60,
],
[
{ period: '1', namespace: 'CustomMetricsNamespace' },
{
range: {
from: new Date(start),
to: new Date(start + (1440 - 1) * 1000),
},
},
hourSec * 3 - 1,
1,
],
[
{ period: '1', namespace: 'CustomMetricsNamespace' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
hourSec * 3 - 1,
1,
],
[
{ period: '60s', namespace: 'CustomMetricsNamespace' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
hourSec * 3,
60,
],
[
{ period: null, namespace: 'CustomMetricsNamespace' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
hourSec * 3 - 1,
60,
],
[
{ period: null, namespace: 'CustomMetricsNamespace' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
hourSec * 3,
60,
],
[
{ period: null, namespace: 'CustomMetricsNamespace' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
daySec * 15,
60,
],
[
{ period: null, namespace: 'CustomMetricsNamespace' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
daySec * 63,
300,
],
[
{ period: null, namespace: 'CustomMetricsNamespace' },
{ range: { from: new Date(start), to: new Date(start + 3600 * 1000) } },
daySec * 455,
3600,
],
];
for (const t of testData) {
const target = t[0];
const options = t[1];
const now = new Date(options.range.from.valueOf() + t[2] * 1000);
const expected = t[3];
const actual = ctx.ds.getPeriod(target, options, now);
expect(actual).toBe(expected);
}
});
});
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment