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

Cloudwatch metrics #180

Merged
merged 6 commits into from
Jun 28, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
19 changes: 17 additions & 2 deletions packages/@aws-cdk/assert/lib/assertions/have-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ import { StackInspector } from "../inspector";

/**
* An assertion to check whether a resource of a given type and with the given properties exists, disregarding properties
*
* Properties can be:
*
* - An object, in which case its properties will be compared to those of the actual resource found
* - A callablage, in which case it will be treated as a predicate that is applied to the Properties of the found resources.
Copy link
Contributor

Choose a reason for hiding this comment

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

What's a callablage? :)

*/
export function haveResource(resourceType: string, properties?: any): Assertion<StackInspector> {
return new HaveResourceAssertion(resourceType, properties);
Expand All @@ -21,7 +26,17 @@ class HaveResourceAssertion extends Assertion<StackInspector> {
const resource = inspector.value.Resources[logicalId];
if (resource.Type === this.resourceType) {
this.inspected.push(resource);
if (isSuperObject(resource.Properties, this.properties)) {

let matches: boolean;
if (typeof this.properties === 'function') {
// If 'properties' is a callable, invoke it
matches = this.properties(resource.Properties);
} else {
// Otherwise treat as property bag that we check superset of
matches = isSuperObject(resource.Properties, this.properties);
}

if (matches) {
return true;
}
}
Expand All @@ -47,7 +62,7 @@ class HaveResourceAssertion extends Assertion<StackInspector> {
*
* A super-object has the same or more property values, recursing into nested objects.
*/
function isSuperObject(superObj: any, obj: any): boolean {
export function isSuperObject(superObj: any, obj: any): boolean {
if (obj == null) { return true; }
if (Array.isArray(superObj) !== Array.isArray(obj)) { return false; }
if (Array.isArray(superObj)) {
Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/cloudwatch/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
*.js
tsconfig.json
tslint.json
*.js.map
*.d.ts
dist
lib/generated/resources.ts
44 changes: 44 additions & 0 deletions packages/@aws-cdk/cloudwatch/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
Add alarms and graphs to CDK applications
=========================================


Making Alarms
-------------

Copy link
Contributor

Choose a reason for hiding this comment

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

Missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I just posted what I had so far to give y'all a chance of shooting at it. Still need to write some sections and add tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also would be nice to document the idiom of construct.metricOfXxx

Making Dashboards
-----------------

Dashboards are set of Widgets stored server-side which can be accessed quickly
from the AWS console. Available widgets are graphs of a metric over time, the
current value of a metric, or a static piece of Markdown which explains what the
graphs mean.

The following widgets are available:

- `GraphWidget` -- shows any number of metrics on both the left and right
vertical axes.
- `AlarmWidget` -- shows the graph and alarm line for a single alarm.
- `SingleValueWidget` -- shows the current value of a set of metrics.
- `TextWidget` -- shows some static Markdown.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we call this MarkdownWidget (what's the name used in the CloudWatch docs?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

text, actually. But we're already deviating slightly from their widget structure. I'm okay with calling it MarkdownWidget.

Copy link
Contributor

Choose a reason for hiding this comment

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

So let's stay with text. It's not worth the deviation.



Dashboard Layout
----------------

The widgets on a dashboard are visually laid out in a grid that is 24 columns
wide. Normally you specify X and Y coordinates for the widgets on a Dashboard,
but because this is inconvenient to do manually, the library contains a simple
layout system to help you lay out your dashboards the way you want them to.

Widgets have a `width` and `height` property, and they will be automatically
laid out either horizontally or vertically stacked to fill out the available
space.

Widgets are added to a Dashboard by calling `add(widget1, widget2, ...)`.
Widgets given in the same call will be laid out horizontally. Widgets given
in different calls will be laid out vertically. To make more complex layouts,
you can use the following widgets to pack widgets together in different ways:

Copy link
Contributor

Choose a reason for hiding this comment

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

I ❤️ this

- `Column`: stack two or more widgets vertically.
- `Row`: lay out two or more widgets horizontally.
- `Spacer`: take up empty space
195 changes: 195 additions & 0 deletions packages/@aws-cdk/cloudwatch/lib/alarm.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
import { Arn, Construct, Token } from '@aws-cdk/core';
import { cloudwatch } from '@aws-cdk/resources';
import { Metric } from './metric';

/**
* Properties for Alarms
*/
export interface AlarmProps {
/**
* The metric to add the alarm on
Copy link
Contributor

Choose a reason for hiding this comment

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

A little documentation hint on how to obtain/create metrics

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

*/
metric: Metric;

/**
* Name of the alarm
*
* @default Automatically generated name
*/
alarmName?: string;

/**
* Description for the alarm
*
* @default No description
*/
alarmDescription?: string;

/**
* Comparison to use to check if metric is breaching
*
* @default GreaterThanOrEqualToThreshold
*/
comparisonOperator?: ComparisonOperator;

/**
* The value against which the specified statistic is compared.
*/
threshold: number;

/**
* The number of periods over which data is compared to the specified threshold.
*/
evaluationPeriods: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we provide a sensible default? 1 maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose. But I kinda feel this is a decision you must consciously make. And 1 is almost never the right answer, I would default to 2 or 3.

Copy link
Contributor

Choose a reason for hiding this comment

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

.... if there is no clear sensible default than I guess we shouldn't provide one... Maybe that's a question for the CW team


/**
* Number of datapoints that must be breaching to trigger the alarm
*
* This is used only if you are setting an "M out of N" alarm. In that case, this value is the M.
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you set an "M out of N" alarm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By setting both evaluationPeriods and dataPointsToAlarm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, heh. I was looking at the API call documentation while writing this. The feature's been released in Dec 2017 but hasn't been added to the CloudFormation API yet. Removing.

*
* @default Not an "M out of N" alarm.
*/
datapointsToAlarm?: number;

/**
* Specifies whether to evaluate the data and potentially change the alarm state if there are too few data points to be statistically significant.
Copy link
Contributor

Choose a reason for hiding this comment

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

wrap

*
* Used only for alarms that are based on percentiles.
*/
evaluateLowSampleCountPercentile?: string;

/**
* Sets how this alarm is to handle missing data points.
*
* @default TreatMissingData.Missing
*/
treatMissingData?: TreatMissingData;

/**
* Whether the actions for this alarm are enabled
*
* @default true
*/
actionsEnabled?: boolean;
}

/**
* Comparison operator for evaluating alarms
*/
export enum ComparisonOperator {
GreaterThanOrEqualToThreshold = 'GreaterThanOrEqualToThreshold',
GreaterThanThreshold = 'GreaterThanThreshold',
LessThanThreshold = 'LessThanThreshold',
LessThanOrEqualToThreshold = 'LessThanOrEqualToThreshold',
}

/**
* Specify how missing data points are treated during alarm evaluation
*/
export enum TreatMissingData {
/**
* Missing data points are treated as breaching the threshold
*/
Breaching = 'breaching',

/**
* Missing data points are treated as being within the threshold
*/
NotBreaching = 'notBreaching',

/**
* The current alarm state is maintained
*/
Ignore = 'ignore',

/**
* The alarm does not consider missing data points when evaluating whether to change state
*/
Missing = 'missing'
}

/**
* An alarm on a CloudWatch metric
*/
export class Alarm extends Construct {
/**
* ARN of this alarm
*/
public readonly alarmArn: AlarmArn;

/**
* The metric object this alarm was based on
*/
public readonly metric: Metric;

private alarmActions?: Arn[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be IAlarmAction instead of just a list of Arns?

Copy link
Contributor Author

@rix0rrr rix0rrr Jun 27, 2018

Choose a reason for hiding this comment

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

Probably, because the typing leaves much to be desired at the moment. Didn't really want to add more public methods to Topic, but yeah, that's what's going to have to happen anyway, you're right.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sadly we don't have ObjectiveC/Swift protocols or C# extensions...

private insufficientDataActions?: Arn[];
private okActions?: Arn[];

constructor(parent: Construct, name: string, props: AlarmProps) {
super(parent, name);

const alarm = new cloudwatch.AlarmResource(this, 'Resource', {
actionsEnabled: props.actionsEnabled,
alarmActions: new Token(() => this.alarmActions),
alarmDescription: props.alarmDescription,
alarmName: props.alarmName,
comparisonOperator: props.comparisonOperator || ComparisonOperator.GreaterThanOrEqualToThreshold,
evaluateLowSampleCountPercentile: props.evaluateLowSampleCountPercentile,
evaluationPeriods: props.evaluationPeriods,
insufficientDataActions: new Token(() => this.insufficientDataActions),
Copy link
Contributor

Choose a reason for hiding this comment

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

Organize all the "actions" properties next to each other...

okActions: new Token(() => this.okActions),
threshold: props.threshold,
treatMissingData: props.treatMissingData,
...props.metric.toAlarmJson()
});

this.alarmArn = alarm.alarmArn;
this.metric = props.metric;
}

/**
* Trigger this action if the alarm fires
*
* Typically the ARN of an SNS topic or ARN of an AutoScaling policy.
*/
public onAlarm(arn: Arn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a little hesitant to use onXxx semantics here to avoid conflating these APIs with the CloudWatch events. Maybe instead whenAlarm, whenInsufficientData or addAlarmAction, addInsufficientDataAction instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but conceivably, it should work just like CloudWatch events. We're describing events, and onXxx is the canonical way of describing event handlers!

(Also, imagine my face when I found it that Alarms don't actually integrate with Events in any way :). Implementation/legacy details bubbling up. I can only imagine that they want to integrate Alarms with CWE at some point in the future. I feel like it makes very little sense not to)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typing will make sure you don't pass the wrong object in, especially when we move to IAlarmAction.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but conceivably, it should work just like CloudWatch events.

It's a good point but then I think the API should be uniform. onXxx([target[, [filter]]) returns an EventRule which you can then .addTarget(t[, transform]) on.

The reason I am getting a bit more sensitive to that is that we are starting to have some conversations about how rich tools like IDEs could reason about the AWS construct library (#181).

if (this.alarmActions === undefined) {
this.alarmActions = [];
}

this.alarmActions.push(arn);
}

/**
* Trigger this action if there is insufficient data to evaluate the alarm
*
* Typically the ARN of an SNS topic or ARN of an AutoScaling policy.
*/
public onInsufficientData(arn: Arn) {
if (this.insufficientDataActions === undefined) {
this.insufficientDataActions = [];
}

this.insufficientDataActions.push(arn);
}

/**
* Trigger this action if the alarm returns from breaching state into ok state
*
* Typically the ARN of an SNS topic or ARN of an AutoScaling policy.
*/
public onOk(arn: Arn) {
if (this.okActions === undefined) {
this.okActions = [];
}

this.okActions.push(arn);
}
}

/**
* The ARN of an Alarm
*/
export class AlarmArn extends Arn {
}
51 changes: 51 additions & 0 deletions packages/@aws-cdk/cloudwatch/lib/dashboard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { Construct, resolve, Token } from "@aws-cdk/core";
import { cloudwatch } from "@aws-cdk/resources";
import { Column, Row } from "./layout";
import { Widget } from "./widget";

export interface DashboardProps {
/**
* Name of the dashboard
*
* @default Automatically generated name
*/
dashboardName?: string;
}

/**
* A CloudWatch dashboard
*/
export class Dashboard extends Construct {
private readonly rows: Widget[] = [];

constructor(parent: Construct, name: string, props?: DashboardProps) {
super(parent, name);

new cloudwatch.DashboardResource(this, 'Resource', {
dashboardName: props && props.dashboardName,
dashboardBody: new Token(() => {
const column = new Column(...this.rows);
column.position(0, 0);
return JSON.stringify(resolve({ widgets: column.toJson() }));
})
});
}

/**
* Add a widget to the dashboard.
*
* Widgets given in multiple calls to add() will be laid out stacked on
* top of each other.
*
* Multiple widgets added in the same call to add() will be laid out next
* to each other.
*/
public add(...widgets: Widget[]) {
if (widgets.length === 0) {
return;
}

const w = widgets.length > 1 ? new Row(...widgets) : widgets[0];
this.rows.push(w);
}
}
Loading