`add_to_summary_dashboard` does not work on `monitor_custom`

[Glyphack]

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

[voho]

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?

[Glyphack]

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?

[voho]

Agree, rename would be a good move. We can deprecate the old property. It is really old :).