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: unify grafana dashboard datasource by use of a variable and add new karpenter 0.15.0 consolidation graphs #2276

Merged
merged 2 commits into from
Sep 1, 2022
Merged

feat: unify grafana dashboard datasource by use of a variable and add new karpenter 0.15.0 consolidation graphs #2276

merged 2 commits into from
Sep 1, 2022

Conversation

pyromaniac3010
Copy link
Contributor

@pyromaniac3010 pyromaniac3010 commented Aug 10, 2022

Get rid of statically assigned datasource uid. This enables users to import the dashboard to kube-prometheus-stack without altering it afterwards to search and replace the datasource uid.

How was this change tested?
using grafana config in kube-prometheus stack with my version results in directly showing graphs. With the old version it shows errors about not found datasource.

    dashboards:
      karpenter:
        capacity-dashboard:
          url: https://raw.githubusercontent.com/pyromaniac3010/karpenter/a811cda492d0a365ae922b666bbbe62439102883/website/content/en/preview/getting-started/getting-started-with-eksctl/karpenter-capacity-dashboard.json
          datasource: Prometheus
        performance-dashboard:
          url: https://raw.githubusercontent.com/pyromaniac3010/karpenter/a811cda492d0a365ae922b666bbbe62439102883/website/content/en/preview/getting-started/getting-started-with-eksctl/karpenter-performance-dashboard.json
          datasource: Prometheus

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note

NONE

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Sorry, something went wrong.

@pyromaniac3010 pyromaniac3010 requested a review from a team as a code owner August 10, 2022 13:08
@netlify
Copy link

netlify bot commented Aug 10, 2022

Deploy Preview for karpenter-docs-prod ready!

Name Link
🔨 Latest commit 04c1fcb
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/62fe4a8eb1e97d000809eaa9
😎 Deploy Preview https://deploy-preview-2276--karpenter-docs-prod.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@pyromaniac3010 pyromaniac3010 changed the title feat: unify grafana dashboard datasource by use of a variable feat: unify grafana dashboard datasource by use of a variable and add new karpenter 0.15.0 consolidation graphs Aug 18, 2022
@pyromaniac3010
Copy link
Contributor Author

I also added the new karpenter 0.15.0 consolidation metrics to the capacity dashboard

@njtran
Copy link
Contributor

njtran commented Aug 22, 2022

Hi @pyromaniac3010, thanks for the contribution!

Since I'm not an expert here, can you explain a little more about what a non-statically defined datasource UID does, or how this helped you for development? When you say With the old version it shows errors about not found datasource., were you using custom datasources with the dashboards and interchanging between that and the default Prometheus data source?

Additionally, for the dashboard, do you mind posting some screenshots of how these look?

@pyromaniac3010
Copy link
Contributor Author

Hi @njtran,

in the "old" version there is a "uid": "PBFA97CFB590B2093" defined for every widget. This only works for installations which actually have a prometheus datasource with this exact UID defined. This leads to problems, if you try to use this dashboard as a pre-provisioned dashboard with kube-prometheus-stack.
I changed that to use a variable, which fixed the behavior for the pre-provisioned dashboards in kube-prometheus-stack.

This is a screenshot of the changes made to include the new "Consolidation" metrics (including "cluster"-label):
image

@njtran
Copy link
Contributor

njtran commented Sep 1, 2022

Hey, sorry for the delay. This looks good. I'll try it out on my cluster. Can you add a little more information on how you tested this? For instance, how did you install the helm charts? Did you import the JSON manually?

Copy link
Contributor

@njtran njtran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @pyromaniac3010, the dashboard looks good to me. Tested it with the following grafana-values.yaml to reflect your newer commit. Looks like it works with or without the datasource: Prometheus part, so going to approve as it looks good as is.

dashboards:
  default:
    capacity-dashboard:
      url: https://raw.githubusercontent.com/pyromaniac3010/karpenter/04c1fcb6c54a1b320ebbbb90ea388563493a67b7/website/content/en/preview/getting-started/getting-started-with-eksctl/karpenter-capacity-dashboard.json
      datasource: Prometheus
    performance-dashboard:
      url: https://raw.githubusercontent.com/pyromaniac3010/karpenter/04c1fcb6c54a1b320ebbbb90ea388563493a67b7/website/content/en/preview/getting-started/getting-started-with-eksctl/karpenter-performance-dashboard.json
      datasource: Prometheus

Thanks for the contribution!

@njtran njtran merged commit dd5c22a into aws:main Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants