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

Dashboards: defined container functions for common resources panels #331

Merged

Conversation

darrenjaneczek
Copy link
Contributor

What this PR does:

Dashboards: defined (overridable) container functions for common resources panels:

  • containerDiskWritesPanel,
  • containerDiskReadsPanel,
  • containerDiskSpaceUtilization

Added whitespace for readability of some related queries.

Verified that the generated dashboard results match, so long as the following following pattern replacement is made:
'\\n\s*' to ''

Checklist

  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@darrenjaneczek darrenjaneczek requested a review from a team as a code owner June 17, 2021 03:52
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Can we move the reformatting without any actual change to a dedicated PR, explicitly stating it's just reformatting stuff? Otherwise, can we avoid reformatting queries at all? It makes very complicated to review these PRs.

Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM! (modulo a nit)

CHANGELOG.md Outdated
@@ -14,6 +14,7 @@
* [ENHANCEMENT] cortex-mixin: Make `cluster_namespace_deployment:kube_pod_container_resource_requests_{cpu_cores,memory_bytes}:sum` backwards compatible with `kube-state-metrics` v2.0.0. #317
* [BUGFIX] Fixed `CortexIngesterHasNotShippedBlocks` alert false positive in case an ingester instance had ingested samples in the past, then no traffic was received for a long period and then it started receiving samples again. #308
* [BUGFIX] Alertmanager: fixed `--alertmanager.cluster.peers` CLI flag passed to alertmanager when HA is enabled. #329
* [CHANGE] Dashboards: defined container functions for common resources panels: containerDiskWritesPanel, containerDiskReadsPanel, containerDiskSpaceUtilization. #331
Copy link
Collaborator

Choose a reason for hiding this comment

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

A CHANGE is a breaking change for the final user. This PR can be considered an ENHANCEMENT. Also we need to keep CHANGELOG entries sorted by type (CHANGE, FEATURE, ENHANCEMENT, BUGFIX).

Suggested change
* [CHANGE] Dashboards: defined container functions for common resources panels: containerDiskWritesPanel, containerDiskReadsPanel, containerDiskSpaceUtilization. #331
* [ENHANCEMENT] Dashboards: defined container functions for common resources panels: containerDiskWritesPanel, containerDiskReadsPanel, containerDiskSpaceUtilization. #331

added:
- containerDiskWritesPanel
- containerDiskReadsPanel
- containerDiskSpaceUtilization
@darrenjaneczek darrenjaneczek force-pushed the darrenjaneczek/resources-dashboards-container-functions branch from 3de040c to e5a38db Compare June 22, 2021 14:15
@pracucci pracucci merged commit b32d042 into main Jun 22, 2021
@pracucci pracucci deleted the darrenjaneczek/resources-dashboards-container-functions branch June 22, 2021 14:29
simonswine pushed a commit to grafana/mimir that referenced this pull request Oct 18, 2021
…czek/resources-dashboards-container-functions

Dashboards: defined container functions for common resources panels
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