✨ (grafana/v1-alpha) : add custom units for custom grafana dashboards

[jkremser]

Based on #2858 (comment) I found the current state where the seconds are used for each metric type quite insufficient. It's good for bootstrapping, but unusable for real stuff.

This PR adds a way to specify the unit for the metric. If the unit is not provided it will try to guess it from the name of the metric (if it contains second -> s, duration -> s, byte -> B), otherwise it's set to none (Numeral type in Grafana), this is imho better default than seconds. The automatic guessing of the unit is based on these Prometheus naming conventions.

In the same link ^ describing the conventions it's also described that people often expose the runtime (non-numeric) information (versions, gitSha, etc.) about the app using the something_name_sdfsdf_info metrics (also described here). So this pr also tries to identify such metrics and if the name of the metrics ends with _info it turns it into the table and it's displayed as on the screenshot:

side-note: OpenTelemetry standard for metrics also support Units metadata, but this is up to Grafana contributors to interpret this in the dashboards.

Signed-off-by: Jirka Kremser jiri.kremser@gmail.com

[k8s-ci-robot]

Welcome @jkremser!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @jkremser. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[camilamacedo86]

Hi @jkremser,

Thank you for your contribution 🥇
It shows a nice addition.

I think would be nice we get the review from @Kavinjsir (its author) and @varshaprasad96
Otherwise, It has my lgtm either.

[camilamacedo86]

/ok-to-test

[varshaprasad96]

/approve
/lgtm
This looks good. @jkremser Thanks for trying out this plugin and contributing to it! We really appreciate it!
@Kavinjsir Can you please take a look too. Holding for your review.
/hold
@jkremser waiting for the plugin author to review. We'll make sure to merge this in by end of this week.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, jkremser, varshaprasad96

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [camilamacedo86,varshaprasad96]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Kavinjsir]

/lgtm

@jkremser Thx for bringing this nice idea! The judgement of unit is actually challenging. Reference of the naming convention sounds good 👍🏼 .

Applying table than time series panel for info metrics also looks good to me!

[varshaprasad96]

@jkremser looks like the testdata is being updated but the changes are not committed. Can you run make generate to re-generate the testdata and commit those changes.

[camilamacedo86]

HI @jkremser,

Could you please just run make generate for us to update the testdata sample and be able to move forward with this one?

[jkremser]

@camilamacedo86 & @varshaprasad96
done in d12c263 (thanks for the hint)

[camilamacedo86]

/lgtm

[camilamacedo86]

Hi @jkremser,

Could you please sort out the lint issue for we move forward?
See that you can run make lint-fix for this case.

[jkremser]

done

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: jkremser / name: Jirka Kremser (8b6e2b5, d12c263, 0b494ae)

[camilamacedo86]

/lgtm

[jkremser]

wrt failed APIDiff check: I was trying the old version (w/o this change) of compiled kubebuilder binary with the new custom-metrics/config.yaml format that can have a unit and it worked. JSON unmarshalling passed and the dashboards were created (no units logic, though)

Similarly when trying the new version (containing the change from this PR) on old version of custom-metrics/config.yaml it works. So it's actually backward compatible change (and also but the previous version also works/don't break with the new stuff)

But yea, strictly speaking it may be considered as a breaking change (some third party that could potentially consume it as a library may depend on the CustomMetricItem struct), so I don't know what standards of sem-ver do you have for plugins. Should I bump the major version somewhere or is it ok?

[camilamacedo86]

Hi @jkremser,

The API check error is:

sigs.k8s.io/kubebuilder/v3/pkg/plugins/common/kustomize/v2-alpha
Incompatible changes:

• KustomizeVersion: value changed from "v4.5.7" to "v4.5.5"

That happens because it is not rebased with master.
Could you please re-base the PR so that we can move forward?
Also, would be great if we can squash the commits.

[jkremser]

@camilamacedo86 rebase & squash - done

[camilamacedo86]

/lgtm