cdklabs / cdk-monitoring-constructs

Easy-to-use CDK constructs for monitoring your AWS infrastructure
https://constructs.dev/packages/cdk-monitoring-constructs
Apache License 2.0
444 stars 56 forks source link

Dynamic Dashboard Default Range Incorrect #430

Closed ryxli closed 9 months ago

ryxli commented 9 months ago

Version

5.0.0

Steps and/or minimal code example to reproduce

Create any MonitoringFacade with dashboardConfigs and DynamicSegments:

new MonitoringFacade(this, `MonitoringFacade`, {
            dashboardNamePrefix: dashboardName,
            dashboardConfigs: [
                { name: DashboardTypes.Type1, range: Duration.days(7). },
            ],
            ...

Expected behavior

Duration should be converted into a properly formatted string, and the default range of the dashboard should be set correctly according to CloudWatch api spec in the CloudFormation template:

start

    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. In this case, the value of start must begin with -PT if you specify a time range in minutes or hours, and must begin with -P if you specify a time range in days, weeks, or months. You can then use M, H, D, W and M as abbreviations for minutes, hours, days, weeks and months. For example, -PT5M shows the last 5 minutes, -PT8H shows the last 8 hours, and -P3M shows the last three months.

    You can also use start along with an end field, to specify an absolute time range. When specifying an absolute time range, use the ISO 8601 format. For example, 2018-12-17T06:00:00.000Z.

    If you omit start, the dashboard shows the default time range when it loads.

    Type: String

    Required: No

Example:

   "Type": "AWS::CloudWatch::Dashboard",
   "Properties": {
    "DashboardBody": {
     "Fn::Join": [
      "",
      [
       "{\"start\":\"-P7D\",\"widgets\":[{\"type\":\"text\",\"width\":......, 
       {
        "Ref": "AWS::Region"
       },

Actual behavior

In the constructor of DynamicDashboardFactory, this is used in the following way:

      const start: string =
        "-" + (dashboardConfig.range ?? Duration.hours(8).toIsoString());

      const dashboard = this.createDashboard(
        renderingPreference,
        dashboardConfig.name,
        {
          dashboardName: `${props.dashboardNamePrefix}-${dashboardConfig.name}`,
          start,
          periodOverride:
            dashboardConfig.periodOverride ?? PeriodOverride.INHERIT,
        }
      );

dashboardConfig.range is restricted to type Duration, and this translates into the cloudformation template, resulting in no default start and the behavior is that it shows 3 hours.

Example:

{
   "Type": "AWS::CloudWatch::Dashboard",
   "Properties": {
    "DashboardBody": {
     "Fn::Join": [
      "",
      [
       "{\"start\":\"-Duration.days(7)\",\"periodOverride\":\"inherit\",\"widgets\"......,
       {
        "Ref": "AWS::Region"
       },

Other details

Maybe let the interface DynamicDashboardConfiguration range property be able to be string? Or fix from

      const start: string =
        "-" + (dashboardConfig.range ?? Duration.hours(8).toIsoString());

to

      const start: string =
        "-" + (dashboardConfig.range.toIsoString() ?? Duration.hours(8).toIsoString());
echeung-amzn commented 9 months ago

436 has the basic fix, although it could potentially accept an absolute date as a separate feature request.