google_logging_metric should be updated instead of recreated when adding new label

pmontepagano commented 1 year ago

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

terraform apply

Important Factoids

References

0000

b/291300763

edwardmedia commented 1 year ago

Change to enhancement

pengq-google commented 1 year ago

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...

mouyang commented 11 months ago

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?

BlueTogepi commented 11 months ago

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 commented 11 months ago

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

pengq-google commented 11 months ago

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

pengq-google commented 11 months ago

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 commented 11 months ago

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!

ryantansey commented 5 months ago

@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.

pengq-google commented 5 months ago

@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!

ryantansey commented 4 months ago

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 commented 1 month ago

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

roaks3 commented 1 month ago

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.

pengq-google commented 1 month ago

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 commented 1 month ago

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

hashicorp / terraform-provider-google