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

Add Cloud Run SLO support #11935

Closed
tiagojsag opened this issue Jun 22, 2022 · 8 comments
Closed

Add Cloud Run SLO support #11935

tiagojsag opened this issue Jun 22, 2022 · 8 comments
Labels
enhancement new-resource size/m tpgtools Issues related to the tpgtools generator
Milestone

Comments

@tiagojsag
Copy link

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 the 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 the issue is assigned to a user, that user is claiming responsibility for the issue. If the issue is assigned to "hashibot", a community member has claimed the issue already.

Description

According to these release notes, Cloud Run recently added support for SLOs. If these are supported already by the current TF resources, it's not really clear how, and I suspect that at least a new google_monitoring_cluster_cloud_run_service data source would be necessary (or at least very convenient). There might be additional needs on new/existing data sources/resources - this is my first time configuring SLOs, so newb alert/disclaimer.

New or Affected Resource(s)

  • google_monitoring_cluster_cloud_run_service - a data source to reference a Cloud Run service on an SLO, similar to what is already done in google_monitoring_app_engine_service, for example.

Potential Terraform Configuration

# Context data
data "google_cloud_run_service" "run-service" {
  name = "my-service"
  location = "us-central1"
}

# the actual new data source
data "google_monitoring_cluster_cloud_run_service" "srv" {
  service_id = data.google_cloud_run_service.run-service.id
}

References

  • #0000
@rileykarson rileykarson added tpgtools Issues related to the tpgtools generator new-resource size/m labels Jun 27, 2022
@rileykarson rileykarson added this to the Goals milestone Jun 27, 2022
@rileykarson
Copy link
Collaborator

Note: I haven't looked at the API yet, but this sounds like it may be a resource rather than a datasource

@jakubsacha
Copy link

Hey,

I've been looking at this today a bit, and you're right. Monitoring service needs to be created so it's resource not a data source.

Regarding the name, shouldn't it be google_monitoring_cloud_run_service instead of google_monitoring_cluster_cloud_run_service? Maybe even google_monitoring_service that supports all possible cases. That would reflect google API a bit better. Google docs states it supports various types of monitoring services:

  • "custom":
  • "appEngine":
  • "clusterIstio":
  • "meshIstio":
  • "istioCanonicalService":
  • "cloudRun":
  • "gkeNamespace":
  • "gkeWorkload":
  • "gkeService":

@etruong42
Copy link

There's an internal Google ticket (b/234447582) about this that I am actively working on.

Preview: jakubsacha's choice of google_monitoring_service matches our design choice as well!

We currently have google_monitoring_custom_service, but we have found that it would be more useful for users to create all sorts of "service" resources to support SLOs in Cloud Run and all the other aforementioned services.

@jakubsacha
Copy link

Thanks for a feedback. Please let me know in case I can support you.

etruong42 added a commit to etruong42/magic-modules that referenced this issue Sep 27, 2022
Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

hashicorp/terraform-provider-google#11935
slevenick pushed a commit to GoogleCloudPlatform/magic-modules that referenced this issue Oct 14, 2022
* Support ability to create "non-custom" services

Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

hashicorp/terraform-provider-google#11935

* Specify BasicService is immutable

* Responding to review comments.

* Add test; make service_id required

* labels still need input: true; remove encoder

* Correct typo

* Ignore service fields in SLO import test

* Correct test

* Make id_format and import_format the same

* Remove custom code

* Correct test typo

* service is actually input-only

* Add resource test
modular-magician added a commit to modular-magician/docs-examples that referenced this issue Oct 14, 2022
* Support ability to create "non-custom" services

Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

hashicorp/terraform-provider-google#11935

* Specify BasicService is immutable

* Responding to review comments.

* Add test; make service_id required

* labels still need input: true; remove encoder

* Correct typo

* Ignore service fields in SLO import test

* Correct test

* Make id_format and import_format the same

* Remove custom code

* Correct test typo

* service is actually input-only

* Add resource test

Signed-off-by: Modular Magician <magic-modules@google.com>
modular-magician added a commit to terraform-google-modules/docs-examples that referenced this issue Oct 14, 2022
* Support ability to create "non-custom" services

Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

hashicorp/terraform-provider-google#11935

* Specify BasicService is immutable

* Responding to review comments.

* Add test; make service_id required

* labels still need input: true; remove encoder

* Correct typo

* Ignore service fields in SLO import test

* Correct test

* Make id_format and import_format the same

* Remove custom code

* Correct test typo

* service is actually input-only

* Add resource test

Signed-off-by: Modular Magician <magic-modules@google.com>

Signed-off-by: Modular Magician <magic-modules@google.com>
modular-magician added a commit to modular-magician/terraform-provider-google-beta that referenced this issue Oct 14, 2022
* Support ability to create "non-custom" services

Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

hashicorp/terraform-provider-google#11935

* Specify BasicService is immutable

* Responding to review comments.

* Add test; make service_id required

* labels still need input: true; remove encoder

* Correct typo

* Ignore service fields in SLO import test

* Correct test

* Make id_format and import_format the same

* Remove custom code

* Correct test typo

* service is actually input-only

* Add resource test

Signed-off-by: Modular Magician <magic-modules@google.com>
modular-magician added a commit to modular-magician/terraform-provider-google that referenced this issue Oct 14, 2022
* Support ability to create "non-custom" services

Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

hashicorp#11935

* Specify BasicService is immutable

* Responding to review comments.

* Add test; make service_id required

* labels still need input: true; remove encoder

* Correct typo

* Ignore service fields in SLO import test

* Correct test

* Make id_format and import_format the same

* Remove custom code

* Correct test typo

* service is actually input-only

* Add resource test

Signed-off-by: Modular Magician <magic-modules@google.com>
modular-magician added a commit to hashicorp/terraform-provider-google-beta that referenced this issue Oct 14, 2022
* Support ability to create "non-custom" services

Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

hashicorp/terraform-provider-google#11935

* Specify BasicService is immutable

* Responding to review comments.

* Add test; make service_id required

* labels still need input: true; remove encoder

* Correct typo

* Ignore service fields in SLO import test

* Correct test

* Make id_format and import_format the same

* Remove custom code

* Correct test typo

* service is actually input-only

* Add resource test

Signed-off-by: Modular Magician <magic-modules@google.com>

Signed-off-by: Modular Magician <magic-modules@google.com>
modular-magician added a commit that referenced this issue Oct 14, 2022
* Support ability to create "non-custom" services

Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

#11935

* Specify BasicService is immutable

* Responding to review comments.

* Add test; make service_id required

* labels still need input: true; remove encoder

* Correct typo

* Ignore service fields in SLO import test

* Correct test

* Make id_format and import_format the same

* Remove custom code

* Correct test typo

* service is actually input-only

* Add resource test

Signed-off-by: Modular Magician <magic-modules@google.com>

Signed-off-by: Modular Magician <magic-modules@google.com>
modular-magician added a commit to modular-magician/terraform-validator that referenced this issue Oct 14, 2022
* Support ability to create "non-custom" services

Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

hashicorp/terraform-provider-google#11935

* Specify BasicService is immutable

* Responding to review comments.

* Add test; make service_id required

* labels still need input: true; remove encoder

* Correct typo

* Ignore service fields in SLO import test

* Correct test

* Make id_format and import_format the same

* Remove custom code

* Correct test typo

* service is actually input-only

* Add resource test

Signed-off-by: Modular Magician <magic-modules@google.com>
modular-magician added a commit to GoogleCloudPlatform/terraform-validator that referenced this issue Oct 14, 2022
* Support ability to create "non-custom" services

Allow users to create various Service Monitoring services: App Engine,
Cloud Run, etc.

hashicorp/terraform-provider-google#11935

* Specify BasicService is immutable

* Responding to review comments.

* Add test; make service_id required

* labels still need input: true; remove encoder

* Correct typo

* Ignore service fields in SLO import test

* Correct test

* Make id_format and import_format the same

* Remove custom code

* Correct test typo

* service is actually input-only

* Add resource test

Signed-off-by: Modular Magician <magic-modules@google.com>

Signed-off-by: Modular Magician <magic-modules@google.com>
@etruong42
Copy link

This has been released. Check out the documentation: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/monitoring_service

@rileykarson
Copy link
Collaborator

Closing, thanks! In the future if you use "Fixes {{issue}}" in a PR body, merging the PR will autoclose the issue.

@rileykarson
Copy link
Collaborator

Ah- @slevenick beat me to it.

@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 Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement new-resource size/m tpgtools Issues related to the tpgtools generator
Projects
None yet
Development

No branches or pull requests

5 participants