✨ Experimental practice to scaffold dashboard manifest for custom metrics #2858

Kavinjsir · 2022-08-04T09:11:09Z

(This PR is experimental and welcome discussion)

Description

This is the experimental practice to see how possible if we can bring dashboards for custom metrics through the Grafana Plugin.

The user can add the desired metrics in grafana/custom-metrics/config.yaml:

---
customMetrics: []
  # - metric: Raw custom metric (required)
  #   type:   Metric type: counter/gauge/histogram (required)
  #   expr:   Prom_ql for the metric (optional)

(If you execute the plugin for the first time(either by init or edit), the above config template will be automatically generated.)

The plugin triggered by edit subcommand will read the config.yaml and generate grafana/custom-metrics/custom-metrics-dashboard.json, which can be directly import in the Grafana Web UI.

Story 1

Users get the config entry when initialize the project or introduce the grafana-plugin for the first time:

# Initialize a project with the plugin enabled
kubebuilder init --plugins=grafana.kubebuilder.io/v1-alpha

# Introduce the Grafana Plugin to an existing project
kubebuilder edit --plugins=grafana.kubebuilder.io/v1-alpha

A nested custom-metrics/config.yaml will be generated under grafana/.

Story 2

Users configure grafana/custom-metrics/config.yaml and run edit subcommand to get Grafana manifest:

kubebuilder edit --plugins=grafana.kubebuilder.io/v1-alpha

The grafana/custom-metrics/config.yaml will be validated and then the manifest will be generated: grafana/custom-metrics/custom-metrics-dashboard.json.

Sample

Specify custom metrics in config.yaml

---
customMetrics:
  - metric: memcached_operator_reconcile_total
    type: counter
  - metric: memcached_operator_reconcile_time_seconds_bucket
    type: histogram

Run kubebuilder edit --plugins=grafana.kubebuilder.io/v1-alpha1 to generate the manifest
Import the manifest into Grafana Web UI:

k8s-ci-robot · 2022-08-04T09:11:18Z

Hi @Kavinjsir. 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.

pkg/plugins/optional/grafana/v1alpha/scaffolds/edit.go

pkg/plugins/optional/grafana/v1alpha/scaffolds/internal/templates/custom.go

k8s-ci-robot · 2022-08-16T06:16:50Z

@Kavinjsir: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test lint

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.

k8s-ci-robot · 2022-08-16T06:19:21Z

@Kavinjsir: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test Lint

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.

Kavinjsir · 2022-08-16T16:41:26Z

Since grafana/custom-metrics/config.yaml is exposed to customize the panels, I'm also looking for methods to test the plugin upon a sample config.

Right now the integration test will generate test data from 0 to 1. I'm wondering how we may insert a config template such as:

---
customMetrics:
  - metric: memcached_operator_reconcile_total
    type: counter
  - metric: memcached_operator_reconcile_time_seconds_bucket
    type: histogram

And run kubebuilder edit ... follow by that in the integration test?

cc: @varshaprasad96 @camilamacedo86 @rashmigottipati

varshaprasad96

Good work @Kavinjsir ! Just a few nits.

pkg/plugins/optional/grafana/v1alpha/scaffolds/edit.go

varshaprasad96 · 2022-08-19T16:29:08Z

pkg/plugins/optional/grafana/v1alpha/scaffolds/edit.go

+	return false
+}
+
+func fillMissingExpr(item templates.CustomMetricItem) templates.CustomMetricItem {


so this func adds a default expression when user does not provide it?
If so, is it compatible with all the three kinds of prometheus metrics like gauge, counter etc.

so this func adds a default expression when user does not provide it?

Yes, the current design is the plugin will check if the user provides expr, if missed, then filled a basic one.

is it compatible with all the three kinds of prometheus metrics like gauge, counter etc.

Yes, very basic though:

counter: sum(rate(custom_metric{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, pod)

histogram: histogram_quantile(0.90, sum by(instance, le) (rate(custom_metric{job=\"$job\", namespace=\"$namespace\"}[5m])))

gauge: the custom_metric itself (show the value directly)

I would be glad if we have better choices of expr, especially for gauge.
(Not quite confident to bring further prom_ql for gauge since it seems such fluctuating values might be observed in various perspectives..)

pkg/plugins/optional/grafana/v1alpha/scaffolds/edit.go

camilamacedo86 · 2022-09-02T18:20:01Z

/test pull-kubebuilder-test

camilamacedo86 · 2022-09-02T18:20:29Z

/test pull-kubebuilder-e2e-k8s-1-18-20

camilamacedo86 · 2022-09-02T18:20:43Z

/test pull-kubebuilder-e2e-k8s-1-19-16

Kavinjsir · 2022-09-02T18:44:53Z

/test pull-kubebuilder-test

Kavinjsir · 2022-09-03T18:52:47Z

/test pull-kubebuilder-e2e-k8s-1-18-20

Kavinjsir · 2022-09-03T18:53:08Z

/test pull-kubebuilder-e2e-k8s-1-19-16

Kavinjsir · 2022-09-03T18:53:26Z

/test pull-kubebuilder-test

camilamacedo86 · 2022-09-03T21:17:46Z

/test pull-kubebuilder-e2e-k8s-1-19-16
/test pull-kubebuilder-test

camilamacedo86 · 2022-09-05T07:58:30Z

/test pull-kubebuilder-test

camilamacedo86 · 2022-09-05T08:10:00Z

/test pull-kubebuilder-test

Kavinjsir · 2022-09-05T16:01:34Z

See that the two test cases remain erroring. Would you like me to fix it by additional coding?
@varshaprasad96 @camilamacedo86

Kavinjsir · 2022-08-16T16:47:00Z

pkg/plugins/optional/grafana/v1alpha/scaffolds/internal/templates/custom_metrics.go

+              }
+            ]
+          },
+          "unit": "s"


Temporary set the unit for all panels to be seconds. This would affect the dimension of the lines shown in each panel:

Not sure if this is acceptable to left for users OR maybe we could make this configurable through config.yaml?

configurable units would be much appreciated!

pkg/plugins/optional/grafana/v1alpha/scaffolds/edit.go

Kavinjsir · 2022-08-26T10:59:27Z

pkg/plugins/optional/grafana/v1alpha/scaffolds/edit.go

+	return false
+}
+
+func fillMissingExpr(item templates.CustomMetricItem) templates.CustomMetricItem {


so this func adds a default expression when user does not provide it?

Yes, the current design is the plugin will check if the user provides expr, if missed, then filled a basic one.

Kavinjsir · 2022-08-26T10:59:39Z

pkg/plugins/optional/grafana/v1alpha/scaffolds/edit.go

+	return false
+}
+
+func fillMissingExpr(item templates.CustomMetricItem) templates.CustomMetricItem {


is it compatible with all the three kinds of prometheus metrics like gauge, counter etc.

Yes, very basic though:

counter: sum(rate(custom_metric{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, pod)

histogram: histogram_quantile(0.90, sum by(instance, le) (rate(custom_metric{job=\"$job\", namespace=\"$namespace\"}[5m])))

gauge: the custom_metric itself (show the value directly)

pkg/plugins/optional/grafana/v1alpha/scaffolds/edit.go

docs/book/src/plugins/grafana-v1-alpha.md

Kavinjsir · 2022-08-31T20:52:28Z

pkg/plugins/optional/grafana/v1alpha/scaffolds/internal/templates/custom.go

+customMetrics:
+#  - metric: # Raw custom metric (required)
+#    type:   # Metric type: counter/gauge/histogram (required)
+#    expr:   # Prom_ql for the metric (optional)


Yeah, that makes sense. I'm not sure how could it be better. Right now, I just append an example following the template. Would this be good?

Kavinjsir · 2022-09-02T16:29:47Z

pkg/plugins/optional/grafana/v1alpha/scaffolds/edit.go

 )

 var _ plugins.Scaffolder = &editScaffolder{}

+const ConfigFilePath = "grafana/custom-metrics/config.yaml"


Sounds good!

Kavinjsir · 2022-09-05T17:26:35Z

test/e2e/grafana/generate_test.go

+
+		BeforeEach(func() {
+			var err error
+			kbc, err = utils.NewTestContext(pluginutil.KubebuilderBinName, "GO111MODULE=on")


Test error may come from here. Since such NewTestContext would call kubectl that assumes an existing kind cluster.

camilamacedo86 · 2022-09-05T21:01:36Z

test/e2e/plugin.sh

@@ -0,0 +1,33 @@
+#!/usr/bin/env bash


@Kavinjsir we should not add a new file here.
In this case, we need only in the Kubebuilder Makefile to call the test.

We would only need to execute this test once. However, to make simple let's call it with the others and a TODO here to sort it out after.

That means:
a) Remove this file plugin.sh
b) Add the new test in : https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/e2e/setup.sh#L65
c) Revert the changes made in the Kubebuilder Makefile
d) Add a comment in the setup.sh and in the new e2e test like TODO: in this case, we do not need the cluster and we would not need to test against many k8s versions. So that it can be improved in the future.

c/c @varshaprasad96

/hold

Sounds good to me. I've updated the code based on the steps and it looks work well.

Co-authored-by: Varsha <varshaprasad96@gmail.com> Camila Macedo <camilamacedo86@gmail.com> - Scaffold `config.yaml` template - Load configured `config.yaml` to create manifest - Add test cases for custom metrics manifests - Documentation Go template syntax ref: https://stackoverflow.com/a/22375000

camilamacedo86

/hold cancel

camilamacedo86 · 2022-09-06T05:34:54Z

/lgtm

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 4, 2022

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 4, 2022

k8s-ci-robot requested review from camilamacedo86 and jmrodri August 4, 2022 09:11

Kavinjsir force-pushed the feat/cutom-metrics-experiment branch from b5194c9 to 15df8cd Compare August 4, 2022 19:57

varshaprasad96 reviewed Aug 5, 2022

View reviewed changes

Kavinjsir force-pushed the feat/cutom-metrics-experiment branch 2 times, most recently from 22a4e5a to 81e8ddc Compare August 16, 2022 05:18

Kavinjsir marked this pull request as ready for review August 16, 2022 05:42

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 16, 2022

Kavinjsir force-pushed the feat/cutom-metrics-experiment branch from 81e8ddc to 3fb8a89 Compare August 16, 2022 05:54

camilamacedo86 closed this Aug 18, 2022

camilamacedo86 reopened this Aug 18, 2022

varshaprasad96 reviewed Aug 19, 2022

View reviewed changes

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 25, 2022

Kavinjsir force-pushed the feat/cutom-metrics-experiment branch 8 times, most recently from 3d40a17 to 9180448 Compare August 26, 2022 19:27

Kavinjsir force-pushed the feat/cutom-metrics-experiment branch 3 times, most recently from ca98b1a to c1fcb7e Compare September 3, 2022 18:05

camilamacedo86 closed this Sep 5, 2022

camilamacedo86 reopened this Sep 5, 2022

Kavinjsir commented Sep 5, 2022

View reviewed changes

camilamacedo86 reviewed Sep 5, 2022

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 5, 2022

Kavinjsir force-pushed the feat/cutom-metrics-experiment branch from c1fcb7e to 001daab Compare September 6, 2022 01:19

camilamacedo86 reviewed Sep 6, 2022

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 6, 2022

k8s-ci-robot assigned camilamacedo86 Sep 6, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 6, 2022

camilamacedo86 merged commit 46f36a3 into kubernetes-sigs:master Sep 6, 2022

Kavinjsir mentioned this pull request Sep 14, 2022

REQUEST: New membership for Kavinjsir kubernetes/org#3694

Closed

9 tasks

This was referenced Sep 22, 2022

Create sample Grafana dashboard reflecting K8GB metrics k8gb-io/k8gb#120

Closed

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Experimental practice to scaffold dashboard manifest for custom metrics #2858

✨ Experimental practice to scaffold dashboard manifest for custom metrics #2858

Kavinjsir commented Aug 4, 2022 •

edited

Loading

k8s-ci-robot commented Aug 4, 2022

k8s-ci-robot commented Aug 16, 2022

k8s-ci-robot commented Aug 16, 2022

Kavinjsir commented Aug 16, 2022

varshaprasad96 left a comment

varshaprasad96 Aug 19, 2022

Kavinjsir Aug 26, 2022

Kavinjsir Aug 26, 2022

Kavinjsir Aug 26, 2022

camilamacedo86 commented Sep 2, 2022

camilamacedo86 commented Sep 2, 2022

camilamacedo86 commented Sep 2, 2022

Kavinjsir commented Sep 2, 2022

Kavinjsir commented Sep 3, 2022

Kavinjsir commented Sep 3, 2022

Kavinjsir commented Sep 3, 2022

camilamacedo86 commented Sep 3, 2022

camilamacedo86 commented Sep 5, 2022

camilamacedo86 commented Sep 5, 2022

Kavinjsir commented Sep 5, 2022

Kavinjsir Aug 16, 2022

jkremser Sep 23, 2022

Kavinjsir Aug 26, 2022

Kavinjsir Aug 26, 2022

Kavinjsir Aug 31, 2022

Kavinjsir Sep 2, 2022

Kavinjsir Sep 5, 2022

camilamacedo86 Sep 5, 2022 •

edited

Loading

camilamacedo86 Sep 5, 2022

Kavinjsir Sep 6, 2022

camilamacedo86 left a comment

camilamacedo86 commented Sep 6, 2022

✨ Experimental practice to scaffold dashboard manifest for custom metrics #2858

✨ Experimental practice to scaffold dashboard manifest for custom metrics #2858

Conversation

Kavinjsir commented Aug 4, 2022 • edited Loading

(This PR is experimental and welcome discussion)

Description

Story 1

Story 2

Sample

k8s-ci-robot commented Aug 4, 2022

k8s-ci-robot commented Aug 16, 2022

k8s-ci-robot commented Aug 16, 2022

Kavinjsir commented Aug 16, 2022

varshaprasad96 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 commented Sep 2, 2022

camilamacedo86 commented Sep 2, 2022

camilamacedo86 commented Sep 2, 2022

Kavinjsir commented Sep 2, 2022

Kavinjsir commented Sep 3, 2022

Kavinjsir commented Sep 3, 2022

Kavinjsir commented Sep 3, 2022

camilamacedo86 commented Sep 3, 2022

camilamacedo86 commented Sep 5, 2022

camilamacedo86 commented Sep 5, 2022

Kavinjsir commented Sep 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 Sep 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 left a comment

Choose a reason for hiding this comment

camilamacedo86 commented Sep 6, 2022

Kavinjsir commented Aug 4, 2022 •

edited

Loading

camilamacedo86 Sep 5, 2022 •

edited

Loading