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

aws_cdk.aws_cloudwatch: CfnAlarm drops MetricDataQueryProperty contents if arguments come from a dict #27372

Closed
SC-CTS opened this issue Oct 2, 2023 · 2 comments
Labels
@aws-cdk/aws-cloudwatch Related to Amazon CloudWatch closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@SC-CTS
Copy link

SC-CTS commented Oct 2, 2023

Describe the bug

When instantiatig a CfnAlarm from a Dict as follows:

CfnAlarm(scope=self, id="cfn-alarm", **{
    'alarm_name': 'Descriptive Name', 
    'alarm_description': 'Very descriptive description.', 
    'actions_enabled': True, 
    'ok_actions': ['arn:aws:sns:eu-central-1:666:XXX_CloudWatch_Integration'], 
    'alarm_actions': ['arn:aws:sns:eu-central-1:666:XXX_CloudWatch_Integration'], 
    'insufficient_data_actions': [], 
    'evaluation_periods': 15, 
    'datapoints_to_alarm': 15, 
    'threshold': 1.5, 
    'comparison_operator': 'LessThanThreshold', 
    'treat_missing_data': 'missing', 
    'metrics': [
      {
        'id': 'e1', 
        'label': 'Currently running Alerts', 
        'return_data': True, 
        'expression': 'MAX(METRICS()/3)'
      }, 
      {
        'id': 'm1', 
        'return_data': False, 
        'metric_stat': {
          'metric': {
            'namespace': 'project', 
            'metric_name': 'gauge.project.value', 
            'dimensions': []
          }, 
          'period': 60, 
          'stat': 'Average'
        }
      }
    ]
  })

It will fail with the following error from the Stack: Resource handler returned message: "Period must not be null (Service: CloudWatch, Status Code: 400, Request ID: 123)"

Expected Behavior

The Alarm should be created successfully and the stack should not fail.

Current Behavior

When checking the synth output in cdk.out I see the following was generated:

{
  "abcde": {
    "Type": "AWS::CloudWatch::Alarm",
    "Properties": {
      "ActionsEnabled": true,
      "AlarmActions": [
        "arn:aws:sns:eu-central-1:666:XXX_CloudWatch_Integration"
      ],
      "AlarmDescription": "Very descriptive description.",
      "AlarmName": "Descriptive Name",
      "ComparisonOperator": "LessThanThreshold",
      "DatapointsToAlarm": 15,
      "EvaluationPeriods": 15,
      "InsufficientDataActions": [],
      "Metrics": [
        {
          "Expression": "MAX(METRICS()/3)",
          "Id": "e1",
          "Label": "Currently running Alerts"
        },
        {
          "Id": "m1"
        }
      ],
      "OKActions": [
        "arn:aws:sns:eu-central-1:666:XXX_CloudWatch_Integration"
      ],
      "Threshold": 1.5,
      "TreatMissingData": "missing"
    },
    "DependsOn": [
      "qwer",
      "zxc"
    ],
    "Metadata": {
      "aws:cdk:path": "hjk"
    }
  }
}

The second object in abcde.Properties.Metrics with "id": "m1" is missing all properties.

Reproduction Steps

See provided code.

Possible Solution

Instantiating the MetricDataQueryProperty manually like so:

CfnAlarm(scope=self, id="cfn-alarm", **{
    'alarm_name': 'Descriptive Name', 
    'alarm_description': 'Very descriptive description.', 
    'actions_enabled': True, 
    'ok_actions': ['arn:aws:sns:eu-central-1:666:XXX_CloudWatch_Integration'], 
    'alarm_actions': ['arn:aws:sns:eu-central-1:666:XXX_CloudWatch_Integration'], 
    'insufficient_data_actions': [], 
    'evaluation_periods': 15, 
    'datapoints_to_alarm': 15, 
    'threshold': 1.5, 
    'comparison_operator': 'LessThanThreshold', 
    'treat_missing_data': 'missing', 
    'metrics': [
      CfnAlarm.MetricDataQueryProperty(**{
        'id': 'e1', 
        'label': 'Currently running Alerts', 
        'return_data': True, 
        'expression': 'MAX(METRICS()/3)'
      }), 
      CfnAlarm.MetricDataQueryProperty(**{
        'id': 'm1', 
        'return_data': False, 
        'metric_stat': {
          'metric': {
            'namespace': 'project', 
            'metric_name': 'gauge.project.value', 
            'dimensions': []
          }, 
          'period': 60, 
          'stat': 'Average'
        }
      })
    ]
  })

Will generate the expected, correct CF code in cdk.out. This is not great because we load the CF from a file and would like to not parse it any further if possible.

Additional Information/Context

When checking the CfnAlarmProps spec: https://docs.aws.amazon.com/cdk/api/v2/python/aws_cdk.aws_cloudwatch/CfnAlarmProps.html
I would interpret the type for metrics (Union[IResolvable, Sequence[Union[IResolvable, MetricDataQueryProperty, Dict[str, Any]]], None]) to allow for Dicts. Maybe I just misinterpeted that?

CDK CLI Version

2.99.1 (build b2a895e)

Framework Version

No response

Node.js Version

18.16.0

OS

Ubuntu 20.04.6 LTS (in WSL on Win 11)

Language

Python

Language Version

Python 3.11

Other information

No response

@SC-CTS SC-CTS added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 2, 2023
@github-actions github-actions bot added the @aws-cdk/aws-cloudwatch Related to Amazon CloudWatch label Oct 2, 2023
@SC-CTS SC-CTS changed the title aws_cdk.aws_cloudwatch: CfnAlarm drops Metrics object contents if arguments come from Dict aws_cdk.aws_cloudwatch: CfnAlarm drops MetricDataQueryProperty object contents if arguments come from a dict Oct 2, 2023
@SC-CTS SC-CTS changed the title aws_cdk.aws_cloudwatch: CfnAlarm drops MetricDataQueryProperty object contents if arguments come from a dict aws_cdk.aws_cloudwatch: CfnAlarm drops MetricDataQueryProperty contents if arguments come from a dict Oct 2, 2023
@khushail khushail added needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. labels Oct 2, 2023
@khushail
Copy link
Contributor

khushail commented Oct 2, 2023

Hi @SC-CTS ,thanks for reaching out.

Going through the available documentation about Metrics, it expects the array objects to be of type CfnAlarm.MetricDataQueryProperty as given in this example.. So when you explicitly added this and corrected the code, it produced the correct CF template.

And you also mentioned about using CF from file, so using Cloudformation Include module might help solve your usecase. Let me know if it does not help.

@khushail khushail added guidance Question that needs advice or information. and removed bug This issue is a bug. investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Oct 2, 2023
@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed guidance Question that needs advice or information. labels Oct 4, 2023
@github-actions
Copy link

github-actions bot commented Oct 5, 2023

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudwatch Related to Amazon CloudWatch closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

3 participants