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_cloudwatch_log_metric_filter: default_value is automatically set to 0 #2384

Closed
iamam34 opened this issue Nov 20, 2017 · 14 comments · Fixed by #5933
Closed

aws_cloudwatch_log_metric_filter: default_value is automatically set to 0 #2384

iamam34 opened this issue Nov 20, 2017 · 14 comments · Fixed by #5933
Labels
bug Addresses a defect in current functionality. good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues.
Milestone

Comments

@iamam34
Copy link

iamam34 commented Nov 20, 2017

When I apply a terraform configuration that does NOT include default_value, terraform creates a log filter in AWS that has the default_value set to 0.

Terraform Version

Terraform v0.10.3 with aws provider v1.3.1

Affected Resource(s)

  • aws_cloudwatch_log_metric_filter

Terraform Configuration Files

resource "aws_cloudwatch_log_metric_filter" "my-function-memory-used" {
  name           = "MyFunction_MemoryUsed"
  pattern        = "[type=REPORT, ... , memory_used, unit]"
  log_group_name = "/aws/lambda/MyFunction"

  metric_transformation {
    name      = "MemoryUsed"
    namespace = "MyFunction.CustomMetrics"
    value     = "$memory_used"
  }
}

Expected Behavior

Creates a log filter with default_value not set, so that log lines that don't match the filter don't output any value for the metric. http://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/MonitoringLogData.html

Actual Behavior

Creates a log filter with default_value set to 0, so that every log line outputs a value for the metric (non-matching lines output 0).

Steps to Reproduce

  1. Write terraform for a log_filter WITHOUT a default_value
  2. terraform apply (note that the console output does not mention default_value at all)
    aws_cloudwatch_log_metric_filter.my-function-memory-used: Creating...
    [11:12:27][Step 2/2]   log_group_name:                    "" => "/aws/lambda/MyFunction"
    [11:12:27][Step 2/2]   metric_transformation.#:           "" => "1"
    [11:12:27][Step 2/2]   metric_transformation.0.name:      "" => "MemoryUsed"
    [11:12:27][Step 2/2]   metric_transformation.0.namespace: "" => "MyFunction.CustomMetrics"
    [11:12:27][Step 2/2]   metric_transformation.0.value:     "" => "$memory_used"
    [11:12:27][Step 2/2]   name:                              "" => "MyFunction_MemoryUsed"
    [11:12:27][Step 2/2]   pattern:                           "" => "[type=REPORT, ... , memory_used, unit]"
    
  3. Visit the AWS Console CloudWatch and see that your new metric has a minimum value of 0 and an unexpectedly low average.

References

@iamam34
Copy link
Author

iamam34 commented Nov 20, 2017

@stack72 @Ninir Since you were involved in the earlier work on this attribute, do you have any special insights?

@paddycarver paddycarver added the bug Addresses a defect in current functionality. label Nov 21, 2017
@Ninir
Copy link
Contributor

Ninir commented Dec 5, 2017

Hi @iamam34

I think I've found the issue (which should be easy to resolve by the way): this attribute is of type Float, which seem to imply that the value is always provided as 0 when not set... as the check deciding whether the value should be set in the API call is a check against "", the value is always sent... and as 0 is a valid value... I think we should just go with having a default to -1 or updating the type from float to string.

If someone wants to take over this, just go :) otherwise will fix it later :)

@Ninir Ninir added the easy label Dec 5, 2017
@iamam34
Copy link
Author

iamam34 commented Dec 10, 2017

Great to hear it's straightforward to fix!
Note: If set, the value must be a float. If not set, we need to not send the value (not even -1... but null might be ok; I haven't checked the AWS API docs).

Up to you whether a empty string is considered valid or invalid terraform syntax (but since it's not a float, we still don't want to send it).

@jocelynthode
Copy link

Hey I'm trying to work on this issue. As it is my first time working on Terraform and I'm really new to Go, I'll probably need guidance. I'll open a WIP PR shortly.

@FrederikNygaardSvendsen

Any news? This has unknowingly rendered our 'ResponseTime' metrics useless (since they are cluttered with zeros)

@jocelynthode
Copy link

I asked for help on my PR as I'm rather new to this however nobody answered.

@mr-olson
Copy link

@jocelynthode while you're waiting for review, would you mind checking whether the opposite case of going from unset to set is handled by your PR? Might require fiddling with different TF / aws provider versions.

I have several aws_cloudwatch_log_metric_filter resources that were created with TF < .11.1. None of them specified a default_value (since it didn't exist), and they're empty when viewed in the AWS console, as expected. Under .11.2 / aws 1.6 when I try to explicitly set to default_value = "0", TF doesn't see it as a change. Setting default_value = "1" does register as a change.

@skynardo
Copy link

skynardo commented Mar 2, 2018

I also have this issue. It seems that if I manually edit the Log Metric Filter via AWS Console and delete the zero, the Default Value field changes to "none". This fixes the issue as long as no changes are made to the resource via terraform (aws provider 1.6).

@paultyng paultyng added good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues. help wanted and removed good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues. help wanted labels Mar 5, 2018
@zytek
Copy link
Contributor

zytek commented May 30, 2018

Just got hit by this when creating alarm on SampleCount >=0. As there are two almost identical PRs open to address it please advise how we could move forward with fixing this issue.

@MooreDerek
Copy link

Have encountered this one as well. Would be good to know if a fix is on the way. The "-1" seems to be a good solution.

@sergeyk
Copy link

sergeyk commented Jun 5, 2018

Also having problems with this.

ahl pushed a commit to ahl/terraform-provider-aws that referenced this issue Sep 19, 2018
@bflad bflad added this to the v1.44.0 milestone Nov 13, 2018
bflad added a commit that referenced this issue Nov 13, 2018
Fixes #2384 aws_cloudwatch_log_metric_filter: default_value is automatically set to 0
@bflad
Copy link
Contributor

bflad commented Nov 13, 2018

The fix for this should be merged and will release with version 1.44.0 of the AWS provider, likely tomorrow. Apologies for the long delays here.

@bflad
Copy link
Contributor

bflad commented Nov 15, 2018

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

@ghost
Copy link

ghost commented Apr 2, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 2, 2020
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. good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues.
Projects
None yet