feat(cloudwatch): Widgets can define start and end times, including relative values#26969
Conversation
aws-cdk-automation
left a comment
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.
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
| * | ||
| * @default When the dashboard loads, the start time will be the default time range. | ||
| */ | ||
| readonly start?: string; |
There was a problem hiding this comment.
hmm i don't love the idea that we're typing these values as string (both for start and end).
can you either:
a) point me to a place elsewhere in aws-cdk-lib where we already type something similar as a string.
b) help me design a better type system that can then generate the format that cloudwatch wants
There was a problem hiding this comment.
I can certainly see that.
a. The start and end are already used in Dashboard.
aws-cdk/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts
Lines 44 to 63 in 2107790
b. For example, I came up with a way to create a private constructor and a static method for each type.
export class StartTimeRange {
public static fromMinutes(start: number): StartTimeRange {
if (start <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange(`-PT${start}M`);
}
public static fromHours(start: number): StartTimeRange {
if (start <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange(`-PT${start}H`);
}
public static fromDays(start: number): StartTimeRange {
if (start <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange(`-P${start}D`);
}
public static fromWeeks(start: number): StartTimeRange {
if (start <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange(`-P${start}W`);
}
public static fromMonths(start: number): StartTimeRange {
if (start <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange(`-P${start}M`);
}
public static fromDate(start: Date): StartTimeRange {
return new StartTimeRange(start.toISOString());
}
private constructor(public readonly start: string) { }
}
export class EndTimeRange {
public static fromDate(end: Date): EndTimeRange {
return new EndTimeRange(end.toISOString());
}
private constructor(public readonly end: string) { }
}There was a problem hiding this comment.
b-2. Or I thought of this too. By creating StartTimeRange interface and separating into unit and date, the combination of end and start can be validated. (In the case of the above b example, the validation can be implemented to check whether starts with '-P' as string.)
e.g. If end is specified, start must specify date instead of unit.
export interface StartTimeRangeType {
unit?: string; // ex. -PT5M, -P3M.
date?: string; // ISO 8601 format. For example, 2018-12-17T06:00:00.000Z.
}
export class StartTimeRange {
public static fromMinutes(unit: number): StartTimeRange {
if (unit <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange({
unit: `-PT${unit}M`,
});
}
public static fromHours(unit: number): StartTimeRange {
if (unit <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange({
unit: `-PT${unit}H`,
});
}
public static fromDays(unit: number): StartTimeRange {
if (unit <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange({
unit: `-P${unit}D`,
});
}
public static fromWeeks(unit: number): StartTimeRange {
if (unit <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange({
unit: `-P${unit}W`,
});
}
public static fromMonths(unit: number): StartTimeRange {
if (unit <= 0) {
throw new Error('start must be greater than 0');
}
return new StartTimeRange({
unit: `-P${unit}M`,
});
}
public static fromDate(date: Date): StartTimeRange {
return new StartTimeRange({
date: date.toISOString(),
});
}
private constructor(public readonly start: StartTimeRangeType) {}
}
export class EndTimeRange {
public static fromDate(date: Date): EndTimeRange {
return new EndTimeRange(date.toISOString());
}
private constructor(public readonly end: string) { }
}
export class GaugeWidget extends Resource {
// ...
// ...
constructor(scope: Construct, id: string, props: GaugeWidgetProps = {}) {
// ...
// ...
if (props.end !== undefined && props.start === undefined) {
throw new Error('If you specify a value for end, you must also specify a value for start.');
}
if (props.end !== undefined && props.start?.start.date === undefined) {
throw new Error('If `end` is specified, `start` must be specified by `StartTimeRange.fromDate`.');There was a problem hiding this comment.
Had a chat with @kaizencc
Given that we already use a string elsewhere, and that it might be quite an effort to provide a complete implementation of this rather complex standard, using string will be fine.
Users can always fallback to native language features and provide something like new Date().toISOString()
Widget can have start and end times
|
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). |
Widget can have start and end timesstart and end times
start and end timesstart and end times
start and end timesstart and end times, including relative values
|
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). |
Pull request has been modified.
|
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). |
…ng relative values (#26969) This PR adds `start` and `end` properties to Widgets. These properties can be used to specify the time range for each graph widget independently from those of the dashboard. The parameters can be specified at `GraphWidget`, `GaugeWidget`, and `SingleValueWidget`. Closes #26945. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR adds
startandendproperties to Widgets.These properties can be used to specify the time range for each graph widget independently from those of the dashboard.
The parameters can be specified at
GraphWidget,GaugeWidget, andSingleValueWidget.Closes #26945.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license