Add support for custom variables in metric templates

[njohnstone2]

This PR is in response to #1351. The aim here is to provide the ability to support the use of custom variables on metric templates.

Templating lets us define common properties in a single location. It feels like a logical extension of the metricTemplate resource to support a common query that uses dynamic metric labels.

Use Case

In my case i have a large number of deployments that i would like to implement canary deployments on (e.g. 300). These deployments can be broken down into several types that share common KPI's.

I am already using helm to manage the templating of these projects so it is possible to template the query directly on the canary resource.

However this feels like an anti pattern since all properties of the metricTemplate would need to be applied to all canary resources (e.g. metrics_url, auth secret, query). Ideally I would like to use the metricTemplate CRD to define the common properties of each metrics grouping in a single location as opposed to each canary definition.

[aryan9600]

thanks for opening this PR! it'd also be nice to have some tests for observers.RenderQuery() since that deals with the actual templating and it has zero test coverage at the moment. would you like to try and add them? no need to add tests for every edge case, just a few cases should be a good start.

[njohnstone2]

@aryan9600 thanks for the feedback. I've added tests to cover some basic scenarios for RenderQuery()

[codecov-commenter]

Codecov Report

Base: 54.37% // Head: 54.41% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (e53fffc) compared to base (e7d8ade).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head e53fffc differs from pull request most recent head e92286f. Consider uploading reports for the commit e92286f to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1355      +/-   ##
==========================================
+ Coverage   54.37%   54.41%   +0.04%     
==========================================
  Files          84       84              
  Lines       10044    10047       +3     
==========================================
+ Hits         5461     5467       +6     
+ Misses       3927     3925       -2     
+ Partials      656      655       -1     

Impacted Files	Coverage Δ
pkg/controller/scheduler_metrics.go	37.64% <100.00%> (+0.74%)	⬆️
pkg/metrics/observers/render.go	62.50% <100.00%> (+18.75%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[njohnstone2]

Is there a problem with the OSM e2e test? It failed to fetch the tag_name of the latest release. Checking the api response does include tag_name so it may just need a retry.

Failed to get latest release information: tag_name key not found in latest release version information from https://api.github.com/repos/openservicemesh/osm/releases/latest

[aryan9600]

@njohnstone2 a couple of things need to be done and then we can merge this PR:

1. e2e tests: we have e2e tests for all the providers and some of them (eg: istio) use custom MetricTemplate objects. please update them to use templateVaribales.
2. docs: please update docs explaining the usage of templateVariables with examples.
3. please squash all commits into 3-4 meaningful commits with appropriate description so that the git history looks clean

thanks!

[njohnstone2]

@aryan9600 thanks for the feedback! I have updated the e2e tests that use MetricTemplates and tidied up the commits. As for the documentation I have only updated the Custom Metrics section that you linked in your comment.

This is to keep tutorials and other examples using the minimum required config to define the metric template and avoid adding complexity. Please let me know if you don't agree with this and I can update the rest of the references to MetricTemplates

[njohnstone2]

The osm e2e test should be fixed now.

[aryan9600]

just a few minor nitpicks in the docs, but othwerwise its looking good!
@stefanprodan could you also take a final look at the docs?

[aryan9600]

lgtm! thanks @njohnstone2 🙇

[stefanprodan]

LGTM

Awesome job @njohnstone2 thanks 🥇 Also thanks @aryan9600 for the reviews.

[njohnstone2]

Thanks team! Really appreciate the guidance getting this added to the project.