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

[Bug]: aws_cloudwatch_metric_alarm with expression throws: "period": conflicts with metric_query #29398

Closed
cmihail opened this issue Feb 14, 2023 · 6 comments · Fixed by #29896
Assignees
Labels
bug Addresses a defect in current functionality. service/cloudwatch Issues and PRs that pertain to the cloudwatch service.
Milestone

Comments

@cmihail
Copy link

cmihail commented Feb 14, 2023

Terraform Core Version

1.3.1

AWS Provider Version

4.32.0

Affected Resource(s)

aws_cloudwatch_metric_alarm

Expected Behavior

terraform apply with success

Actual Behavior

I get the error "period": conflicts with metric_query even though https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudwatch-alarm-metricdataquery.html mentions that period is now supported inside MetricDataQuery

Also using AWS Console I was able to create the alarm manually with the same expression as the one from above configuration.

The error seems to be due to period missing from

func flattenMetricAlarmMetrics(metrics []*cloudwatch.MetricDataQuery) []map[string]interface{} {

Relevant Error/Panic Output Snippet

No response

Terraform Configuration Files

resource "aws_cloudwatch_metric_alarm" "millis_behind" {
  alarm_name          = "MillisBehind alarm for ${var.app_name}"
  alarm_description   = "MillisBehind alarm for ${var.app_name}"
  comparison_operator = "GreaterThanOrEqualToThreshold"
  evaluation_periods  = "3"
  datapoints_to_alarm = "3"
  threshold           = 30000
  treat_missing_data  = "breaching"

  metric_query {
    id          = "mq-millis-behind"
    expression  = "SELECT MAX(MillisBehindLatest) FROM SCHEMA(\"${var.app_name}\", Operation, ShardId) WHERE Operation = 'ProcessTask'"
    period      = "60"
    return_data = "true"
  }
}

Steps to Reproduce

Define var.app_name using the name of a KCL kubernates application and then create a new CW alarm with period inside meric_query (see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudwatch-alarm-metricdataquery.html for documentation).

Debug Output

Error: Conflicting configuration arguments

  with module.metrics.aws_cloudwatch_metric_alarm.millis_behind,
  on ../../../../modules/kcl/cloudwatch.tf line 7, in resource "aws_cloudwatch_metric_alarm" "millis_behind":
   7:   period              = "60"

"period": conflicts with metric_query

Panic Output

No response

Important Factoids

No response

References

No response

Would you like to implement a fix?

None

@cmihail cmihail added bug Addresses a defect in current functionality. needs-triage Waiting for first response or review from a maintainer. labels Feb 14, 2023
@github-actions
Copy link

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

@github-actions github-actions bot added the service/cloudwatch Issues and PRs that pertain to the cloudwatch service. label Feb 14, 2023
@cmihail cmihail changed the title [Bug]: aws_cloudwatch_metric_alarm with expression throws ValidationError Period must not be null [Bug]: aws_cloudwatch_metric_alarm with expression throws ValidationError Period must not be null Feb 14, 2023
@cmihail cmihail changed the title [Bug]: aws_cloudwatch_metric_alarm with expression throws ValidationError Period must not be null [Bug]: aws_cloudwatch_metric_alarm with expression throws: "ValidationError Period must not be null" Feb 14, 2023
@cmihail cmihail changed the title [Bug]: aws_cloudwatch_metric_alarm with expression throws: "ValidationError Period must not be null" [Bug]: aws_cloudwatch_metric_alarm with expression throws: ""period": conflicts with metric_query" Feb 14, 2023
@cmihail cmihail changed the title [Bug]: aws_cloudwatch_metric_alarm with expression throws: ""period": conflicts with metric_query" [Bug]: aws_cloudwatch_metric_alarm with expression throws: "period": conflicts with metric_query Feb 14, 2023
@gdavison
Copy link
Contributor

gdavison commented Mar 9, 2023

HI @cmihail, thanks for submitting this. To clarify, is the error message that you're seeing the following?

Error: Unsupported argument

     on terraform_plugin_test.tf line 14, in resource "aws_cloudwatch_metric_alarm" "test":
     14:     period      = "60"
   
   An argument named "period" is not expected here.

That's what I see when I try the example code.

@cmihail
Copy link
Author

cmihail commented Mar 9, 2023

HI @cmihail, thanks for submitting this. To clarify, is the error message that you're seeing the following?

Error: Unsupported argument

     on terraform_plugin_test.tf line 14, in resource "aws_cloudwatch_metric_alarm" "test":
     14:     period      = "60"
   
   An argument named "period" is not expected here.

That's what I see when I try the example code.

Hey @gdavison . Yes, sorry, the error I get is:

Error: Unsupported argument

  on ../../../../modules/kcl/cloudwatch.tf line 23, in resource "aws_cloudwatch_metric_alarm" "millis_behind_critical":
  23:     period      = "60"

An argument named "period" is not expected here.

The previous error was probably from some experimenting code and I copy pasted the wrong error.

Though based on AWS docs period in metric query should work. I also tried it now with AWS CLI, and the following code worked:

$ aws cloudwatch put-metric-alarm  --region eu-west-1 --alarm-name "test" --evaluation-periods 3 --comparison-operator GreaterThanThreshold --threshold 10000 --metrics '[{"Id": "test", "Period": 60, "ReturnData": true, "Expression": "SELECT MAX(MillisBehindLatest) FROM SCHEMA(\"app-name\", Operation,ShardId)"}]'

@gdavison gdavison self-assigned this Mar 9, 2023
@Jetbo
Copy link

Jetbo commented Mar 9, 2023

@gdavison I can provide some additional context to this bug. Tested with 4.57.1

Say I manually make a CloudWatch Alarm test with this expression that uses a custom Metric:

SELECT SUM(AuthorizationFailureCount) FROM SCHEMA(CloudTrailMetrics, accountId,region)

I can import this alarm and show that state as:

❯ terraform import aws_cloudwatch_metric_alarm.test test
❯ terraform state show aws_cloudwatch_metric_alarm.test
# aws_cloudwatch_metric_alarm.test:
resource "aws_cloudwatch_metric_alarm" "test" {
    actions_enabled           = true
    alarm_actions             = []
    alarm_description         = "test"
    alarm_name                = "test"
    arn                       = "arn:aws:cloudwatch:us-east-1:accountId:alarm:test"
    comparison_operator       = "GreaterThanOrEqualToThreshold"
    datapoints_to_alarm       = 1
    dimensions                = {}
    evaluation_periods        = 1
    id                        = "test"
    insufficient_data_actions = []
    ok_actions                = []
    period                    = 0
    tags                      = {}
    tags_all                  = {}
    threshold                 = 1
    treat_missing_data        = "notBreaching"

    metric_query {
        expression  = "SELECT SUM(AuthorizationFailureCount) FROM SCHEMA(CloudTrailMetrics, accountId,region)"
        id          = "e1"
        label       = "AuthorizationFailureCount"
        return_data = true
    }
}

Notice period is set to 0. But when I try to make this exact same resource instead of importing it, I get this error:


│ Error: Conflicting configuration arguments
│ 
│   with aws_cloudwatch_metric_alarm.test,
│   on cloudwatch.tf line 188, in resource "aws_cloudwatch_metric_alarm" "test":
│  188:   period                    = 0
│ 
│ "period": conflicts with metric_query

If I remove period, I get this error instead:

│ Error: Updating metric alarm failed: ValidationError: Period must not be null
│       status code: 400, request id: ...

I would also like to note that with a Metrics Insights query alarm, the period must be between 1 minute and 3 hours. So I have this alarm set to a period of 60 seconds in the AWS Dashboard, but Terraform imported it as 0 for unknown reasons.

@github-actions github-actions bot removed the needs-triage Waiting for first response or review from a maintainer. label Mar 15, 2023
@github-actions github-actions bot added this to the v4.59.0 milestone Mar 15, 2023
@github-actions
Copy link

This functionality has been released in v4.59.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/cloudwatch Issues and PRs that pertain to the cloudwatch service.
Projects
None yet
3 participants