Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cloudwatch): added defaultInterval prop to cw-dashboard #24707

Merged
merged 3 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions packages/@aws-cdk/aws-cloudwatch/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -697,3 +697,18 @@ new cloudwatch.Row(widgetA, widgetB);

You can add a widget after object instantiation with the method
`addWidget()`.

### Interval duration for dashboard

Interval duration for metrics in dashboard. You can specify `defaultInterval` with
the relative time(eg. 7 days) as `cdk.Duration.days(7)`.

```ts
import * as cw from '@aws-cdk/aws-cloudwatch';

const dashboard = new cw.Dashboard(stack, 'Dash', {
defaultInterval: cdk.Duration.days(7),
});
```

Here, the dashboard would show the metrics for the last 7 days.
18 changes: 15 additions & 3 deletions packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Lazy, Resource, Stack, Token, Annotations } from '@aws-cdk/core';
import { Lazy, Resource, Stack, Token, Annotations, Duration } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { CfnDashboard } from './cloudwatch.generated';
import { Column, Row } from './layout';
Expand Down Expand Up @@ -31,6 +31,14 @@ export interface DashboardProps {
*/
readonly dashboardName?: string;

/**
* Interval duration for metrics.
* You can specify defaultInterval with the relative time(eg. cdk.Duration.days(7)).
*
* @default When the dashboard loads, the defaultInterval time will be the default time range.
*/
readonly defaultInterval?: Duration

/**
* The start of the time range to use for each widget on the dashboard.
* You can specify start without specifying end to specify a relative time range that ends with the current time.
Expand Down Expand Up @@ -107,15 +115,19 @@ export class Dashboard extends Resource {
}
}

if (props.start !== undefined && props.defaultInterval !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (props.start !== undefined && props.defaultInterval !== undefined) {
if (props.start && props.defaultInterval) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases it is safe to omit !== undefined, and in some cases it leads to bugs (for example, in the case of nullable integers, and occasionally in the case of nullable strings depending on what you want to do with the string).

When you're not super experienced yet, it's probably safer to always write !== undefined. So I support the current use.

throw ('both properties defaultInterval and start cannot be set at once');
}

const dashboard = new CfnDashboard(this, 'Resource', {
dashboardName: this.physicalName,
dashboardBody: Lazy.string({
produce: () => {
const column = new Column(...this.rows);
column.position(0, 0);
return Stack.of(this).toJsonString({
start: props.start,
end: props.end,
start: props.defaultInterval !== undefined ? `-${props.defaultInterval?.toIsoString()}` : props.start,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
start: props.defaultInterval !== undefined ? `-${props.defaultInterval?.toIsoString()}` : props.start,
start: props.defaultInterval ? `-${props.defaultInterval?.toIsoString()}` : props.start,

Copy link
Contributor

@rix0rrr rix0rrr Mar 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it would be safe to leave out, but also it doesn't hurt that much. Readability might be slightly better without.

end: props.defaultInterval !== undefined ? undefined : props.end,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
end: props.defaultInterval !== undefined ? undefined : props.end,
end: props.defaultInterval ? undefined : props.end,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it would be safe to leave out, but also it doesn't hurt that much. Readability might be slightly better without.

periodOverride: props.periodOverride,
widgets: column.toJson(),
});
Expand Down
27 changes: 26 additions & 1 deletion packages/@aws-cdk/aws-cloudwatch/test/dashboard.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Template, Annotations, Match } from '@aws-cdk/assertions';
import { App, Stack } from '@aws-cdk/core';
import { App, Duration, Stack } from '@aws-cdk/core';
import { Dashboard, GraphWidget, PeriodOverride, TextWidget, MathExpression, TextWidgetBackground } from '../lib';

describe('Dashboard', () => {
Expand Down Expand Up @@ -131,6 +131,31 @@ describe('Dashboard', () => {

});

test('defaultInterval test', () => {
// GIVEN
const stack = new Stack();
// WHEN
const dashboard = new Dashboard(stack, 'Dash', {
defaultInterval: Duration.days(7),
});
dashboard.addWidgets(
new GraphWidget({ width: 1, height: 1 }), // GraphWidget has internal reference to current region
);

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::CloudWatch::Dashboard', {
DashboardBody: {
'Fn::Join': ['', [
'{"start":"-P7D",\
"widgets":[{"type":"metric","width":1,"height":1,"x":0,"y":0,"properties":{"view":"timeSeries","region":"',
{ Ref: 'AWS::Region' },
'","yAxis":{}}}]}',
]],
},
});

});

test('DashboardName is set when provided', () => {
// GIVEN
const app = new App();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "30.0.0",
"version": "31.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "30.0.0",
"version": "31.0.0",
"files": {
"53eb5ec97b9df3953bc84bdc2aee87ace7b502c665b7e5b9f7b7d14dd46cea69": {
"1a70f8470c838c02020b9010528363b17eebd55d55c1a53fb3e0f6760a606c98": {
"source": {
"path": "DashboardIntegrationTestStack.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "53eb5ec97b9df3953bc84bdc2aee87ace7b502c665b7e5b9f7b7d14dd46cea69.json",
"objectKey": "1a70f8470c838c02020b9010528363b17eebd55d55c1a53fb3e0f6760a606c98.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"DashCCD7F836": {
"Type": "AWS::CloudWatch::Dashboard",
"Properties": {
"DashboardBody": "{\"widgets\":[{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":0,\"y\":0,\"properties\":{\"markdown\":\"I don't have a background\",\"background\":\"transparent\"}}]}"
"DashboardBody": "{\"start\":\"-P7D\",\"widgets\":[{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":0,\"y\":0,\"properties\":{\"markdown\":\"I don't have a background\",\"background\":\"transparent\"}}]}"
}
}
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"30.0.0"}
{"version":"31.0.0"}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "30.0.0",
"version": "31.0.0",
"testCases": {
"DashboardIntegrationTest/DefaultTest": {
"stacks": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "30.0.0",
"version": "31.0.0",
"artifacts": {
"DashboardIntegrationTestStack.assets": {
"type": "cdk:asset-manifest",
Expand All @@ -17,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/53eb5ec97b9df3953bc84bdc2aee87ace7b502c665b7e5b9f7b7d14dd46cea69.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/1a70f8470c838c02020b9010528363b17eebd55d55c1a53fb3e0f6760a606c98.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"attributes": {
"aws:cdk:cloudformation:type": "AWS::CloudWatch::Dashboard",
"aws:cdk:cloudformation:props": {
"dashboardBody": "{\"widgets\":[{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":0,\"y\":0,\"properties\":{\"markdown\":\"I don't have a background\",\"background\":\"transparent\"}}]}"
"dashboardBody": "{\"start\":\"-P7D\",\"widgets\":[{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":0,\"y\":0,\"properties\":{\"markdown\":\"I don't have a background\",\"background\":\"transparent\"}}]}"
}
},
"constructInfo": {
Expand Down Expand Up @@ -75,7 +75,7 @@
"path": "DashboardIntegrationTest/DefaultTest/Default",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.252"
"version": "10.1.270"
}
},
"DeployAssert": {
Expand Down Expand Up @@ -121,7 +121,7 @@
"path": "Tree",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.252"
"version": "10.1.270"
}
}
},
Expand Down
4 changes: 3 additions & 1 deletion packages/@aws-cdk/aws-cloudwatch/test/integ.dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ const app = new cdk.App();

const stack = new cdk.Stack(app, 'DashboardIntegrationTestStack');

const dashboard = new cloudwatch.Dashboard(stack, 'Dash');
const dashboard = new cloudwatch.Dashboard(stack, 'Dash', {
defaultInterval: cdk.Duration.days(7),
});

dashboard.addWidgets(new cloudwatch.TextWidget({
markdown: 'I don\'t have a background',
Expand Down