Skip to content

Commit

Permalink
feat(cloudwatch): use string instead of any for cloudwatch dimens…
Browse files Browse the repository at this point in the history
…ion values (#15097)

This is an alternative solution to  PR #14978 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
madeline-k authored Jun 16, 2021
1 parent 7d218c2 commit dc3cf13
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 10 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const hostedZone = new route53.HostedZone(this, 'MyHostedZone', { zoneName: "exa
const metric = new Metric({
namespace: 'AWS/Route53',
metricName: 'DNSQueries',
dimensions: {
dimensionsMap: {
HostedZoneId: hostedZone.hostedZoneId
}
})
Expand All @@ -53,7 +53,7 @@ you can instantiate a `Metric` object to represent it. For example:
const metric = new Metric({
namespace: 'MyNamespace',
metricName: 'MyMetric',
dimensions: {
dimensionsMap: {
ProcessingStep: 'Download'
}
});
Expand Down
28 changes: 22 additions & 6 deletions packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { Construct } from '@aws-cdk/core';

export type DimensionHash = {[dim: string]: any};

export type DimensionsMap = { [dim: string]: string };

/**
* Options shared by most methods accepting metric options
*/
Expand Down Expand Up @@ -43,9 +45,18 @@ export interface CommonMetricOptions {
* Dimensions of the metric
*
* @default - No dimensions.
*
* @deprecated Use 'dimensionsMap' instead.
*/
readonly dimensions?: DimensionHash;

/**
* Dimensions of the metric
*
* @default - No dimensions.
*/
readonly dimensionsMap?: DimensionsMap;

/**
* Unit used to filter the metric stream
*
Expand Down Expand Up @@ -216,10 +227,7 @@ export class Metric implements IMetric {
if (periodSec !== 1 && periodSec !== 5 && periodSec !== 10 && periodSec !== 30 && periodSec % 60 !== 0) {
throw new Error(`'period' must be 1, 5, 10, 30, or a multiple of 60 seconds, received ${periodSec}`);
}
if (props.dimensions) {
this.validateDimensions(props.dimensions);
}
this.dimensions = props.dimensions;
this.dimensions = this.validateDimensions(props.dimensionsMap ?? props.dimensions);
this.namespace = props.namespace;
this.metricName = props.metricName;
// Try parsing, this will throw if it's not a valid stat
Expand Down Expand Up @@ -249,12 +257,14 @@ export class Metric implements IMetric {
// For these we're not going to do deep equality, misses some opportunity for optimization
// but that's okay.
&& (props.dimensions === undefined)
&& (props.dimensionsMap === undefined)
&& (props.period === undefined || props.period.toSeconds() === this.period.toSeconds())) {
return this;
}

return new Metric({
dimensions: ifUndefined(props.dimensions, this.dimensions),
dimensionsMap: props.dimensionsMap,
namespace: this.namespace,
metricName: this.metricName,
period: ifUndefined(props.period, this.period),
Expand Down Expand Up @@ -398,7 +408,11 @@ export class Metric implements IMetric {
return list;
}

private validateDimensions(dims: DimensionHash): void {
private validateDimensions(dims?: DimensionHash): DimensionHash | undefined {
if (!dims) {
return dims;
}

var dimsArray = Object.keys(dims);
if (dimsArray?.length > 10) {
throw new Error(`The maximum number of dimensions is 10, received ${dimsArray.length}`);
Expand All @@ -416,6 +430,8 @@ export class Metric implements IMetric {
throw new Error(`Dimension value must be at least 1 and no more than 255 characters; received ${dims[key]}`);
};
});

return dims;
}
}

Expand Down Expand Up @@ -746,4 +762,4 @@ interface IModifiableMetric {

function isModifiableMetric(m: any): m is IModifiableMetric {
return typeof m === 'object' && m !== null && !!m.with;
}
}
111 changes: 109 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { expect, haveResource } from '@aws-cdk/assert-internal';
import { expect, haveResource, haveResourceLike } from '@aws-cdk/assert-internal';
import * as iam from '@aws-cdk/aws-iam';
import * as cdk from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { Metric } from '../lib';
import { Alarm, Metric } from '../lib';

export = {
'metric grant'(test: Test) {
Expand Down Expand Up @@ -100,6 +100,24 @@ export = {
test.done();
},

'cannot use long dimension values in dimensionsMap'(test: Test) {
const arr = new Array(256);
const invalidDimensionValue = arr.fill('A', 0).join('');

test.throws(() => {
new Metric({
namespace: 'Test',
metricName: 'ACount',
period: cdk.Duration.minutes(10),
dimensionsMap: {
DimensionWithLongValue: invalidDimensionValue,
},
});
}, `Dimension value must be at least 1 and no more than 255 characters; received ${invalidDimensionValue}`);

test.done();
},

'throws error when there are more than 10 dimensions'(test: Test) {
test.throws(() => {
new Metric({
Expand All @@ -124,4 +142,93 @@ export = {

test.done();
},

'throws error when there are more than 10 dimensions in dimensionsMap'(test: Test) {
test.throws(() => {
new Metric({
namespace: 'Test',
metricName: 'ACount',
period: cdk.Duration.minutes(10),
dimensionsMap: {
dimensionA: 'value1',
dimensionB: 'value2',
dimensionC: 'value3',
dimensionD: 'value4',
dimensionE: 'value5',
dimensionF: 'value6',
dimensionG: 'value7',
dimensionH: 'value8',
dimensionI: 'value9',
dimensionJ: 'value10',
dimensionK: 'value11',
},
} );
}, /The maximum number of dimensions is 10, received 11/);

test.done();
},

'can create metric with dimensionsMap property'(test: Test) {
const stack = new cdk.Stack();
const metric = new Metric({
namespace: 'Test',
metricName: 'Metric',
dimensionsMap: {
dimensionA: 'value1',
dimensionB: 'value2',
},
});

new Alarm(stack, 'Alarm', {
metric: metric,
threshold: 10,
evaluationPeriods: 1,
});

test.deepEqual(metric.dimensions, {
dimensionA: 'value1',
dimensionB: 'value2',
});
expect(stack).to(haveResourceLike('AWS::CloudWatch::Alarm', {
Namespace: 'Test',
MetricName: 'Metric',
Dimensions: [
{
Name: 'dimensionA',
Value: 'value1',
},
{
Name: 'dimensionB',
Value: 'value2',
},
],
Threshold: 10,
EvaluationPeriods: 1,
}));

test.done();
},

'"with" with a different dimensions property'(test: Test) {
const dims = {
dimensionA: 'value1',
};

const metric = new Metric({
namespace: 'Test',
metricName: 'Metric',
period: cdk.Duration.minutes(10),
dimensionsMap: dims,
});

const newDims = {
dimensionB: 'value2',
};

test.deepEqual(metric.with({
dimensionsMap: newDims,
}).dimensions, newDims);

test.done();
},
};

0 comments on commit dc3cf13

Please sign in to comment.