Skip to content

Commit

Permalink
Merge branch 'main' into glue-ver-5
Browse files Browse the repository at this point in the history
  • Loading branch information
mazyu36 authored Dec 11, 2024
2 parents b1d0d18 + 59e96a3 commit af74f76
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 524 deletions.
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-appsync/lib/code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export abstract class Code {
/**
* Inline code for AppSync function
* @returns `InlineCode` with inline code.
* @param code The actual handler code (limited to 4KiB)
* @param code The actual handler code (the resulting zip file cannot exceed 4MB)
*/
public static fromInline(code: string): InlineCode {
return new InlineCode(code);
Expand Down
73 changes: 63 additions & 10 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,35 @@ export interface MathExpressionProps extends MathExpressionOptions {
* The key is the identifier that represents the given metric in the
* expression, and the value is the actual Metric object.
*
* The `period` of each metric in `usingMetrics` is ignored and instead overridden
* by the `period` specified for the `MathExpression` construct. Even if no `period`
* is specified for the `MathExpression`, it will be overridden by the default
* value (`Duration.minutes(5)`).
*
* Example:
*
* ```ts
* declare const metrics: elbv2.IApplicationLoadBalancerMetrics;
* new cloudwatch.MathExpression({
* expression: 'm1+m2',
* label: 'AlbErrors',
* usingMetrics: {
* m1: metrics.custom('HTTPCode_ELB_500_Count', {
* period: Duration.minutes(1), // <- This period will be ignored
* statistic: 'Sum',
* label: 'HTTPCode_ELB_500_Count',
* }),
* m2: metrics.custom('HTTPCode_ELB_502_Count', {
* period: Duration.minutes(1), // <- This period will be ignored
* statistic: 'Sum',
* label: 'HTTPCode_ELB_502_Count',
* }),
* },
* period: Duration.minutes(3), // <- This overrides the period of each metric in `usingMetrics`
* // (Even if not specified, it is overridden by the default value)
* });
* ```
*
* @default - Empty map.
*/
readonly usingMetrics?: Record<string, IMetric>;
Expand Down Expand Up @@ -605,12 +634,19 @@ export class MathExpression implements IMetric {
constructor(props: MathExpressionProps) {
this.period = props.period || cdk.Duration.minutes(5);
this.expression = props.expression;
this.usingMetrics = changeAllPeriods(props.usingMetrics ?? {}, this.period);
this.label = props.label;
this.color = props.color;
this.searchAccount = props.searchAccount;
this.searchRegion = props.searchRegion;

const { record, overridden } = changeAllPeriods(props.usingMetrics ?? {}, this.period);
this.usingMetrics = record;

const warnings: { [id: string]: string } = {};
if (overridden) {
warnings['CloudWatch:Math:MetricsPeriodsOverridden'] = `Periods of metrics in 'usingMetrics' for Math expression '${this.expression}' have been overridden to ${this.period.toSeconds()} seconds.`;
}

const invalidVariableNames = Object.keys(this.usingMetrics).filter(x => !validVariableName(x));
if (invalidVariableNames.length > 0) {
throw new Error(`Invalid variable names in expression: ${invalidVariableNames}. Must start with lowercase letter and only contain alphanumerics.`);
Expand All @@ -624,7 +660,6 @@ export class MathExpression implements IMetric {
// we can add warnings.
const missingIdentifiers = allIdentifiersInExpression(this.expression).filter(i => !this.usingMetrics[i]);

const warnings: { [id: string]: string } = {};
if (!this.expression.toUpperCase().match('\\s*INSIGHT_RULE_METRIC|SELECT|SEARCH|METRICS\\s.*') && missingIdentifiers.length > 0) {
warnings['CloudWatch:Math:UnknownIdentifier'] = `Math expression '${this.expression}' references unknown identifiers: ${missingIdentifiers.join(', ')}. Please add them to the 'usingMetrics' map.`;
}
Expand Down Expand Up @@ -879,23 +914,33 @@ function ifUndefined<T>(x: T | undefined, def: T | undefined): T | undefined {
/**
* Change periods of all metrics in the map
*/
function changeAllPeriods(metrics: Record<string, IMetric>, period: cdk.Duration): Record<string, IMetric> {
const ret: Record<string, IMetric> = {};
for (const [id, metric] of Object.entries(metrics)) {
ret[id] = changePeriod(metric, period);
function changeAllPeriods(metrics: Record<string, IMetric>, period: cdk.Duration): { record: Record<string, IMetric>; overridden: boolean } {
const retRecord: Record<string, IMetric> = {};
let retOverridden = false;
for (const [id, m] of Object.entries(metrics)) {
const { metric, overridden } = changePeriod(m, period);
retRecord[id] = metric;
if (overridden) {
retOverridden = true;
}
}
return ret;
return { record: retRecord, overridden: retOverridden };
}

/**
* Return a new metric object which is the same type as the input object, but with the period changed
* Return a new metric object which is the same type as the input object but with the period changed,
* and a flag to indicate whether the period has been overwritten.
*
* Relies on the fact that implementations of `IMetric` are also supposed to have
* an implementation of `with` that accepts an argument called `period`. See `IModifiableMetric`.
*/
function changePeriod(metric: IMetric, period: cdk.Duration): IMetric {
function changePeriod(metric: IMetric, period: cdk.Duration): { metric: IMetric; overridden: boolean} {
if (isModifiableMetric(metric)) {
return metric.with({ period });
const overridden =
isMetricWithPeriod(metric) && // always true, as the period property is set with a default value even if it is not specified
metric.period.toSeconds() !== cdk.Duration.minutes(5).toSeconds() && // exclude the default value of a metric, assuming the user has not specified it
metric.period.toSeconds() !== period.toSeconds();
return { metric: metric.with({ period }), overridden };
}

throw new Error(`Metric object should also implement 'with': ${metric}`);
Expand Down Expand Up @@ -927,6 +972,14 @@ function isModifiableMetric(m: any): m is IModifiableMetric {
return typeof m === 'object' && m !== null && !!m.with;
}

interface IMetricWithPeriod {
period: cdk.Duration;
}

function isMetricWithPeriod(m: any): m is IMetricWithPeriod {
return typeof m === 'object' && m !== null && !!m.period;
}

// Polyfill for string.matchAll(regexp)
function matchAll(x: string, re: RegExp): RegExpMatchArray[] {
const ret = new Array<RegExpMatchArray>();
Expand Down
84 changes: 84 additions & 0 deletions packages/aws-cdk-lib/aws-cloudwatch/test/metric-math.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,90 @@ describe('Metric Math', () => {
});
});

test('warn if a period is specified in usingMetrics and not equal to the value of the period for MathExpression', () => {
const m = new MathExpression({
expression: 'm1',
usingMetrics: {
m1: new Metric({ namespace: 'Test', metricName: 'ACount', period: Duration.minutes(4) }),
},
period: Duration.minutes(3),
});

expect(m.warningsV2).toMatchObject({
'CloudWatch:Math:MetricsPeriodsOverridden': 'Periods of metrics in \'usingMetrics\' for Math expression \'m1\' have been overridden to 180 seconds.',
});
});

test('warn if periods are specified in usingMetrics and one is not equal to the value of the period for MathExpression', () => {
const m = new MathExpression({
expression: 'm1 + m2',
usingMetrics: {
m1: new Metric({ namespace: 'Test', metricName: 'ACount', period: Duration.minutes(4) }),
m2: new Metric({ namespace: 'Test', metricName: 'BCount', period: Duration.minutes(3) }),
},
period: Duration.minutes(3),
});

expect(m.warningsV2).toMatchObject({
'CloudWatch:Math:MetricsPeriodsOverridden': 'Periods of metrics in \'usingMetrics\' for Math expression \'m1 + m2\' have been overridden to 180 seconds.',
});
});

test('warn if a period is specified in usingMetrics and not equal to the default value of the period for MathExpression', () => {
const m = new MathExpression({
expression: 'm1',
usingMetrics: {
m1: new Metric({ namespace: 'Test', metricName: 'ACount', period: Duration.minutes(4) }),
},
});

expect(m.warningsV2).toMatchObject({
'CloudWatch:Math:MetricsPeriodsOverridden': 'Periods of metrics in \'usingMetrics\' for Math expression \'m1\' have been overridden to 300 seconds.',
});
});

test('can raise multiple warnings', () => {
const m = new MathExpression({
expression: 'e1 + m1',
usingMetrics: {
e1: new MathExpression({
expression: 'n1 + n2',
}),
m1: new Metric({ namespace: 'Test', metricName: 'ACount', period: Duration.minutes(4) }),
},
period: Duration.minutes(3),
});

expect(m.warningsV2).toMatchObject({
'CloudWatch:Math:MetricsPeriodsOverridden': 'Periods of metrics in \'usingMetrics\' for Math expression \'e1 + m1\' have been overridden to 180 seconds.',
'CloudWatch:Math:UnknownIdentifier': expect.stringContaining("'n1 + n2' references unknown identifiers"),
});
});

test('don\'t warn if a period is not specified in usingMetrics', () => {
const m = new MathExpression({
expression: 'm1',
usingMetrics: {
m1: new Metric({ namespace: 'Test', metricName: 'ACount' }),
},
period: Duration.minutes(3),
});

expect(m.warningsV2).toBeUndefined();
});

test('don\'t warn if a period is specified in usingMetrics but equal to the value of the period for MathExpression', () => {
const m = new MathExpression({
expression: 'm1',
usingMetrics: {
m1: new Metric({ namespace: 'Test', metricName: 'ACount', period: Duration.minutes(3) }),
},
period: Duration.minutes(3),
});

expect(m.warningsV2).toBeUndefined();
});

describe('in graphs', () => {
test('MathExpressions can be added to a graph', () => {
// GIVEN
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-lambda/lib/code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export abstract class Code {
/**
* Inline code for Lambda handler
* @returns `LambdaInlineCode` with inline code.
* @param code The actual handler code (limited to 4KiB)
* @param code The actual handler code (the resulting zip file cannot exceed 4MB)
*/
public static fromInline(code: string): InlineCode {
return new InlineCode(code);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { Construct } from 'constructs';
import { Stack, Duration } from 'aws-cdk-lib';
import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch';
import * as elbv2 from 'aws-cdk-lib/aws-elasticloadbalancingv2';
import * as route53 from 'aws-cdk-lib/aws-route53';
import * as sns from 'aws-cdk-lib/aws-sns';
import * as lambda from 'aws-cdk-lib/aws-lambda';
Expand Down
Loading

0 comments on commit af74f76

Please sign in to comment.