Skip to content

Commit

Permalink
fix(facade): avoid duplicated construct names for default dashboard f…
Browse files Browse the repository at this point in the history
…actories (#154)

If you have multiple facades in the same app using the default `dashboardFactory`, you'll end up with errors like:

```
[...]/node_modules/constructs/lib/construct.js:335
            throw new Error(`There is already a Construct with name '${childName}' in ${typeName}${name.length > 0 ? ' [' + name + ']' : ''}`);
            ^

Error: There is already a Construct with name 'Dashboards' in MyMonitoringFacade [MyApp-NestedTest]
[...]
```

---

_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 Jun 1, 2022
1 parent 8a9cde6 commit 7ab6393
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 6 deletions.
2 changes: 1 addition & 1 deletion lib/facade/MonitoringFacade.d.ts.map

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

4 changes: 3 additions & 1 deletion lib/facade/MonitoringFacade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,13 @@ export interface MonitoringFacadeProps {
* @default empty (no preferences)
*/
readonly metricFactoryDefaults?: MetricFactoryDefaults;

/**
* Defaults for alarm factory.
* @default actions enabled, facade logical ID used as default alarm name prefix
*/
readonly alarmFactoryDefaults?: AlarmFactoryDefaults;

/**
* Defaults for dashboard factory.
* @default `DefaultDashboardFactory`; facade logical ID used as default name
Expand Down Expand Up @@ -170,7 +172,7 @@ export class MonitoringFacade extends MonitoringScope {
scope: Construct,
defaultName: string
): IDashboardFactory {
return new DefaultDashboardFactory(scope, "Dashboards", {
return new DefaultDashboardFactory(scope, `${defaultName}-Dashboards`, {
dashboardNamePrefix: defaultName,
});
}
Expand Down
24 changes: 20 additions & 4 deletions test/facade/MonitoringFacade.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,31 @@ import { Template } from "aws-cdk-lib/assertions";
import { MonitoringFacade } from "../../lib";

describe("test of defaults", () => {
const stack = new Stack();
new MonitoringFacade(stack, "Test");
const result = Template.fromStack(stack);

test("only default dashboard gets created by default", () => {
const stack = new Stack();
new MonitoringFacade(stack, "Test");
const result = Template.fromStack(stack);

result.resourceCountIs("AWS::CloudWatch::Dashboard", 1);

result.hasResourceProperties("AWS::CloudWatch::Dashboard", {
DashboardName: "Test",
});
});

test("handles multiple facades", () => {
const stack = new Stack();
new MonitoringFacade(stack, "Test1");
new MonitoringFacade(stack, "Test2");
const result = Template.fromStack(stack);

result.resourceCountIs("AWS::CloudWatch::Dashboard", 2);

result.hasResourceProperties("AWS::CloudWatch::Dashboard", {
DashboardName: "Test1",
});
result.hasResourceProperties("AWS::CloudWatch::Dashboard", {
DashboardName: "Test2",
});
});
});

0 comments on commit 7ab6393

Please sign in to comment.