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): add verticalAnnotations property to GraphWidget #22568

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
5 changes: 4 additions & 1 deletion packages/@aws-cdk/aws-cloudwatch/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ dashboard.addWidgets(new cloudwatch.GraphWidget({

Using the methods `addLeftMetric()` and `addRightMetric()` you can add metrics to a graph widget later on.

Graph widgets can also display annotations attached to the left or the right y-axis.
Graph widgets can also display annotations attached to the left or right y-axes or the x-axis.

```ts
declare const dashboard: cloudwatch.Dashboard;
Expand All @@ -384,6 +384,9 @@ dashboard.addWidgets(new cloudwatch.GraphWidget({
{ value: 1800, label: Duration.minutes(30).toHumanString(), color: cloudwatch.Color.RED, },
{ value: 3600, label: '1 hour', color: '#2ca02c', }
],
verticalAnnotations: [
{ value: '2022-10-19T00:00:00Z', label: 'Deployment', color: Color.RED, }
]
}));
```

Expand Down
78 changes: 76 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/lib/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,13 @@ export interface GraphWidgetProps extends MetricWidgetProps {
*/
readonly rightAnnotations?: HorizontalAnnotation[];

/**
* Annotations for the vertical axis
*
* @default - No annotations
*/
readonly verticalAnnotations?: VerticalAnnotation[];

/**
* Whether the graph should be shown as stacked lines
*
Expand Down Expand Up @@ -413,6 +420,14 @@ export class GraphWidget extends ConcreteWidget {
...(this.props.leftAnnotations || []).map(mapAnnotation('left')),
...(this.props.rightAnnotations || []).map(mapAnnotation('right')),
];
const verticalAnnotations = (this.props.verticalAnnotations || []).map(({ date, ...rest }) => ({
value: date.toISOString(),
...rest,
}));
const annotations = horizontalAnnotations.length > 0 || verticalAnnotations.length > 0 ? ({
horizontal: horizontalAnnotations.length > 0 ? horizontalAnnotations : undefined,
vertical: verticalAnnotations.length > 0 ? verticalAnnotations : undefined,
}) : undefined;

const metrics = allMetricsGraphJson(this.leftMetrics, this.rightMetrics);
return [{
Expand All @@ -427,7 +442,7 @@ export class GraphWidget extends ConcreteWidget {
region: this.props.region || cdk.Aws.REGION,
stacked: this.props.stacked,
metrics: metrics.length > 0 ? metrics : undefined,
annotations: horizontalAnnotations.length > 0 ? { horizontal: horizontalAnnotations } : undefined,
annotations,
yAxis: {
left: this.props.leftYAxis ?? undefined,
right: this.props.rightYAxis ?? undefined,
Expand Down Expand Up @@ -641,7 +656,46 @@ export interface HorizontalAnnotation {
}

/**
* Fill shading options that will be used with an annotation
* Vertical annotation to be added to a graph
*/
export interface VerticalAnnotation {
/**
* The date and time in the graph where the vertical annotation line is to appear
*/
readonly date: Date;

/**
* Label for the annotation
*
* @default - No label
*/
readonly label?: string;

/**
* The hex color code, prefixed with '#' (e.g. '#00ff00'), to be used for the annotation.
* The `Color` class has a set of standard colors that can be used here.
*
* @default - Automatic color
*/
readonly color?: string;

/**
* Add shading before or after the annotation
*
* @default No shading
*/
readonly fill?: VerticalShading;

/**
* Whether the annotation is visible
*
* @default true
*/
readonly visible?: boolean;
}

/**
* Fill shading options that will be used with a horizontal annotation
*/
export enum Shading {
/**
Expand All @@ -660,6 +714,26 @@ export enum Shading {
BELOW = 'below'
}

/**
* Fill shading options that will be used with a vertical annotation
*/
export enum VerticalShading {
/**
* Don't add shading
*/
NONE = 'none',

/**
* Add shading before the annotation
*/
BEFORE = 'before',

/**
* Add shading after the annotation
*/
AFTER = 'after'
}

/**
* A set of standard colours that can be used in annotations in a GraphWidget.
*/
Expand Down
45 changes: 43 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/test/graphs.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Duration, Stack } from '@aws-cdk/core';
import { Alarm, AlarmWidget, Color, GraphWidget, GraphWidgetView, LegendPosition, LogQueryWidget, Metric, Shading, SingleValueWidget, LogQueryVisualizationType, CustomWidget, GaugeWidget } from '../lib';
import { Alarm, AlarmWidget, Color, CustomWidget, GaugeWidget, GraphWidget, GraphWidgetView, LegendPosition, LogQueryVisualizationType, LogQueryWidget, Metric, Shading, SingleValueWidget, VerticalShading } from '../lib';

describe('Graphs', () => {
test('add stacked property to graphs', () => {
Expand Down Expand Up @@ -413,7 +413,7 @@ describe('Graphs', () => {
}]);
});

test('add annotations to graph', () => {
test('add horizontal annotations to graph', () => {
// WHEN
const stack = new Stack();
const widget = new GraphWidget({
Expand Down Expand Up @@ -453,8 +453,49 @@ describe('Graphs', () => {
yAxis: {},
},
}]);
});

test('add vertical annotations to graph', () => {
const date = new Date('2021-07-29T02:31:09.890Z');

// WHEN
const stack = new Stack();
const widget = new GraphWidget({
title: 'My fancy graph',
left: [
new Metric({ namespace: 'CDK', metricName: 'Test' }),
],
verticalAnnotations: [{
date,
color: '667788',
fill: VerticalShading.AFTER,
label: 'this is the annotation',
}],
});

// THEN
expect(stack.resolve(widget.toJson())).toEqual([{
type: 'metric',
width: 6,
height: 6,
properties: {
view: 'timeSeries',
title: 'My fancy graph',
region: { Ref: 'AWS::Region' },
metrics: [
['CDK', 'Test'],
],
annotations: {
vertical: [{
value: '2021-07-29T02:31:09.890Z',
color: '667788',
fill: 'after',
label: 'this is the annotation',
}],
},
yAxis: {},
},
}]);
});

test('convert alarm to annotation', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"QueueName"
]
},
"\"]],\"annotations\":{\"horizontal\":[{\"label\":\"ApproximateNumberOfMessagesVisible >= 100 for 2 datapoints within 15 minutes\",\"value\":100,\"yAxis\":\"left\"}]},\"yAxis\":{}}},{\"type\":\"metric\",\"width\":6,\"height\":3,\"x\":0,\"y\":17,\"properties\":{\"view\":\"singleValue\",\"title\":\"Current messages in queue\",\"region\":\"",
"\"]],\"annotations\":{\"horizontal\":[{\"label\":\"ApproximateNumberOfMessagesVisible >= 100 for 2 datapoints within 15 minutes\",\"value\":100,\"yAxis\":\"left\"}],\"vertical\":[{\"value\":\"2022-10-19T00:00:00.000Z\",\"label\":\"Deployment\",\"color\":\"#d62728\"}]},\"yAxis\":{}}},{\"type\":\"metric\",\"width\":6,\"height\":3,\"x\":0,\"y\":17,\"properties\":{\"view\":\"singleValue\",\"title\":\"Current messages in queue\",\"region\":\"",
{
"Ref": "AWS::Region"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ dashboard.addWidgets(new cloudwatch.GraphWidget({
title: 'More messages in queue with alarm annotation',
left: [numberOfMessagesVisibleMetric],
leftAnnotations: [alarm.toAnnotation()],
verticalAnnotations: [
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to see a new test that includes these annotations instead of editing an existing test.

Copy link
Author

Choose a reason for hiding this comment

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

may I ask why? that would involve a whole lot of boilerplate setting up a new test, this was trivial to add

Copy link
Contributor

Choose a reason for hiding this comment

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

Because when we pile too much functionality into one test, it's harder to decipher if we're getting the output we're looking for and harder to troubleshoot when a test fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, was this test run and deployed? If so, I'd expect to see changes to more than these two files. It appears that this was run with --dry-run or that it was manually edited.

{
date: new Date('2022-10-19T00:00:00Z'),
label: 'Deployment',
color: cloudwatch.Color.RED,
},
],
}));
dashboard.addWidgets(new cloudwatch.SingleValueWidget({
title: 'Current messages in queue',
Expand Down