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

feat: Add a Dashboard for Monitoring the Operator Metrics #1860

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Daniel-Vaz
Copy link

@Daniel-Vaz Daniel-Vaz commented Feb 12, 2025

With this PR I'm adding the possibility to create a ConfigMap containing a Dashboard definition leveraging the operator metrics, based on a new set of Helm Values.

The new dashboard is based on this one.

Closes #653

@theSuess Could you please have a look ?
Feel free to mention any changes you see that could make sense.

@CLAassistant
Copy link

CLAassistant commented Feb 12, 2025

CLA assistant check
All committers have signed the CLA.

@NissesSenap
Copy link
Collaborator

It feels so wrong using a configmap for the grafana-operator when the whole point of the operator is to not use configmaps 😄

But I understand that we get a chicken and egg issue. we can't create grafana-operator CRDs as part of the helm chart.

But I do think we should document how to actually use this dashboard, with calling it from a CRD.
I would also suggest documentating that you can just use the url feature of the dashboard CRD, and point to https://grafana.com/grafana/dashboards/22785-grafana-operator/.

On the implementation side, I would suggest storing the pure json as a file, and call on it from helm: https://helm.sh/docs/chart_template_guide/accessing_files/#basic-example

This way you can easily copy and paste in new versions and it's a bit cleaner to read in my mind.

@theSuess
Copy link
Member

+1 on the splitting up into a separate JSON file. That way we can also provide docs on how to include this dashboard using a remote reference for those who don't manage the operator using helm

@Daniel-Vaz
Copy link
Author

Daniel-Vaz commented Feb 17, 2025

@NissesSenap @theSuess

Thank you for your feedback!

I’ve updated the ConfigMap template to fetch the JSON definition from a dedicated file, as requested. I also enhanced the Values comments to include at least a mention of the Grafana.com URL and the dashboard ID.

Would you like me to add or modify an existing documentation page to cover the alternative method of deploying the Dashboard using an operator-managed CR? If so, which page in your opinion, would make the most sense for this to be mentioned in ?

@theSuess
Copy link
Member

I think this could live on the same level as the installation guides with a title like Operations & Observability

Happy to hear out other suggestions though

@Daniel-Vaz Daniel-Vaz force-pushed the master branch 2 times, most recently from c7f8f3f to f4670b8 Compare February 25, 2025 16:40
@Daniel-Vaz Daniel-Vaz requested a review from weisdd February 25, 2025 16:40
@github-actions github-actions bot added the documentation Issues relating to documentation, missing, non-clear etc. label Feb 25, 2025
@theSuess
Copy link
Member

The doc page looks good! The last thing I'd add here, is an example GrafanaDashboard referencing the deployed configmap for people using the helm option

@Daniel-Vaz
Copy link
Author

The doc page looks good! The last thing I'd add here, is an example GrafanaDashboard referencing the deployed configmap for people using the helm option

Done 😄

@theSuess theSuess added this to the v5.17.0 milestone Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues relating to documentation, missing, non-clear etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grafana Operator Internal Dashboard
5 participants