Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cloudwatch): support for metric math #5582

Merged
merged 48 commits into from
Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
4f102bf
Dummy change
AhmedSedek Dec 30, 2019
ff022bd
Created base interfaces
AhmedSedek Dec 30, 2019
d2a1438
Consume new toMetricConfig() API for dashboard widgets
rix0rrr Dec 30, 2019
2b08eec
Added MathExpression
AhmedSedek Dec 30, 2019
1eae9f2
Consume new API for rendering Alarms
rix0rrr Dec 30, 2019
c357ebc
Merge branch 'cloudwatch-expressions' of github.com:AhmedSedek/aws-cd…
rix0rrr Dec 30, 2019
0844513
Make period a Duration, add a dispatch helper function, fix compilation
rix0rrr Dec 30, 2019
ea6d0b5
More properly detect empty options object, tests are still failing
rix0rrr Dec 30, 2019
8eeac0d
Created BaseMetric
AhmedSedek Dec 30, 2019
3ce70e8
Merge branch 'master' into cloudwatch-expressions
AhmedSedek Dec 30, 2019
271130a
Fix tests
rix0rrr Dec 31, 2019
ce6b58b
Added docstrings
AhmedSedek Dec 31, 2019
6379353
Updated autoscaling libs to use new API
AhmedSedek Dec 31, 2019
ad70862
Merge branch 'cloudwatch-expressions' of https://github.com/AhmedSede…
AhmedSedek Dec 31, 2019
b77aefe
Updated README with MathExpression
AhmedSedek Dec 31, 2019
02194cb
Adding tests, still breaking
rix0rrr Dec 31, 2019
067af60
Merge branch 'cloudwatch-expressions' of github.com:AhmedSedek/aws-cd…
rix0rrr Dec 31, 2019
6a18672
Make more tests pass
rix0rrr Dec 31, 2019
ca15a28
Fixed integ test
AhmedSedek Dec 31, 2019
675bf85
Properly add unit test file
rix0rrr Dec 31, 2019
5b93536
Merge branch 'cloudwatch-expressions' of github.com:AhmedSedek/aws-cd…
rix0rrr Dec 31, 2019
f0aaabd
Add some final unit tests
rix0rrr Dec 31, 2019
f4a7019
Update expectation to swap dimensions around
rix0rrr Dec 31, 2019
03eb4a0
Review comments
rix0rrr Dec 31, 2019
de6d63c
Change generated id
rix0rrr Dec 31, 2019
8501005
Adjust periods on metrics used in expression so they're all the same
rix0rrr Dec 31, 2019
0466f1e
Restrict the properties visible to MathExpressions
rix0rrr Dec 31, 2019
218d586
Rename 'expressionMetrics' => 'usingMetrics'
rix0rrr Dec 31, 2019
10a5970
Also rename expressionMetrics => usingMetrics in return object
rix0rrr Dec 31, 2019
7b6936f
UPdate docstrings
rix0rrr Dec 31, 2019
c3aad11
Make changePeriod recursive, fix cache key bug for prototyped objects
rix0rrr Jan 2, 2020
bf6999d
Merge branch 'master' into cloudwatch-expressions
rix0rrr Jan 2, 2020
b3702cc
Added more tests
AhmedSedek Jan 2, 2020
680b1ba
Merge remote-tracking branch 'upstream/master' into cloudwatch-expres…
AhmedSedek Jan 2, 2020
1a2d996
Review comments
rix0rrr Jan 2, 2020
15df3a8
Merge branch 'cloudwatch-expressions' of github.com:AhmedSedek/aws-cd…
rix0rrr Jan 2, 2020
4de3951
Small fixes remaining
rix0rrr Jan 2, 2020
9763e33
Update README
rix0rrr Jan 2, 2020
728f53f
Revert rash decision to drop support for `unit` completely, now just …
rix0rrr Jan 2, 2020
8a2a67d
Update packages/@aws-cdk/aws-applicationautoscaling/lib/step-scaling-…
rix0rrr Jan 2, 2020
58f63b3
Update packages/@aws-cdk/aws-applicationautoscaling/lib/target-tracki…
rix0rrr Jan 2, 2020
70bd8bb
Update packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
rix0rrr Jan 2, 2020
6c0ee40
Revert quotes change
rix0rrr Jan 2, 2020
bc86893
Review comments
rix0rrr Jan 3, 2020
41c00c0
Fixed test
AhmedSedek Jan 3, 2020
38f5d38
Merge remote-tracking branch 'upstream/master' into cloudwatch-expres…
AhmedSedek Jan 3, 2020
a35662f
attachTo() is not in this branch yet
rix0rrr Jan 3, 2020
1339ab2
Merge branch 'cloudwatch-expressions' of github.com:AhmedSedek/aws-cd…
rix0rrr Jan 3, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 52 additions & 3 deletions packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { CfnAlarm } from './cloudwatch.generated';
import { HorizontalAnnotation } from './graph';
import { CreateAlarmOptions } from './metric';
import { IMetric } from './metric-types';
import { MetricSet } from './metric-util';
import { parseStatistic } from './util.statistic';

export interface IAlarm extends IResource {
Expand Down Expand Up @@ -121,8 +122,6 @@ export class Alarm extends Resource implements IAlarm {

const comparisonOperator = props.comparisonOperator || ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD;

const config = props.metric.toAlarmConfig();

const alarm = new CfnAlarm(this, 'Resource', {
// Meta
alarmDescription: props.alarmDescription,
Expand All @@ -143,7 +142,7 @@ export class Alarm extends Resource implements IAlarm {
okActions: Lazy.listValue({ produce: () => this.okActionArns }),

// Metric
...dropUndef(config),
...renderAlarmMetric(props.metric),
...dropUndef({
// Alarm overrides
period: props.period && props.period.toSeconds(),
Expand Down Expand Up @@ -228,6 +227,56 @@ export class Alarm extends Resource implements IAlarm {
}
}

function renderAlarmMetric(metric: IMetric) {
const config = metric.toMetricConfig();

if (config.metricStat) {
const st = config.metricStat;
return dropUndef({
dimensions: st.dimensions,
namespace: st.namespace,
metricName: st.metricName,
period: st.period,
statistic: renderIfSimpleStatistic(st.statistic),
extendedStatistic: renderIfExtendedStatistic(st.statistic),
});
} else if (config.mathExpression) {
// Expand the math expression metric into a set
const mset = new MetricSet<boolean>();
mset.addPrimary(true, metric);

let mid = 0;
function uniqueMetricId() {
return `mid${++mid}`;
}

return {
metrics: mset.entries.map(entry => {
const conf = entry.metric.toMetricConfig();

return dropUndef({
expression: conf.mathExpression?.expression,
metricStat: conf.metricStat ? {
metric: {
metricName: conf.metricStat.metricName,
namespace: conf.metricStat.namespace,
dimensions: conf.metricStat.dimensions,
},
period: conf.metricStat.period,
stat: conf.metricStat.statistic,
// unit: not used, on purpose. Does not do what we need it to do.
} : undefined,
id: entry.id || uniqueMetricId(),
label: conf.renderingProperties?.label,
returnData: entry.tag ? undefined : false, // Tag stores "primary" attribute, default is "true"
} as CfnAlarm.MetricDataQueryProperty);
})
};
} else {
throw new Error(`Metric object must have either 'metricStat' or 'mathExpression'`);
}
}

/**
* Return a human readable string for this period
*
Expand Down
33 changes: 4 additions & 29 deletions packages/@aws-cdk/aws-cloudwatch/lib/graph.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as cdk from '@aws-cdk/core';
import { IAlarm } from "./alarm";
import { IMetric } from "./metric-types";
import { allMetricsGraphJson } from './metric-util';
import { ConcreteWidget } from "./widget";

/**
Expand Down Expand Up @@ -169,8 +170,7 @@ export class GraphWidget extends ConcreteWidget {
public toJson(): any[] {
const horizontalAnnoations = (this.props.leftAnnotations || []).map(mapAnnotation('left')).concat(
(this.props.rightAnnotations || []).map(mapAnnotation('right')));
const metrics = (this.props.left || []).map(m => metricJson(m, 'left')).concat(
(this.props.right || []).map(m => metricJson(m, 'right')));
const metrics = allMetricsGraphJson(this.props.left || [], this.props.right || []);
RomainMuller marked this conversation as resolved.
Show resolved Hide resolved
return [{
type: 'metric',
width: this.width,
Expand Down Expand Up @@ -232,7 +232,7 @@ export class SingleValueWidget extends ConcreteWidget {
view: 'singleValue',
title: this.props.title,
region: this.props.region || cdk.Aws.REGION,
metrics: this.props.metrics.map(m => metricJson(m, 'left')),
metrics: allMetricsGraphJson(this.props.metrics, []),
setPeriodToTimeRange: this.props.setPeriodToTimeRange
}
}];
Expand Down Expand Up @@ -298,29 +298,4 @@ function mapAnnotation(yAxis: string): ((x: HorizontalAnnotation) => any) {
return (a: HorizontalAnnotation) => {
return { ...a, yAxis };
};
}

/**
* Return the JSON structure which represents this metric in a graph
*
* This will be called by GraphWidget, no need for clients to call this.
*/
function metricJson(metric: IMetric, yAxis: string): any[] {
const config = metric.toGraphConfig();

// Namespace and metric Name
const ret: any[] = [
config.namespace,
config.metricName,
];

// Dimensions
for (const dim of (config.dimensions || [])) {
ret.push(dim.name, dim.value);
}

// Options
ret.push({ yAxis, ...config.renderingProperties });

return ret;
}
}
162 changes: 129 additions & 33 deletions packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,30 @@

/**
* Interface for metrics
*/
export interface IMetric {
/**
* Inspect the details of the metric object
*/
toMetricConfig(): MetricConfig;

/**
* Turn this metric object into an alarm configuration
*
* @deprecated Use `toMetricsConfig()` instead.
*/
toAlarmConfig(): MetricAlarmConfig;

/**
* Turn this metric object into a graph configuration
*
* @deprecated Use `toMetricsConfig()` instead.
*/
toGraphConfig(): MetricGraphConfig;
}

/**
* Metric dimension
*
*/
export interface Dimension {
/**
Expand All @@ -33,48 +42,131 @@ export interface Dimension {
* Statistic to use over the aggregation period
*/
export enum Statistic {
SAMPLE_COUNT = 'SampleCount',
AVERAGE = 'Average',
SUM = 'Sum',
MINIMUM = 'Minimum',
MAXIMUM = 'Maximum',
SAMPLE_COUNT = "SampleCount",
AVERAGE = "Average",
SUM = "Sum",
MINIMUM = "Minimum",
MAXIMUM = "Maximum"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert this change.

}

/**
* Unit for metric
*/
export enum Unit {
SECONDS = 'Seconds',
MICROSECONDS = 'Microseconds',
MILLISECONDS = 'Milliseconds',
BYTES = 'Bytes',
KILOBYTES = 'Kilobytes',
MEGABYTES = 'Megabytes',
GIGABYTES = 'Gigabytes',
TERABYTES = 'Terabytes',
BITS = 'Bits',
KILOBITS = 'Kilobits',
MEGABITS = 'Megabits',
GIGABITS = 'Gigabits',
TERABITS = 'Terabits',
PERCENT = 'Percent',
COUNT = 'Count',
BYTES_PER_SECOND = 'Bytes/Second',
KILOBYTES_PER_SECOND = 'Kilobytes/Second',
MEGABYTES_PER_SECOND = 'Megabytes/Second',
GIGABYTES_PER_SECOND = 'Gigabytes/Second',
TERABYTES_PER_SECOND = 'Terabytes/Second',
BITS_PER_SECOND = 'Bits/Second',
KILOBITS_PER_SECOND = 'Kilobits/Second',
MEGABITS_PER_SECOND = 'Megabits/Second',
GIGABITS_PER_SECOND = 'Gigabits/Second',
TERABITS_PER_SECOND = 'Terabits/Second',
COUNT_PER_SECOND = 'Count/Second',
NONE = 'None'
SECONDS = "Seconds",
MICROSECONDS = "Microseconds",
MILLISECONDS = "Milliseconds",
BYTES = "Bytes",
KILOBYTES = "Kilobytes",
MEGABYTES = "Megabytes",
GIGABYTES = "Gigabytes",
TERABYTES = "Terabytes",
BITS = "Bits",
KILOBITS = "Kilobits",
MEGABITS = "Megabits",
GIGABITS = "Gigabits",
TERABITS = "Terabits",
PERCENT = "Percent",
COUNT = "Count",
BYTES_PER_SECOND = "Bytes/Second",
KILOBYTES_PER_SECOND = "Kilobytes/Second",
MEGABYTES_PER_SECOND = "Megabytes/Second",
GIGABYTES_PER_SECOND = "Gigabytes/Second",
TERABYTES_PER_SECOND = "Terabytes/Second",
BITS_PER_SECOND = "Bits/Second",
KILOBITS_PER_SECOND = "Kilobits/Second",
MEGABITS_PER_SECOND = "Megabits/Second",
GIGABITS_PER_SECOND = "Gigabits/Second",
TERABITS_PER_SECOND = "Terabits/Second",
COUNT_PER_SECOND = "Count/Second",
NONE = "None"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert this change, (and if possible, drop a trailing , after the NONE entry, too)

}

/**
* Properties of a rendered metric
*/
export interface MetricConfig {
/**
* In case the metric represents a query, the details of the query
*/
readonly metricStat?: MetricStatConfig;

/**
* In case the metric is a math expression, the details of the math expression
*/
readonly mathExpression?: MetricExpressionConfig;

/**
* Additional properties which will be rendered if the metric is used in a dashboard
*
* Examples are 'label' and 'color', but any key in here will be
* added to dashboard graphs.
*/
readonly renderingProperties?: Record<string, any>;
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Properties for a concrete metric
*
* NOTE: `unit` is no longer on this object since it is only used for `Alarms`, and doesn't mean what one
* would expect it to mean there anyway. It is most likely to be misused.
*/
export interface MetricStatConfig {
/**
* The dimensions to apply to the alarm
*/
readonly dimensions?: Dimension[];

/**
* Namespace of the metric
*/
readonly namespace: string;

/**
* Name of the metric
*/
readonly metricName: string;

/**
* How many seconds to aggregate over
*/
readonly period: number;

/**
* Aggregation function to use (can be either simple or a percentile)
*/
readonly statistic: string;

/**
* Region of the metric
*/
readonly region?: string;

/**
* Account of the metric
*/
readonly account?: string;
}

/**
* Properties for a concrete metric
*/
export interface MetricExpressionConfig {
/**
* Math expression for the metric.
*/
readonly expression: string;

/**
* Metrics used in the math expression
*/
readonly expressionMetrics: Record<string, IMetric>;
}

/**
* Properties used to construct the Metric identifying part of an Alarm
*
* @deprecated Replaced by MetricConfig
*/
export interface MetricAlarmConfig {
/**
Expand Down Expand Up @@ -115,6 +207,8 @@ export interface MetricAlarmConfig {

/**
* Properties used to construct the Metric identifying part of a Graph
* *
* @deprecated Replaced by MetricConfig
*/
export interface MetricGraphConfig {
/**
Expand Down Expand Up @@ -175,6 +269,8 @@ export interface MetricGraphConfig {

/**
* Custom rendering properties that override the default rendering properties specified in the yAxis parameter of the widget object.
*
* @deprecated Replaced by `MetricConfig`.
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
*/
export interface MetricRenderingProperties {
/**
Expand Down
Loading