Skip to content

Commit

Permalink
feat(custom): use addToSummaryDashboard from CustomMonitoringProps if…
Browse files Browse the repository at this point in the history
… not specified in metric groups (#278)

It's confusing that the top-level `addToSummaryDashboard` didn't do anything. This is now the lowest-precedence way to add things to the summary dashboard. This still defaults to `false` to maintain existing behaviour.

---

_By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license_
  • Loading branch information
echeung-amzn authored Dec 2, 2022
1 parent 8d01b3c commit 8f413ed
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 8 deletions.
17 changes: 14 additions & 3 deletions API.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions lib/monitoring/custom/CustomMonitoring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,12 @@ 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
* @see addToSummaryDashboard
*/
readonly important?: boolean;
/**
* Flag indicating this metric group should be included in the summary as well.
* @default false
* @default - addToSummaryDashboard from CustomMonitoringProps, defaulting to false
*/
readonly addToSummaryDashboard?: boolean;
/**
Expand Down Expand Up @@ -238,6 +237,7 @@ export class CustomMonitoring extends Monitoring {
readonly description?: string;
readonly descriptionWidgetHeight?: number;
readonly height?: number;
readonly addToSummaryDashboard: boolean;
readonly customAlarmFactory: CustomAlarmFactory;
readonly anomalyDetectingAlarmFactory: AnomalyDetectingAlarmFactory;
readonly metricGroups: CustomMetricGroupWithAnnotations[];
Expand All @@ -251,6 +251,7 @@ export class CustomMonitoring extends Monitoring {
this.description = props.description;
this.descriptionWidgetHeight = props.descriptionWidgetHeight;
this.height = props.height;
this.addToSummaryDashboard = props.addToSummaryDashboard ?? false;

const alarmFactory = this.createAlarmFactory(
namingStrategy.resolveAlarmFriendlyName()
Expand Down Expand Up @@ -313,7 +314,7 @@ export class CustomMonitoring extends Monitoring {
(group) =>
group.metricGroup.addToSummaryDashboard ??
group.metricGroup.important ??
false
this.addToSummaryDashboard
)
: this.metricGroups;

Expand Down
28 changes: 27 additions & 1 deletion test/monitoring/custom/CustomMonitoring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,34 @@ test("addToSummaryDashboard attribute takes precedence over important in metric
metricGroups: [
{
title: "DummyGroup1",
important: false,
addToSummaryDashboard: true,
important: false,
metrics: [
// regular metric
new Metric({ metricName: "DummyMetric1", namespace, dimensionsMap }),
],
},
],
});

addMonitoringDashboardsToStack(stack, monitoring);
expect(Template.fromStack(stack)).toMatchSnapshot();
});

test("addToSummaryDashboard attribute takes value from CustomMonitoringProps if not specified 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",
metrics: [
// regular metric
new Metric({ metricName: "DummyMetric1", namespace, dimensionsMap }),
Expand Down
81 changes: 81 additions & 0 deletions test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 8f413ed

Please sign in to comment.