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

google_logging_metric should be updated instead of recreated when adding new label #14998

Open
pmontepagano opened this issue Jun 27, 2023 · 15 comments

Comments

@pmontepagano
Copy link

pmontepagano commented Jun 27, 2023

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

@edwardmedia edwardmedia self-assigned this Jun 27, 2023
@edwardmedia
Copy link
Contributor

similar to #13759

Change to enhancement

@edwardmedia edwardmedia removed their assignment Jun 27, 2023
@rileykarson rileykarson added this to the Goals milestone Jul 10, 2023
@pengq-google
Copy link

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
Copy link

mouyang commented Oct 23, 2023

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
Copy link

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
Copy link

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

@pengq-google
Copy link

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

@pengq-google
Copy link

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
Copy link

Likely the the behavior changed in 3.67.0,
caused by GoogleCloudPlatform/magic-modules#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
Copy link

ryantansey commented Apr 29, 2024

@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
Copy link

@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
Copy link

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
Copy link

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

@roaks3
Copy link
Collaborator

roaks3 commented Aug 30, 2024

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
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants