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

Use of logging module and not stdout. #24

Closed Djabx closed 4 years ago

Djabx commented 4 years ago

@ocervell

ocervell commented 4 years ago

Thanks ! Lint check is failing, could you please run make docker_test_lint and fix the output ?

Checking for documentation generation
Checking for trailing whitespace
Checking for missing newline at end of file
Error: No newline at end of file ./modules/slo/code/requirements.txt.tpl
Running shellcheck
Checking file headers
Running flake8
./modules/slo/code/main.py:39:80: E501 line too long (80 > 79 characters)
Running terraform fmt
Running terraform validate
terraform_validate ./examples/simple_example 
Success! The configuration is valid.

terraform_validate ./modules/slo 
Success! The configuration is valid.

terraform_validate ./modules/slo-pipeline 
Success! The configuration is valid.

terraform_validate ./test/fixtures/simple_example 
Success! The configuration is valid.

terraform_validate ./test/setup 
Success! The configuration is valid.

Error: The following tests have failed: check_whitespace check_python
Djabx commented 4 years ago

There is still some failure, but I can't access cloud build.

ocervell commented 4 years ago

Lint errors are fixed. There is an error while downloading the google-cloud-logging package (I think it's the quotes around the package name that errors out).

Step #2 - "converge":        Error: Error waiting for Creating CloudFunctions Function: Error code 3, message: Build failed: `pip_download_wheels` had stderr output:
Step #2 - "converge":        ERROR: Invalid requirement: '"google-cloud-logging"' (from line 2 of requirements.txt)
Step #2 - "converge":        
Step #2 - "converge":        error: `pip_download_wheels` returned code: 1; Error ID: 0A4F16D6
Step #2 - "converge":        

We should also add the logging role (roles/logging.logWriter) to our prerequisites section in the README.

ocervell commented 4 years ago

Thanks, we still need to grant the role roles/logging.logWriter to the GCF service account in iam.tf.

resource "google_project_iam_member" "logs-writer" {
  project = var.project_id
  role    = "roles/logging.logWriter"
  member  = "serviceAccount:${local.service_account_email}"
}
ocervell commented 4 years ago

Hey @Djabx , this is breaking logging for the new release, so I'm going to revert it. Basically we have duplicated logs showing up, and the logs sent with the client don't have the right severity field set, so they all show as either warning or error.