terraform-google-modules / terraform-google-slo

Creates SLOs on Google Cloud from custom Stackdriver metrics capability to export SLOs to Google Cloud services and other systems
https://registry.terraform.io/modules/terraform-google-modules/slo/google
Apache License 2.0
63 stars 29 forks source link

Error when using SLO with Backend Prometheus #42

Closed svenmueller closed 3 years ago

svenmueller commented 3 years ago

I get an error when using an SLO with backend class "Prometheus".

---
service_name:      product
feature_name:      search
slo_name:          freshness
slo_description:   95% of messages in queue are fresh
slo_target:        0.95
backend:
  class:           Prometheus
  method:          good_bad_ratio
  url:             ${prometheus_url}
  measurement:
    filter_good:  <query-1>
    filter_valid:  <query-2>
exporters:
- class:           Stackdriver
  project_id:      ${stackdriver_host_project_id}

Error when running TF (missing field changes with each run):

Error: Invalid value for module argument

  on main.tf line 130, in module "product_search_freshness":
 130:   config                     = local.slo_config_map["product-search-freshness"]

The given value is not suitable for child module variable "config" defined at
.terraform/modules/product_search_freshness/modules/slo/variables.tf:41,1-18:
attribute "slo_target": number required.

Somehow looks like there is an issue with the YAML? When i output the configmap in TF, i can see the SLO with all it's fields from the file.

output "product_search_freshness" {
  value = local.slo_config_map["product-search-freshness"]
}

TF output:

product_search_freshness = {
  "backend" = {
    "class" = "Prometheus"
    "measurement" = {
      "filter_good" = "query-1"
      "filter_valid" = "query-2"
    }
    "method" = "good_bad_ratio"
    "url" = "https://prometheus.com/"
  }
  "exporters" = [
    {
      "class" = "Pubsub"
      "project_id" = "my-project-id"
      "topic_name" = "slo-export-topic"
    },
  ]
  "feature_name" = "search"
  "service_name" = "product"
  "slo_description" = "95% of messages in queue are fresh"
  "slo_name" = "freshness"
  "slo_target" = 0.95
}

Any idea what can be wrong here?

Thx Sven

ocervell commented 3 years ago

Does it run with the slo-generator CLI ? It seems that it's a malformed YAML but I'm don't see anything wrong with it. Which version of TF are you using ?

palladius commented 3 years ago
The given value is not suitable for child module variable "config" defined at
.terraform/modules/product_search_freshness/modules/slo/variables.tf:41,1-18:
attribute "slo_target": number required.

Strange indeed, 0.95 should be parsed as float from YAML. Just for troubleshooting, have you tried changing to 1?

https://symfony.com/doc/current/components/yaml/yaml_format.html#numbers

Maybe TF somehow converts the float to string?

ocervell commented 3 years ago

@palladius weird, the examples in https://github.com/terraform-google-modules/terraform-google-slo/blob/master/examples/slo-generator/yaml_example/templates/ use floats and they're converting okay.

ocervell commented 3 years ago

Ok, found the error: it seems like the backend block requires a project_id field, and Terraform is trying to enforce that. Please add a project_id field to your backend section and it should work:

---
service_name:      product
feature_name:      search
slo_name:          freshness
slo_description:   95% of messages in queue are fresh
slo_target:        0.95
backend:
  class:           Prometheus
  method:          good_bad_ratio
  project_id:      test
  url:             ${prometheus_url}
  measurement:
    filter_good:  <query-1>
    filter_valid:  <query-2>
exporters:
- class:           Stackdriver
  project_id:      ${stackdriver_host_project_id}

Since Prometheus does not have a project_id per say, we need to remove this requirement for this type of backend.

It's worth noting that Terraform error is really wrong here, and that there is nothing wrong with the YAML or the slo_target number ...

ocervell commented 3 years ago

This PR is the fix to avoid having to specify the useless project_id: https://github.com/terraform-google-modules/terraform-google-slo/pull/43

svenmueller commented 3 years ago

When we add a dummy key project_id in the Terrfaform SLO configuration template, we can execute the plan/apply step. But now we see an issue when the cloud-function gets triggered and runs slo-generator to generate the report, we see the following issue in the log evens ("unexpected keyword argument 'project_id'"):

Traceback (most recent call last): File "/env/local/lib/python3.7/site-packages/google/cloud/functions/worker_v1.py", line 402, in run_background_function _function_handler.invoke_user_function(event_object) File "/env/local/lib/python3.7/site-packages/google/cloud/functions/worker_v1.py", line 222, in invoke_user_function return call_user_function(request_or_event) File "/env/local/lib/python3.7/site-packages/google/cloud/functions/worker_v1.py", line 219, in call_user_function event_context.Context(**request_or_event.context)) File "/user_code/main.py", line 46, in main do_export=True) File "/env/local/lib/python3.7/site-packages/slo_generator/compute.py", line 56, in compute delete=delete) File "/env/local/lib/python3.7/site-packages/slo_generator/report.py", line 84, in __init__ result = self.run_backend(config, client=client, delete=delete) File "/env/local/lib/python3.7/site-packages/slo_generator/report.py", line 163, in run_backend instance = utils.get_backend_cls(cls)(client=client, **backend_cfg) TypeError: __init__() got an unexpected keyword argument 'project_id'
ocervell commented 3 years ago

Ah okay, so the PR above will fix that too, a new minor version will be released sometimes tomorrow or Thursday including other updates as well.

ocervell commented 3 years ago

@svenmueller the new version 0.2.1 is released. You won't need the project_id in the YAML's backend section anymore and it should work okay. Let me know !