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

fix(dashboards): use new grafonnet for apiserver dashboard #890

Conversation

Duologic
Copy link
Contributor

@Duologic Duologic commented Feb 16, 2024

ref: #888

This updates the apiserver dashboard to use the new Grafonnet.

This will cause some changes but should result in the same dashboard.

A few noteable changes:

  • Less 'defaults' are set, grafonnet-lib was quite verbose, the new grafonnet only adds the necessary.
  • deprecated graph panel is replaced by timeSeries panel
  • deprecated singlestat panel is replaced by the stat panel
  • the 'row' structure is now calculated on the grid based on the width of the panels, it should match the original layout

The code abstracts a few of these panels into separate functions to avoid duplication a bit.

Also see inline comments about a few changes outside the dashboard.

scripts/go.mod Outdated Show resolved Hide resolved
@@ -11,7 +11,7 @@
refresh: kubernetesMixin._config.grafanaK8s.refresh,
tags: kubernetesMixin._config.grafanaK8s.dashboardTags,

rows: [
[if 'rows' in super then 'rows']: [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Grafana layout engine doesn't operate on 'rows' anymore (is really old), it now uses a grid system. I've included the defaults that exist in this file directly on the dashboard, we can generalize as more dashboards get ported.

.lint Outdated Show resolved Hide resolved
Comment on lines +6 to +9
panel-datasource-rule:
reason: The new Grafonnet promotes the use of datasources at the query level. This should probably end up in the linter as a valid option.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The linter wants that the Panel has a datasource set that matches the template variable, this is debatable. Panels do not really deal with datasources, the value at the panel level is passed through by the UI to the query targets.

When you only use one data source, this doesn't matter that much but when combining queries from multiple data sources on the same panel, then you have to set it to -- Mixed -- on the Panel level.

This is the what the new Grafonnet does, from a jsonnet/code perspective you'll see that the data source is configured on the query rather than the panel.

@Duologic Duologic marked this pull request as ready for review February 16, 2024 19:59
@povilasv
Copy link
Contributor

Thanks for this, rebase needed :)

@Duologic Duologic force-pushed the duologic/rewrite_apiserver_dashboard branch from 18b187a to bec80ad Compare February 19, 2024 12:49
@Duologic
Copy link
Contributor Author

Rebased, dropped a few commits.

Copy link
Contributor

@povilasv povilasv left a comment

Choose a reason for hiding this comment

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

Thanks for this

@povilasv povilasv merged commit 883f294 into kubernetes-monitoring:master Feb 21, 2024
6 checks passed
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.

2 participants