Repository: cdklabs/cdk-monitoring-constructs (language: python)

Issue:
`add_to_summary_dashboard` does not work on `monitor_custom`
### Version

1.22.4

### Steps and/or minimal code example to reproduce

I'm using the python version:
1. Change dashboard factory to also create the summary
```
dashboard_factory=monitoring.DefaultDashboardFactory(
                self,
                "DashboardFactory",
                dashboard_name_prefix="streamProcessors",
                create_dashboard=True,
                detail_dashboard_range=aws_cdk.Duration.hours(4),
            ),
```
2. monitor a custom resource and supply `add_to_summary_dashboard=True`
```
monitoring_obj.monitor_custom(
            alarm_friendly_name="Processor performance",
            add_to_summary_dashboard=True,
            metric_groups=[]
)
```

### Expected behavior

Expected to see the custom monitoring widget in the summary like other resources (dynamo DB in this case) I set to be shown in the summary.

### Actual behavior

`monitor_custom` contents are not shown in the summary.

### Other details

_No response_


Discussion / hints:
Hi! This is a bit confusing behaviour, but there is this "important" flag, which is related to every metric group. Only the ones with "important: true" get added to summary dashboard.

I believe we could change the requirements in this issue to get rid of it, to rename it (from `important` to `addToSummaryDashboard`), or something else, e.g. checking if there is at least one important widget in the custom metric construct, guiding the user to use the flag if they want anything shown.

We could also remove it, but that would disallow users to hide only a certain widgets.

Do you have any other thoughts or ideas?
Hey, thanks for the clarification. So I did not read the docs carefully for this, and this was because I was searching for the `summary` word. I think it makes sense to have control over each metric group, but in this case, you have to both set the `addToSummaryDashboard` and the `important` flag.

There I two ways IMO:
- Having the current behavior but renaming the `important` to `addToSummaryDashboard`(or something with the summary word in it). After I read the first few pages of the docs, I assumed that anywhere I wanted to move something to the summary dashboard, I should look for the summary argument, and the current name is a bit inconsistent.
- Removing the `addToSummaryDashboard` from monitor_custom args and only leaving it in the MetricGroup. In this way, only selected metric groups from the custom monitoring component will show up in summary.

Examples:

First way:

metric1 would show up in summary ✅ 
```
monitoring_obj.monitor_custom(
            alarm_friendly_name="Processor performance",
            add_to_summary_dashboard=True,
            metric_groups=[
			monitoring.CustomMetricGroup(
					title="metric1",
					# other args
                    add_to_summary_dashboard=True
                ),
			]
)
```
 
metric1 would not show up in summary ❌ 
```
monitoring_obj.monitor_custom(
            alarm_friendly_name="Processor performance",
            add_to_summary_dashboard=True,
            metric_groups=[
			monitoring.CustomMetricGroup(
					title="metric1",
					# other args
                ),
			]
)
```

```
monitoring_obj.monitor_custom(
            alarm_friendly_name="Processor performance",
            metric_groups=[
			monitoring.CustomMetricGroup(
					title="metric1",
					# other args
                    add_to_summary_dashboard=True
                ),
			]
)
```

Second way:

metric1 would show up in summary ✅ 
```
monitoring_obj.monitor_custom(
            alarm_friendly_name="Processor performance",
            metric_groups=[
			monitoring.CustomMetricGroup(
					title="metric1",
					# other args
                    add_to_summary_dashboard=True
                ),
			]
)
```

metric1 would show up in summary ❌  
```
monitoring_obj.monitor_custom(
            alarm_friendly_name="Processor performance",
            metric_groups=[
			monitoring.CustomMetricGroup(
					title="metric1",
					# other args
                ),
			]
)
```

What do you think?
Agree, rename would be a good move. We can deprecate the old property. It is really old :).

Fix patch:
diff --git a/API.md b/API.md
index 7de3b9e5..c5cbec35 100644
--- a/API.md
+++ b/API.md
@@ -10223,13 +10223,14 @@ const customMetricGroup: CustomMetricGroup = { ... }
 | --- | --- | --- |
 | <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.metrics">metrics</a></code> | <code>aws-cdk-lib.aws_cloudwatch.Metric \| aws-cdk-lib.aws_cloudwatch.MathExpression \| <a href="#cdk-monitoring-constructs.CustomMetricWithAlarm">CustomMetricWithAlarm</a> \| <a href="#cdk-monitoring-constructs.CustomMetricWithAnomalyDetection">CustomMetricWithAnomalyDetection</a> \| <a href="#cdk-monitoring-constructs.CustomMetricSearch">CustomMetricSearch</a>[]</code> | list of metrics in the group (can be defined in different ways, see the type documentation). |
 | <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.title">title</a></code> | <code>string</code> | title of the whole group. |
+| <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.addToSummaryDashboard">addToSummaryDashboard</a></code> | <code>boolean</code> | Flag indicating this metric group should be included in the summary as well. |
 | <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.graphWidgetAxis">graphWidgetAxis</a></code> | <code>aws-cdk-lib.aws_cloudwatch.YAxisProps</code> | optional axis. |
 | <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.graphWidgetLegend">graphWidgetLegend</a></code> | <code>aws-cdk-lib.aws_cloudwatch.LegendPosition</code> | graph widget legend. |
 | <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.graphWidgetRightAxis">graphWidgetRightAxis</a></code> | <code>aws-cdk-lib.aws_cloudwatch.YAxisProps</code> | optional right axis. |
 | <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.graphWidgetType">graphWidgetType</a></code> | <code><a href="#cdk-monitoring-constructs.GraphWidgetType">GraphWidgetType</a></code> | type of the widget. |
 | <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.horizontalAnnotations">horizontalAnnotations</a></code> | <code>aws-cdk-lib.aws_cloudwatch.HorizontalAnnotation[]</code> | optional custom horizontal annotations which will be displayed over the metrics on the left axis (if there are any alarms, any existing annotations will be merged together). |
 | <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.horizontalRightAnnotations">horizontalRightAnnotations</a></code> | <code>aws-cdk-lib.aws_cloudwatch.HorizontalAnnotation[]</code> | optional custom horizontal annotations which will be displayed over the metrics on the right axis (if there are any alarms, any existing annotations will be merged together). |
-| <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.important">important</a></code> | <code>boolean</code> | Flag indicating, whether this is an important metric group that should be included in the summary as well. |
+| <code><a href="#cdk-monitoring-constructs.CustomMetricGroup.property.important">important</a></code> | <code>boolean</code> | *No description.* |
 
 ---
 
@@ -10257,6 +10258,19 @@ title of the whole group.
 
 ---
 
+##### `addToSummaryDashboard`<sup>Optional</sup> <a name="addToSummaryDashboard" id="cdk-monitoring-constructs.CustomMetricGroup.property.addToSummaryDashboard"></a>
+
+```typescript
+public readonly addToSummaryDashboard: boolean;
+```
+
+- *Type:* boolean
+- *Default:* false
+
+Flag indicating this metric group should be included in the summary as well.
+
+---
+
 ##### `graphWidgetAxis`<sup>Optional</sup> <a name="graphWidgetAxis" id="cdk-monitoring-constructs.CustomMetricGroup.property.graphWidgetAxis"></a>
 
 ```typescript
@@ -10333,7 +10347,10 @@ optional custom horizontal annotations which will be displayed over the metrics
 
 ---
 
-##### `important`<sup>Optional</sup> <a name="important" id="cdk-monitoring-constructs.CustomMetricGroup.property.important"></a>
+##### ~~`important`~~<sup>Optional</sup> <a name="important" id="cdk-monitoring-constructs.CustomMetricGroup.property.important"></a>
+
+- *Deprecated:* use addToSummaryDashboard. addToSummaryDashboard will take precedence over important.
+Flag indicating, whether this is an important metric group that should be included in the summary as well.
 
 ```typescript
 public readonly important: boolean;
@@ -10342,8 +10359,6 @@ public readonly important: boolean;
 - *Type:* boolean
 - *Default:* false
 
-Flag indicating, whether this is an important metric group that should be included in the summary as well.
-
 ---
 
 ### CustomMetricGroupWithAnnotations <a name="CustomMetricGroupWithAnnotations" id="cdk-monitoring-constructs.CustomMetricGroupWithAnnotations"></a>
diff --git a/lib/monitoring/custom/CustomMonitoring.ts b/lib/monitoring/custom/CustomMonitoring.ts
index ce2cd344..cb325804 100644
--- a/lib/monitoring/custom/CustomMonitoring.ts
+++ b/lib/monitoring/custom/CustomMonitoring.ts
@@ -170,10 +170,16 @@ export interface CustomMetricGroup {
    */
   readonly graphWidgetLegend?: LegendPosition;
   /**
+   * @deprecated use addToSummaryDashboard. addToSummaryDashboard will take precedence over important.
    * Flag indicating, whether this is an important metric group that should be included in the summary as well.
    * @default false
    */
   readonly important?: boolean;
+  /**
+   * Flag indicating this metric group should be included in the summary as well.
+   * @default false
+   */
+  readonly addToSummaryDashboard?: boolean;
   /**
    * list of metrics in the group (can be defined in different ways, see the type documentation)
    */
@@ -304,7 +310,10 @@ export class CustomMonitoring extends Monitoring {
   private getAllWidgets(summary: boolean): IWidget[] {
     const filteredMetricGroups = summary
       ? this.metricGroups.filter(
-          (group) => group.metricGroup.important ?? false
+          (group) =>
+            group.metricGroup.addToSummaryDashboard ??
+            group.metricGroup.important ??
+            false
         )
       : this.metricGroups;
 


Test patch:
diff --git a/test/monitoring/custom/CustomMonitoring.test.ts b/test/monitoring/custom/CustomMonitoring.test.ts
index 420915d3..72e15db1 100644
--- a/test/monitoring/custom/CustomMonitoring.test.ts
+++ b/test/monitoring/custom/CustomMonitoring.test.ts
@@ -37,7 +37,7 @@ test("snapshot test", () => {
     metricGroups: [
       {
         title: "DummyGroup1",
-        important: true,
+        addToSummaryDashboard: true,
         metrics: [
           // regular metric
           new Metric({ metricName: "DummyMetric1", namespace, dimensionsMap }),
@@ -175,6 +175,34 @@ test("snapshot test", () => {
   expect(Template.fromStack(stack)).toMatchSnapshot();
 });
 
+test("addToSummaryDashboard attribute takes precedence over important in metric group", () => {
+  const stack = new Stack();
+  const scope = new TestMonitoringScope(stack, "Scope");
+
+  const monitoring = new CustomMonitoring(scope, {
+    alarmFriendlyName: "DummyAlarmName",
+    humanReadableName: "DummyName",
+    description: "Monitoring widget that shows up in the summary dashboard",
+    descriptionWidgetHeight: 2,
+    height: 100,
+    addToSummaryDashboard: true,
+    metricGroups: [
+      {
+        title: "DummyGroup1",
+        important: false,
+        addToSummaryDashboard: true,
+        metrics: [
+          // regular metric
+          new Metric({ metricName: "DummyMetric1", namespace, dimensionsMap }),
+        ],
+      },
+    ],
+  });
+
+  addMonitoringDashboardsToStack(stack, monitoring);
+  expect(Template.fromStack(stack)).toMatchSnapshot();
+});
+
 test("anomaly detection", () => {
   const stack = new Stack();
   const metricFactory = new MetricFactory({
@@ -403,7 +431,7 @@ test("throws error if attempting to add alarm on a search query", () => {
         metricGroups: [
           {
             title: "DummyGroup1",
-            important: true,
+            addToSummaryDashboard: true,
             metrics: [
               {
                 searchQuery: "DummyMetric4-",
@@ -442,7 +470,7 @@ test("throws error if attempting to add both a regular alarm and an anomoly dete
         metricGroups: [
           {
             title: "DummyGroup1",
-            important: true,
+            addToSummaryDashboard: true,
             metrics: [
               {
                 metric: new Metric({
diff --git a/test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap b/test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap
index 3a0a962b..ea6e5259 100644
--- a/test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap
+++ b/test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap
@@ -1,5 +1,86 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`addToSummaryDashboard attribute takes precedence over important in metric group 1`] = `
+Object {
+  "Parameters": Object {
+    "BootstrapVersion": Object {
+      "Default": "/cdk-bootstrap/hnb659fds/version",
+      "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]",
+      "Type": "AWS::SSM::Parameter::Value<String>",
+    },
+  },
+  "Resources": Object {
+    "Alarm7103F465": Object {
+      "Properties": Object {
+        "DashboardBody": "{\\"widgets\\":[]}",
+      },
+      "Type": "AWS::CloudWatch::Dashboard",
+    },
+    "Resource": Object {
+      "Properties": Object {
+        "DashboardBody": Object {
+          "Fn::Join": Array [
+            "",
+            Array [
+              "{\\"widgets\\":[{\\"type\\":\\"text\\",\\"width\\":24,\\"height\\":1,\\"x\\":0,\\"y\\":0,\\"properties\\":{\\"markdown\\":\\"### DummyName\\"}},{\\"type\\":\\"text\\",\\"width\\":24,\\"height\\":2,\\"x\\":0,\\"y\\":1,\\"properties\\":{\\"markdown\\":\\"Monitoring widget that shows up in the summary dashboard\\"}},{\\"type\\":\\"metric\\",\\"width\\":24,\\"height\\":100,\\"x\\":0,\\"y\\":3,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"DummyGroup1\\",\\"region\\":\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"metrics\\":[[\\"DummyCustomNamespace\\",\\"DummyMetric1\\",\\"CustomDimension\\",\\"CustomDimensionValue\\"]],\\"yAxis\\":{}}}]}",
+            ],
+          ],
+        },
+      },
+      "Type": "AWS::CloudWatch::Dashboard",
+    },
+    "Summary68521F81": Object {
+      "Properties": Object {
+        "DashboardBody": Object {
+          "Fn::Join": Array [
+            "",
+            Array [
+              "{\\"widgets\\":[{\\"type\\":\\"text\\",\\"width\\":24,\\"height\\":1,\\"x\\":0,\\"y\\":0,\\"properties\\":{\\"markdown\\":\\"### DummyName\\"}},{\\"type\\":\\"metric\\",\\"width\\":24,\\"height\\":100,\\"x\\":0,\\"y\\":1,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"DummyGroup1\\",\\"region\\":\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"metrics\\":[[\\"DummyCustomNamespace\\",\\"DummyMetric1\\",\\"CustomDimension\\",\\"CustomDimensionValue\\"]],\\"yAxis\\":{}}}]}",
+            ],
+          ],
+        },
+      },
+      "Type": "AWS::CloudWatch::Dashboard",
+    },
+  },
+  "Rules": Object {
+    "CheckBootstrapVersion": Object {
+      "Assertions": Array [
+        Object {
+          "Assert": Object {
+            "Fn::Not": Array [
+              Object {
+                "Fn::Contains": Array [
+                  Array [
+                    "1",
+                    "2",
+                    "3",
+                    "4",
+                    "5",
+                  ],
+                  Object {
+                    "Ref": "BootstrapVersion",
+                  },
+                ],
+              },
+            ],
+          },
+          "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI.",
+        },
+      ],
+    },
+  },
+}
+`;
+
 exports[`anomaly detection 1`] = `
 Object {
   "Parameters": Object {