GoogleCloudPlatform / prometheus-engine

Google Cloud Managed Service for Prometheus libraries and manifests.
https://g.co/cloud/managedprometheus
Apache License 2.0
196 stars 92 forks source link

refactor: move monitoring and rules defaulters and validators to types #949

Closed TheSpiritXIII closed 6 months ago

TheSpiritXIII commented 7 months ago

Changes

  1. Merge rules package into rules type, and move rules validators to the rules type.

In my opinion, this is where rules generation should live. In my ideal world, types would be a separate package than the operator, and external tools could import the types package and validate them. Two examples of where this would be useful is a prometheus-operator converter (which would even be able to compare resulting Prometheus configurations this way) and a dedicated webhook package.

This is also consistent with the monitoring types. RuleGroupsConfig is analogous to ScrapeConfigs.

  1. Expand rules generation tests to validate that forbidden labels return an error.

  2. Move monitoring defaulters to inline in the types.

  3. Moved around monitoring types methods to be local to each class (e.g. PodMonitoring methods next to other PodMonitoring methods, instead of interleaved methods all over).

  4. The monitoring and rules validate methods no longer take dummy projects, names and locations. This makes the code less confusing.

TheSpiritXIII commented 6 months ago

I'm finding it a bit hard to split because the validation methods call the methods to generate the Prometheus configurations and moving all of those doesn't make sense to live in the validation file. The end result was that the _validation files were small because the Validate methods are only a single line. Instead I moved the generation methods to _config.go and kept the validate local to _types.go. Thoughts?