feat(cloudwatch): added defaultInterval prop to cw-dashboard #24707
Conversation
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
pattasai
changed the title
| @@ -107,15 +115,19 @@ export class Dashboard extends Resource { | |||
| } | |||
| } | |||
|
|
|||
| if (props.start !== undefined && props.defaultInterval !== undefined) { | |||
There was a problem hiding this comment.
| if (props.start !== undefined && props.defaultInterval !== undefined) { | |
| if (props.start && props.defaultInterval) { |
There was a problem hiding this comment.
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.
| 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, |
There was a problem hiding this comment.
| start: props.defaultInterval !== undefined ? `-${props.defaultInterval?.toIsoString()}` : props.start, | |
| start: props.defaultInterval ? `-${props.defaultInterval?.toIsoString()}` : props.start, |
There was a problem hiding this comment.
Here it would be safe to leave out, but also it doesn't hurt that much. Readability might be slightly better without.
| start: props.start, | ||
| end: props.end, | ||
| start: props.defaultInterval !== undefined ? `-${props.defaultInterval?.toIsoString()}` : props.start, | ||
| end: props.defaultInterval !== undefined ? undefined : props.end, |
There was a problem hiding this comment.
| end: props.defaultInterval !== undefined ? undefined : props.end, | |
| end: props.defaultInterval ? undefined : props.end, |
There was a problem hiding this comment.
Here it would be safe to leave out, but also it doesn't hurt that much. Readability might be slightly better without.
rix0rrr
dismissed
corymhall’s stale review
Overruled by the senate, peace treaty signed :P
pattasai
changed the title
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
This PR adds defaultInterval to cloudwatch dashboard, which allows interval duration in relative time eg. 7 days.
```ts
const dashboard = cw.Dashboard(stack, 'Dash', {
defaultInterval: cdk.Duration.days(7),
});
```
Here, the dashboard would show the metrics for the last 7 days.
> [CONTRIBUTING GUIDE]: https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md
> [DESIGN GUIDELINES]: https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md
Closes #<issue number here>.
----
*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR adds defaultInterval to cloudwatch dashboard, which allows interval duration in relative time eg. 7 days.
Here, the dashboard would show the metrics for the last 7 days.
Closes #.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license