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 create fails if missing bucket_options.exponential_buckets.num_finite_buckets #12964

Closed
mattalbr opened this issue Nov 7, 2022 · 9 comments
Labels
breaking-change forward/linked persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work service/logging
Milestone

Comments

@mattalbr
Copy link

mattalbr commented Nov 7, 2022

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.3.2
on linux_arm64

  • provider registry.terraform.io/hashicorp/google v4.42.1
  • provider registry.terraform.io/hashicorp/google-beta v4.42.1
  • provider registry.terraform.io/hashicorp/random v3.4.3

Affected Resource(s)

  • google_logging_metric

Terraform Configuration Files

resource "google_logging_metric" "response_latency" {
  name   = "response_count"
  filter = "resource.type = \"cloud_run_revision\" AND resource.labels.configuration_name = \"fastapi\" AND jsonPayload.metric_id = \"/gaia/response_latency\" "
  metric_descriptor {
    metric_kind = "DELTA"
    value_type  = "DISTRIBUTION"
    unit        = "1"
    labels {
      key        = "response_code"
      value_type = "INT64"
    }
    labels {
      key        = "response_code_class"
      value_type = "INT64"
    }
    labels {
      key        = "path"
      value_type = "STRING"
    }
    labels {
      key        = "method"
      value_type = "STRING"
    }
    display_name = "Response Latency (Server)"
  }
  value_extractor = "EXTRACT(jsonPayload.metric_value)"
  label_extractors = {
    "response_code"       = "EXTRACT(jsonPayload.metric_labels.response_code)"
    "response_code_class" = "EXTRACT(jsonPayload.metric_labels.response_code_class)"
    "path"                = "EXTRACT(jsonPayload.metric_labels.path)"
    "method"              = "EXTRACT(jsonPayload.metric_labels.method)"
  }
  bucket_options {
    exponential_buckets {
      # no num_finite_buckets specified
      growth_factor = 1.2
      scale         = 1000000
    }
  }
}

Debug Output

https://gist.github.com/mattalbr/57f256d98431f1359f3d0d8d005ff27d

Panic Output

N/A

Expected Behavior

num_finite_buckets is listed as optional in the documentation, so I expect it to create successfully when not specified in the configuration

Actual Behavior

│ Error: Error creating Metric: googleapi: Error 400: Bucket options are invalid: Distribution |bucket_options.exponential_buckets| has a |num_finite_buckets| of 0, which is less than 1.

│ with google_logging_metric.response_latency,
│ on metrics.tf line 34, in resource "google_logging_metric" "response_latency":
│ 34: resource "google_logging_metric" "response_latency" ***

Steps to Reproduce

  1. terraform apply

Important Factoids

N/A

References

N/A

b/293328011

@mattalbr mattalbr added the bug label Nov 7, 2022
@edwardmedia edwardmedia self-assigned this Nov 7, 2022
@edwardmedia
Copy link
Contributor

edwardmedia commented Nov 8, 2022

Looks like num_finite_buckets, growth_factor and scale are all required. Currently AtLeastOneOf is applied on them. Has the API updated its rule?

bucket_options.0.linear_buckets could share the same rule.

@edwardmedia edwardmedia assigned megan07 and unassigned edwardmedia Nov 8, 2022
@megan07
Copy link
Contributor

megan07 commented Nov 8, 2022

Unfortunately, making these all required would be a breaking change. I'll mark this as such and we'll consider it for the next major release. Sorry about that!

@mattalbr
Copy link
Author

mattalbr commented Nov 8, 2022

Megan, to be clear, do you mean changing this to be required would be a breaking change? I think the intended, and documented, behavior is to be optional, but as it stands they are currently defacto required by code

@edwardmedia
Copy link
Contributor

@mattalbr currently you see the error when you apply the config. With the proposed change, it will be in the plan.

@mattalbr
Copy link
Author

mattalbr commented Nov 8, 2022

Got it! Huge fan of any validation errors going from apply -> plan, that's one of my biggest frustrations with some of the google/hashicorp integration points and would make the developer experience way better.

If we've decided that having them required is indeed the intended behavior though, would be great to at least update the documentation regardless of where the error happens.

@rileykarson rileykarson added this to the Future Major Release milestone Feb 1, 2023
@rileykarson rileykarson added persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work and removed bug labels Mar 6, 2023
@pengq-google
Copy link

I apologize for the confusion. The Advanced (Histogram buckets) section is optional. However, if you choose to configure this section, the number of finite buckets (num_finite_buckets) is required for both the Exponential and Linear types.

@c2thorn
Copy link
Collaborator

c2thorn commented Aug 17, 2023

@pengq-google Referring to the API documentation, if bucketOptions.exponentialBuckets is specified, numFiniteBuckets must be set? This would mean that it is a required field, and must be updated in Terraform.

@c2thorn
Copy link
Collaborator

c2thorn commented Sep 11, 2023

closed with GoogleCloudPlatform/magic-modules#8780

This will release with v5.0.0. Check #15582 for more details on the major release.

@c2thorn c2thorn closed this as completed Sep 11, 2023
@github-actions
Copy link

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change forward/linked persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work service/logging
Projects
None yet
Development

No branches or pull requests

6 participants