terraform-provider-google icon indicating copy to clipboard operation
terraform-provider-google copied to clipboard

google_logging_metric should be updated instead of recreated when adding new label

Open pmontepagano opened this issue 1 year ago • 10 comments

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
  • If an issue is assigned to the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

Terraform v1.5.1
on darwin_arm64
+ provider registry.terraform.io/hashicorp/google v4.71.0

Affected Resource(s)

  • google_logging_metric

Terraform Configuration Files

The original resource was modified as shown below:

resource "google_logging_metric" "logging_error" {
  filter = "resource.type=\"gae_app\"\nlogName=\"projects/my-project/logs/app\")\n\"_error:\"\n"

  label_extractors = {
    reason = "REGEXP_EXTRACT(protoPayload.line.logMessage, \"_error: reason=(.+) error_id=\")"
+   reason_v2 = "REGEXP_EXTRACT(textPayload, \"_error: reason=(.+) error_id=\")"
  }

  metric_descriptor {
    labels {
      key        = "reason"
      value_type = "STRING"
    }
+  labels {
+    key    = "reason_v2"
+    value_type = "STRING"
+  }
    
    metric_kind = "DELTA"
    unit        = "1"
    value_type  = "INT64"
  }

  name    = "logging_error"
  project = "my-project"
}

Expected Behavior

Terraform should have modified the metric without recreating.

Actual Behavior

Terraform will perform the following actions:

  # google_logging_metric.logging_error must be replaced
-/+ resource "google_logging_metric" "logging_error" {
      - disabled         = false -> null
      ~ id               = "logging_error" -> (known after apply)
      ~ label_extractors = {
          + "reason_v2" = "REGEXP_EXTRACT(textPayload, \"_error: reason=(.+) error_id=\")"
            # (1 unchanged element hidden)
        }
        name             = "logging_error"
        # (1 unchanged attribute hidden)

      ~ metric_descriptor {
            # (3 unchanged attributes hidden)

          + labels { # forces replacement
              + key        = "reason_v2"
              + value_type = "STRING"
            }

            # (1 unchanged block hidden)
        }

      - timeouts {}
    }

Plan: 1 to add, 0 to change, 1 to destroy.

Steps to Reproduce

  1. terraform apply

Important Factoids

References

  • #0000

b/291300763

pmontepagano avatar Jun 27 '23 21:06 pmontepagano

similar to https://github.com/hashicorp/terraform-provider-google/issues/13759

Change to enhancement

edwardmedia avatar Jun 27 '23 21:06 edwardmedia

I tried to reproduce the error. However my log metric is update in-place

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # google_logging_metric.tf_project_metric will be updated in-place
  ~ resource "google_logging_metric" "tf_project_metric" {
        filter           = "severity>=ERROR"
        id               = "tf-projectmetric"
      ~ label_extractors = {
          + "reason_v2" = "REGEXP_EXTRACT(textPayload, \"_error: reason=(.+) error_id=\")"
            "sku"       = "REGEXP_EXTRACT(protoPayload.line.logMessage, \"_error: reason=(.+) error_id=\")"
        }
        name             = "tf-projectmetric"
        project          = myproject

      ~ metric_descriptor {
            metric_kind = "DELTA"
            unit        = "1"
            value_type  = "INT64"

          + labels {
              + key        = "reason_v2"
              + value_type = "STRING"
            }
            labels {
                description = "Identifying number for item"
                key         = "sku"
                value_type  = "INT64"
            }
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

I am still investigating...

pengq-google avatar Aug 10 '23 04:08 pengq-google

Also similar to #9921.

Thank you @pengq-google for investigating this issue. Which version of the Google provider are you using, same is OP (4.71.0) or a newer one?

mouyang avatar Oct 23 '23 18:10 mouyang

I also experience this issue, I'm setting the lifecycle.prevent_destroy to true and terraform plan does not let me add more labels, it says "...the plan calls for this resource to be destroyed. ..."

I'm using terraform as the following version: Terraform v1.5.7 on darwin_arm64

  • provider registry.terraform.io/hashicorp/google v5.1.0

ps. I noticed that the new version is available (v1.6.2) but haven't test this on the new version yet, going to try updating when I'm at places with good internet.

BlueTogepi avatar Oct 24 '23 15:10 BlueTogepi

I've tested with v1.6.2, the problem still persists

BlueTogepi avatar Oct 25 '23 09:10 BlueTogepi

Thanks all for the datapoints. I'll keep investigating.

pengq-google avatar Oct 25 '23 13:10 pengq-google

Also similar to #9921.

Thank you @pengq-google for investigating this issue. Which version of the Google provider are you using, same is OP (4.71.0) or a newer one?

I used v3.46.0 it updated in place.

  # google_logging_metric.tf_project_metric will be updated in-place
  ~ resource "google_logging_metric" "tf_project_metric" {
        filter           = "severity>=ERROR"
        id               = "codelab-terraform-pengq tf-projectmetric"
        label_extractors = {
            "reason_v2"        = "REGEXP_EXTRACT(textPayload, \"_error: reason=(.+) error_id=\")"
            "sku"              = "REGEXP_EXTRACT(textPayload, \"_error: reason=(.+) error_id=\")"
            "sku_force_change" = "REGEXP_EXTRACT(protoPayload.line.logMessage, \"_error: reason=(.+) error_id=\")"
        }
        name             = "tf-projectmetric"
        project          = "codelab-terraform-pengq"

      ~ metric_descriptor {
            metric_kind = "DELTA"
            unit        = "1"
            value_type  = "INT64"

          - labels {
              - key        = "reason_v2" -> null
              - value_type = "STRING" -> null
            }
            labels {
                description = "sku"
                key         = "sku"
                value_type  = "INT64"
            }
          + labels {
              + description = "sku_force_change"
              + key         = "sku_force_change - 1"
              + value_type  = "INT64"
            }
          - labels {
              - description = "sku_force_change" -> null
              - key         = "sku_force_change" -> null
              - value_type  = "INT64" -> null
            }
          + labels {
              + key        = "reason_v2"
              + value_type = "STRING"
            }
        }
    }

  # google_logging_project_bucket_config.tf_bucket_beta will be created
  + resource "google_logging_project_bucket_config" "tf_bucket_beta" {
      + bucket_id       = "tf_bucket_beta"
      + description     = "tf_bucket_beta"
      + id              = (known after apply)
      + lifecycle_state = (known after apply)
      + location        = "global"
      + name            = (known after apply)
      + project         = "projects/codelab-terraform-pengq"
      + retention_days  = 30
    }

However when changed to v4.82.0 it is replacing.

  # google_logging_metric.tf_project_metric must be replaced
-/+ resource "google_logging_metric" "tf_project_metric" {
      - disabled         = false -> null
        filter           = "severity>=ERROR"
      ~ id               = "codelab-terraform-pengq tf-projectmetric" -> (known after apply)
        label_extractors = {
            "reason_v2"        = "REGEXP_EXTRACT(textPayload, \"_error: reason=(.+) error_id=\")"
            "sku"              = "REGEXP_EXTRACT(textPayload, \"_error: reason=(.+) error_id=\")"
            "sku_force_change" = "REGEXP_EXTRACT(protoPayload.line.logMessage, \"_error: reason=(.+) error_id=\")"
        }
        name             = "tf-projectmetric"
        project          = "codelab-terraform-pengq"

      ~ metric_descriptor {
            metric_kind = "DELTA"
            unit        = "1"
            value_type  = "INT64"

            labels {
                key        = "reason_v2"
                value_type = "STRING"
            }
            labels {
                description = "sku"
                key         = "sku"
                value_type  = "INT64"
            }
          + labels { # forces replacement
              + description = "sku_force_change"
              + key         = "sku_force_change - 1"
              + value_type  = "INT64"
            }
          - labels { # forces replacement
              - description = "sku_force_change" -> null
              - key         = "sku_force_change" -> null
              - value_type  = "INT64" -> null
            }
        }
    }

I am asking our Terraform team for more details.

pengq-google avatar Oct 31 '23 20:10 pengq-google

Likely the the behavior changed in 3.67.0, caused by https://github.com/GoogleCloudPlatform/magic-modules/pull/4734

I will double check with our team to see if the behavior is work as intended, if not, will update it in new pr.

Thanks for all your patience!

pengq-google avatar Nov 01 '23 21:11 pengq-google

@pengq-google @roaks3 Is there any movement on this? Not being able to edit and add new labels to existing metrics via terraform is a huge problem for us. We work around it by duplicating it and renaming it + updating all alert/dash references, but it deletes history due to being a new metric.

It severely disrupts that continuity and our ability to respond to changes necessary in a live production environment. Everything is done through IaC so having this addressed would be a huge help.

ryantansey avatar Apr 29 '24 19:04 ryantansey

@ryantansey sorry I just saw this. I can confirm it also happens to me and I'll add this ticket to my next sprint. I'll keep you updated!

pengq-google avatar May 09 '24 03:05 pengq-google

Hi @pengq-google, just wanted to check in on this (not sure how long your sprints are/where you were at in the previous one). Thank you for your time.

ryantansey avatar May 31 '24 16:05 ryantansey

Anyone still around at Google that is working on this? @rileykarson

ryantansey avatar Aug 30 '24 13:08 ryantansey

Given the amount of time since the last update, my guess is that this was superseded by other priorities.

@pengq-google is this in a spot where we should at least re-open the internal bug b/291300763, to make sure it is still being tracked? It had been closed previously due to missing API support.

roaks3 avatar Aug 30 '24 13:08 roaks3

This is happening because of some implementation details. We can't fix it without a big refactoring, and it is on our roadmap, but need time to make it happen. I know seeing 'replaced' instead of 'update' can be alarming, but we guarantee no data loss here.

pengq-google avatar Aug 30 '24 13:08 pengq-google

And yes, we are aware of it, and have ticket tracking it. Sorry for the inconvenience!

pengq-google avatar Aug 30 '24 13:08 pengq-google