Skip to content

Commit

Permalink
feat: drop peer dependency on @aws-cdk/aws-redshift-alpha (#530)
Browse files Browse the repository at this point in the history
Fixes #212.

We don't _really_ use the L2 constructs directly, so we add our own
rough type checking logic to replace the one usage we did have but
retain the dev dependency for testing purposes.

For reference, [here's the current L2 `Cluster`
implementation](https://github.com/aws/aws-cdk/blob/v2.146.0/packages/@aws-cdk/aws-redshift-alpha/lib/cluster.ts#L432).

---

_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 25, 2024
1 parent 23caa55 commit 061d16f
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 17 deletions.
5 changes: 0 additions & 5 deletions .projen/deps.json

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

4 changes: 0 additions & 4 deletions .projenrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ _By submitting this pull request, I confirm that my contribution is made under t

// Experimental modules
["@aws-cdk/aws-redshift-alpha"].forEach((dep) => {
project.deps.addDependency(
`${dep}@^${CDK_VERSION}-alpha.0`,
DependencyType.PEER
);
project.deps.addDependency(
`${dep}@${CDK_VERSION}-alpha.0`,
DependencyType.DEVENV
Expand Down
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ In your `package.json`:
"cdk-monitoring-constructs": "^7.0.0",

// peer dependencies of cdk-monitoring-constructs
"@aws-cdk/aws-redshift-alpha": "^2.112.0-alpha.0",
"aws-cdk-lib": "^2.112.0",
"constructs": "^10.0.5"

Expand Down
17 changes: 13 additions & 4 deletions lib/facade/MonitoringAspect.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as redshift from "@aws-cdk/aws-redshift-alpha";
import { IAspect } from "aws-cdk-lib";
import * as apigw from "aws-cdk-lib/aws-apigateway";
import * as apigwv2 from "aws-cdk-lib/aws-apigatewayv2";
Expand All @@ -17,6 +16,7 @@ import * as kinesisfirehose from "aws-cdk-lib/aws-kinesisfirehose";
import * as lambda from "aws-cdk-lib/aws-lambda";
import * as opensearch from "aws-cdk-lib/aws-opensearchservice";
import * as rds from "aws-cdk-lib/aws-rds";
import { CfnCluster } from "aws-cdk-lib/aws-redshift";
import * as s3 from "aws-cdk-lib/aws-s3";
import * as secretsmanager from "aws-cdk-lib/aws-secretsmanager";
import * as sns from "aws-cdk-lib/aws-sns";
Expand Down Expand Up @@ -341,15 +341,24 @@ export class MonitoringAspect implements IAspect {

private monitorRedshift(node: IConstruct) {
const [isEnabled, props] = this.getMonitoringDetails(this.props.redshift);
if (isEnabled && node instanceof redshift.Cluster) {
if (isEnabled && this.isProbablyL2RedshiftCluster(node)) {
const cfnCluster = (node as any).cluster as CfnCluster;
this.monitoringFacade.monitorRedshiftCluster({
clusterIdentifier: node.clusterName,
alarmFriendlyName: node.node.path,
clusterIdentifier: cfnCluster.ref,
alarmFriendlyName: cfnCluster.node.path,
...props,
});
}
}

private isProbablyL2RedshiftCluster(node: IConstruct): boolean {
return (
(node as any).cluster instanceof CfnCluster &&
!!(node as any).clusterName &&
!!(node as any).node?.path
);
}

private monitorS3(node: IConstruct) {
const [isEnabled, props] = this.getMonitoringDetails(this.props.s3);
if (isEnabled && node instanceof s3.Bucket) {
Expand Down
1 change: 0 additions & 1 deletion package.json

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

4 changes: 2 additions & 2 deletions test/facade/__snapshots__/MonitoringAspect.test.ts.snap

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

0 comments on commit 061d16f

Please sign in to comment.