adding update support for labels on google_logging_metric

enricojonas commented 3 years 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.0.5
on linux_amd64
+ provider registry.terraform.io/hashicorp/google v3.81.0

Affected Resource(s)

google_logging_metric

Terraform Configuration Files

Before

resource "google_logging_metric" "test_metric" {
  project = var.gcp_project
  name    = "testMetric"

  filter = <<-EOT
        resource.type="cloud_run_revision"
    EOT

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

    labels {
      key        = "operationName"
      value_type = "STRING"
    }

  }

  label_extractors = {
    "operationName" = "EXTRACT(jsonPayload.apiOperationName)"
  }

}

After adding an additional metric_descriptor label and label_extractor "clientApplicationName"

resource "google_logging_metric" "test_metric" {
  project = var.gcp_project
  name    = "testMetric"

  filter = <<-EOT
        resource.type="cloud_run_revision"
    EOT

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

    labels {
      key        = "operationName"
      value_type = "STRING"
    }

    labels {
      key        = "clientApplicationName"
      value_type = "STRING"
    }
  }

  label_extractors = {
    "operationName" = "EXTRACT(jsonPayload.apiOperationName)"
    "clientApplicationName"    = "EXTRACT(jsonPayload.clientApplicationName)"
  }

}

Debug Output

N/A

Panic Output

N/A

Expected Behavior

Terraform should update the resource in-place. This will preserve historical data for this metric. You can modify the metric in the UI, preserving the data.

Actual Behavior

Terraform is re-creating the resource and the metric's historical data is lost.

Terraform will perform the following actions:

  # google_logging_metric.test_metric must be replaced
-/+ resource "google_logging_metric" "test_metric" {
      ~ id               = "testMetric" -> (known after apply)
      ~ label_extractors = {
          + "clientApplicationName" = "EXTRACT(jsonPayload.clientApplicationName)"
            # (1 unchanged element hidden)
        }
        name             = "testMetric"
        # (2 unchanged attributes hidden)

      ~ metric_descriptor {
            # (3 unchanged attributes hidden)

          + labels { # forces replacement
              + key        = "clientApplicationName"
              + value_type = "STRING"
            }
            # (1 unchanged block hidden)
        }
    }

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

Steps to Reproduce

Create a google_logging_metric resource
Attempt to add a label_extractor and metric_descriptor label as in above example config
Observe the provider error as above

Important Factoids

N/A

References

N/A

edwardmedia commented 3 years ago

@enricojonas Terraform sees you were changing the resource by adding an attribute. To compare, you can see the result by updating the key or value_type. I don't think this can be fixed. Does this make sense?

    labels {
      key        = "operationName" ---> "changed"
      value_type = "STRING"
    }

enricojonas commented 3 years ago

@edwardmedia thanks for looking into this. Yes, since you need to add the attibute as well. If you do it via UI it looks like this:

Before:

Adding the label:

Metric updated, data preserved:

After saving, terraform is detecting correctly the same change that I try to achieve via TF code and says it's up to date.

google_logging_metric.test_metric: Refreshing state... [id=testMetric]

Note: Objects have changed outside of Terraform

Terraform detected the following changes made outside of Terraform since the last "terraform apply":

  # google_logging_metric.test_metric has been changed
  ~ resource "google_logging_metric" "test_metric" {
        id               = "testMetric"
      ~ label_extractors = {
          + "clientApplicationName" = "EXTRACT(jsonPayload.clientApplicationName)"
            # (1 unchanged element hidden)
        }
        name             = "testMetric"
        # (2 unchanged attributes hidden)

      ~ metric_descriptor {
            # (3 unchanged attributes hidden)

          + labels {
              + key        = "clientApplicationName"
              + value_type = "STRING"
            }
            # (1 unchanged block hidden)
        }
    }

Unless you have made equivalent changes to your configuration, or ignored the relevant attributes using
ignore_changes, the following plan may include actions to undo or respond to these changes.

──────────────────────────────────────────────

No changes. Your infrastructure matches the configuration.

What do you think?

edwardmedia commented 3 years ago

adding update support to a field/resource is not a bug, although fixing a broken update method is. Changed the label to enhancement accordingly

melinath commented 2 years ago

Note that the API doesn't actually support an "update" operation, but it seems like "creating" something that already exists sort of updates it? This is how we currently handle updating the labels field to e.g. add new labels

melinath commented 2 years ago

b/239950421

mattalbr commented 1 year ago

FWIW, calling this an "enhancement" seems disingenuous to me. I think an accurate description of the bug would be "google_cloud_logging loses historical data upon adding new labels".

tm185187 commented 1 year ago

This is an issue, from console you can add a labels after creating a metric, but with terraform we are unable to add a label after creation of log based metric, this leads to metric update in place. This leads to a large issue if any metric is referenced by an alerting policy, it will not let you delete it.

mouyang commented 11 months ago

Note that the API doesn't actually support an "update" operation, but it seems like "creating" something that already exists sort of updates it? This is how we currently handle updating the labels field to e.g. add new labels

+1 @tm185187

@enricojonas isn't trying to update the resource (MetricDescriptor) in the link above directly. They are trying to add a label to an existing LogMetric's MetricDescriptor (https://cloud.google.com/logging/docs/reference/v2/rest/v2/projects.metrics), and LogMetric does support the update operation. I was able to add a label through the update operation in the API and therefore I should be able to do this through Terraform as well.

tm185187 commented 11 months ago

@mouyang Nope, GCP API does support update operation, but Terraform doesn't support. You can reproduce in your local and let us know!

roaks3 commented 9 months ago

This is a duplicate of https://github.com/hashicorp/terraform-provider-google/issues/14998. There is recent discussion on both issues, so we can keep this open for reference until it is resolved, but please try post any new comments on https://github.com/hashicorp/terraform-provider-google/issues/14998 (it is being actively looked at by the service team).

hashicorp / terraform-provider-google