-
Notifications
You must be signed in to change notification settings - Fork 56
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
Cannot create an Alarm based on a MathExpression which specifies a searchAccount or searchRegion #428
Comments
I think this is because of the global defaults. With #415, the account and region in the global defaults will be passed into MathExpression and I happened to have the global defaults defined. |
Easiest fix that I could think of is just strip out account and region somewhere in
searchAccount and searchRegion .
I have also tried to workaround the problem by providing a metricAdjuster to unset https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-cloudwatch/lib/metric.ts#L652-L660 public with(props: MathExpressionOptions): MathExpression {
// Short-circuit creating a new object if there would be no effective change
if ((props.label === undefined || props.label === this.label)
&& (props.color === undefined || props.color === this.color)
&& (props.period === undefined || props.period.toSeconds() === this.period.toSeconds())
&& (props.searchAccount === undefined || props.searchAccount === this.searchAccount)
&& (props.searchRegion === undefined || props.searchRegion === this.searchRegion)) {
return this;
}
return new MathExpression({
expression: this.expression,
usingMetrics: this.usingMetrics,
label: ifUndefined(props.label, this.label),
color: ifUndefined(props.color, this.color),
period: ifUndefined(props.period, this.period),
searchAccount: ifUndefined(props.searchAccount, this.searchAccount),
searchRegion: ifUndefined(props.searchRegion, this.searchRegion),
});
} Something like the following will do for the time being adjustMetric: (metric: MetricWithAlarmSupport, alarmScope: Construct, props: AddAlarmProps) => {
if (metric.toMetricConfig().mathExpression) {
return new MathExpression({...metric, searchAccount: undefined, searchRegion: undefined} as MathExpressionProps);
} else {
return metric;
}
} |
Yeah, #415 would be the cause of this and you should be right with the relation to global defaults being set. We could potentially pass in the stack's info to You'd still encounter the error if you try to alarm on those given you can't do search expressions cross-region/region anyway. |
Does this mean that I can't deploy a stack with alarms containing a I started having this problem when monitoring my RDS cluster ( |
Version
5.10.0
Steps and/or minimal code example to reproduce
I have a few constructs that create DDB alarms, they used to be work with 5.4.2 but fails to build with 5.10.0, I am getting
Here is my alarm config
Expected behavior
DDB alarms are created.
Actual behavior
Not able to get alarms created.
Other details
I think this is related to #415 where we added
searchAccount
andsearchRegion
tocreateMetricMath
and ReadThrottledEventsCountAlarm and WriteThrottledEventsCountAlarm use math expression.cdk-monitoring-constructs/lib/monitoring/aws-dynamo/DynamoTableMetricFactory.ts
Lines 138 to 141 in b07f361
The change impacts more than just DDB alarms I feel, it affects all alarm creations where math expression will be used
The text was updated successfully, but these errors were encountered: