feature: Add text boxes and descriptions to reads and writes dashboards #324

darrenjaneczek · 2021-06-09T04:33:07Z

What this PR does:

Focussing on the reads and writes dashboards,
added some info panels and hover-over descriptions
for some of the panels.
Some common code used by the compactor also
received additional text content.

New functions:

addRows
addRowsIf
...to add a list of rows to a dashboard.

The thanosMemcachedCache function has had some of its query text
sprawled out for easier reading and comparison with similar dashboard
queries.

Checklist

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

Focussing on the reads and writes dashboards, added some info panels and hover-over descriptions for some of the panels. Some common code used by the compactor also received additional text content. New functions: - addRows - addRowsIf ...to add a list of rows to a dashboard. The `thanosMemcachedCache` function has had some of its query text sprawled out for easier reading and comparison with similar dashboard queries.

pracucci

I'm not super sure about this change because of a couple of reasons:

It makes some rows very verbose
It adds 1 panel to some (already very crowded) rows, which will make it more difficult to render nicely on not-very-large screens

darrenjaneczek · 2021-06-10T13:29:14Z

I'm not super sure about this change because of a couple of reasons:

It makes some rows very verbose

It adds 1 panel to some (already very crowded) rows, which will make it more difficult to render nicely on not-very-large screens

Perhaps we can block these changes with a $._config setting that is defaulted to false.
However, if so, would that $.config setting also affect the text panels added to compactor.

cortex-mixin/dashboards/dashboard-utils.libsonnet

osg-grafana

Use sentence case. For more information, refer to https://developers.google.com/style/capitalization#capitalization-in-titles-and-headings.

…ctor dashboard under tooltips, rather than making them text boxes.

09jvilla · 2021-06-13T22:27:26Z

@pracucci I've thinned out a lot of the original text, mostly removing the text boxes and tooltip descriptions in the top left of the panels, but leaving a "Description" text box at the top.

I edited a few of the row names to try to be a bit more explicit. For example instead of
"Blocks Index Cache (Store-Gateway)" its now
"Blocks Index Cache (Store-Gateway Accesses)".
The goal here was to try to be a bit friendlier to a new operator, who might not know the relationship between the blocks index cache and store-gateway.

To make the compactor dashboard a little easier to look at, I took the text that is already in the main branch and tucked it under the panel descriptions, eliminating the text boxes that are there today.

With these updates, we shouldn't have any rows that are wider than 3 panels, except for the 4-panel wide object storage rows (which are unchanged from what they are today in the main branch).

CHANGELOG.md

cortex-mixin/dashboards/compactor.libsonnet

cortex-mixin/dashboards/writes.libsonnet

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

…github.com:grafana/cortex-jsonnet into darrenjaneczek/dashboard-descriptions-reads-writes

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

…github.com:grafana/cortex-jsonnet into darrenjaneczek/dashboard-descriptions-reads-writes

- Most existing "per second" panel titles in `main` are written "/ sec", corrected recent commits to match.

pracucci

Thanks for working on this and addressing my feedback. I left few comments about tiny things to fix and then we can merge.

pracucci · 2021-06-10T15:19:13Z

CHANGELOG.md

@@ -12,6 +12,7 @@
 * [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
 * [CHANGE] Dashboards: added overridable `job_labels` and `cluster_labels` to the configuration object as label lists to uniquely identify jobs and clusters in the metric names and group-by lists in dashboards. #319
 * [CHANGE] Dashboards: `alert_aggregation_labels` has been removed from the configuration and overriding this value has been deprecated. Instead the labels are now defined by the `cluster_labels` list, and should be overridden accordingly through that list. #319
+* [ENHANCEMENT] Added documentation text panels and descriptions to Reads and Writes dashboards. 


Suggested change

* [ENHANCEMENT] Added documentation text panels and descriptions to Reads and Writes dashboards.

* [ENHANCEMENT] Added documentation text panels and descriptions to Reads and Writes dashboards. #324

pracucci · 2021-06-10T15:20:40Z

cortex-mixin/dashboards/dashboard-utils.libsonnet

+  getObjectStoreRows(title, component):: [
+    ($.row(title) { height: '25px' })
+    .addPanel(
+      $.textPanel(


Do we really need this? Looks like what it describes is pretty trivial. You can easily notice it's the latency and that it displays avg, median and 99th percentile.

pracucci · 2021-06-18T11:25:22Z

CHANGELOG.md

@@ -12,6 +12,7 @@
 * [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
 * [CHANGE] Dashboards: added overridable `job_labels` and `cluster_labels` to the configuration object as label lists to uniquely identify jobs and clusters in the metric names and group-by lists in dashboards. #319
 * [CHANGE] Dashboards: `alert_aggregation_labels` has been removed from the configuration and overriding this value has been deprecated. Instead the labels are now defined by the `cluster_labels` list, and should be overridden accordingly through that list. #319
+* [ENHANCEMENT] Added documentation text panels and descriptions to reads and writes dashboards. 


[nit] We're used to link the related PR.

Suggested change

* [ENHANCEMENT] Added documentation text panels and descriptions to reads and writes dashboards.

* [ENHANCEMENT] Added documentation text panels and descriptions to reads and writes dashboards. #324

pracucci · 2021-06-18T11:33:20Z

cortex-mixin/dashboards/compactor.libsonnet

+        $.panelDescription(
+          'Compacted blocks / sec',
+          |||
+            Display the amount of time that it’s taken to generate a single compacted block.


The description looks wrong here.

Suggested change

Display the amount of time that it’s taken to generate a single compacted block.

Rate of blocks that are generated as a result of a compaction operation.

pracucci · 2021-06-18T11:33:42Z

cortex-mixin/dashboards/compactor.libsonnet

+        $.panelDescription(
+          'Per-block compaction duration',
+          |||
+            Rate of blocks that are generated as a result of a compaction operation.


The description looks wrong here.

Suggested change

Rate of blocks that are generated as a result of a compaction operation.

Display the amount of time that it’s taken to generate a single compacted block.

pracucci · 2021-06-18T12:04:10Z

cortex-mixin/dashboards/dashboard-utils.libsonnet

-      $.panel('QPS') +
-      $.queryPanel('sum by(operation) (rate(thanos_memcached_operations_total{%s,component="%s",name="%s"}[$__rate_interval]))' % [$.jobMatcher(jobName), component, cacheName], '{{operation}}') +
+      $.panel('Requests per second') +
+      $.queryPanel(


Note to reviewers: just reformatted.

pracucci · 2021-06-18T12:04:52Z

cortex-mixin/dashboards/dashboard-utils.libsonnet

+      jobMatcher: $.jobMatcher(jobName),
+      component: component,
+      cacheName: cacheName,
+      cacheNameReadable: std.strReplace(cacheName, '-', ' '),


This is unused. I think can be removed.

pracucci · 2021-06-18T12:05:30Z

cortex-mixin/dashboards/dashboard-utils.libsonnet

    )
    .addPanel(
      $.panel('Latency (getmulti)') +
-      $.latencyPanel('thanos_memcached_operation_duration_seconds', '{%s,operation="getmulti",component="%s",name="%s"}' % [$.jobMatcher(jobName), component, cacheName])
+      $.latencyPanel(


Note to reviewers: just reformatted.

pracucci · 2021-06-18T12:06:02Z

cortex-mixin/dashboards/dashboard-utils.libsonnet

-                     cacheName,
-                   ], 'items') +
-      { yaxes: $.yaxes('percentunit') },
+      $.queryPanel(


Note to reviewers: just reformatted.

pracucci · 2021-06-18T12:06:56Z

cortex-mixin/dashboards/dashboard-utils.libsonnet

    ),

  filterNodeDiskContainer(containerName)::
    |||
-      ignoring(%s) group_right() (label_replace(count by(%s, %s, device) (container_fs_writes_bytes_total{%s,container="%s",device!~".*sda.*"}), "device", "$1", "device", "/dev/(.*)") * 0)
-    ||| % [$._config.per_instance_label, $._config.per_node_label, $._config.per_instance_label, $.namespaceMatcher(), containerName],
+      ignoring(%s) group_right() (


Note to reviewers: just reformatted.

pracucci · 2021-06-18T12:27:39Z

cortex-mixin/dashboards/writes.libsonnet

@@ -5,20 +5,42 @@ local utils = import 'mixin-utils/utils.libsonnet';
    ($.dashboard('Cortex / Writes') + { uid: '0156f6d15aa234d452a33a4f13c838e3' })
    .addClusterSelectorTemplates()
    .addRow(


This new row takes quite a lot of vertical space.

To make everyone happy, could we add a config option to enable/disable the dashboard description and conditionally add this row only when enabled? I'm fine to keep it enabled by default but would allow us to disable it in our infra.

Same comment applies to the read dashboard. A single config option shared between the 2 dashboards looks fine.

pracucci · 2021-06-18T12:31:46Z

cortex-mixin/dashboards/compactor.libsonnet

+          'Tenants compaction progress',
+          |||
+            In a multi-tenant cluster, display the progress of tenants that are compacted while compaction is running.
+            Reset to `0` after the compaction run is completed for all tenants in the shard.


I tried to render these changes and doesn't look the tooltip is displayed much nicely (Grafana 8.1.0). Not a blocker for this PR, but I would suggest you to talk to grafana team.

I can fix the 0 by wrapping it in <tt/> instead.
I'll ask about the dark heading font.
Thanks for the heads up.

Ok, thanks! Keep in mind I've commented here, but this may apply to other placesu/descriptions too (eg. the heading color applies to any description).

pracucci

LGTM, thanks!

…writes

Feedback has been addressed

…czek/dashboard-descriptions-reads-writes feature: Add text boxes and descriptions to reads and writes dashboards

darrenjaneczek requested a review from a team as a code owner June 9, 2021 04:33

darrenjaneczek requested a review from 09jvilla June 9, 2021 04:34

darrenjaneczek added 3 commits June 10, 2021 09:25

fix: text replacements, repair addRows

77f8609

fix: changelog

c4db3e1

pracucci reviewed Jun 10, 2021

View reviewed changes

darrenjaneczek force-pushed the darrenjaneczek/dashboard-descriptions-reads-writes branch from 9394e02 to c4db3e1 Compare June 10, 2021 13:27

darrenjaneczek commented Jun 10, 2021

View reviewed changes

cortex-mixin/dashboards/dashboard-utils.libsonnet Outdated Show resolved Hide resolved

osg-grafana changed the title ~~feature: add some text boxes and descriptions to reads/writes dashboards~~ feature: Add text boxes and descriptions to reads and writes dashboards Jun 11, 2021

osg-grafana self-requested a review June 11, 2021 15:10

osg-grafana suggested changes Jun 11, 2021

View reviewed changes

Jennifer Villa added 3 commits June 13, 2021 14:23

Changing copy to add 'latency' as well.

9e6c2f4

Cut down on text from initial PR. Tucked existing text from the compa…

7a7b13c

…ctor dashboard under tooltips, rather than making them text boxes.

Getting rid of a few space/comma errors.

acc320a

osg-grafana previously requested changes Jun 14, 2021

View reviewed changes

darrenjaneczek and others added 13 commits June 15, 2021 13:27

Update CHANGELOG.md

8368248

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/compactor.libsonnet

6ad57cd

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/compactor.libsonnet

c33303a

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/compactor.libsonnet

cb7054c

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/compactor.libsonnet

357db43

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/compactor.libsonnet

19cb601

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/compactor.libsonnet

4735870

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

fix: formatting - limit to 4 panels per row

fa48a91

Merge branch 'darrenjaneczek/dashboard-descriptions-reads-writes' of …

dafb212

…github.com:grafana/cortex-jsonnet into darrenjaneczek/dashboard-descriptions-reads-writes

fmt

c7b7871

fix: remove accidental line

6c0066c

Update cortex-mixin/dashboards/dashboard-utils.libsonnet

773926a

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/reads.libsonnet

a12d815

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

darrenjaneczek and others added 13 commits June 15, 2021 13:47

Update cortex-mixin/dashboards/reads.libsonnet

b335df9

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/writes.libsonnet

73e65cf

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/writes.libsonnet

13f0fa3

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/writes.libsonnet

6c0ebb8

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/writes.libsonnet

ea7d87d

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/writes.libsonnet

4aed696

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Update cortex-mixin/dashboards/reads.libsonnet

c411115

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

fix: Requests per second

0c17f02

fix: text

b8ccacc

Apply suggestions from code review as per @osg-grafana

d5b14c1

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

fix: clarity

b22d22e

Apply suggestions from code review as per @osg-grafana

dffe62a

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

Merge branch 'darrenjaneczek/dashboard-descriptions-reads-writes' of …

2aae011

…github.com:grafana/cortex-jsonnet into darrenjaneczek/dashboard-descriptions-reads-writes

darrenjaneczek requested review from pracucci and osg-grafana June 15, 2021 20:09

darrenjaneczek added 4 commits June 17, 2021 13:34

fix: query formatting to aid in merge

eafdbfc

fix: query formatting to aid in merge

dddd6e7

fix: consistent labelling

fcc4896

fix: ensure panel titles are consistent

513b096

- Most existing "per second" panel titles in `main` are written "/ sec", corrected recent commits to match.

pracucci reviewed Jun 18, 2021

View reviewed changes

fix: resolve review feedback

5794607

pracucci approved these changes Jun 22, 2021

View reviewed changes

Merge branch 'main' into darrenjaneczek/dashboard-descriptions-reads-…

4fb7275

…writes

pracucci merged commit 383a286 into main Jun 22, 2021

pracucci deleted the darrenjaneczek/dashboard-descriptions-reads-writes branch June 22, 2021 09:58

simonswine pushed a commit to grafana/mimir that referenced this pull request Oct 18, 2021

Merge pull request grafana/cortex-jsonnet#324 from grafana/darrenjane…

fd3df9a

…czek/dashboard-descriptions-reads-writes feature: Add text boxes and descriptions to reads and writes dashboards

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: Add text boxes and descriptions to reads and writes dashboards #324

feature: Add text boxes and descriptions to reads and writes dashboards #324

darrenjaneczek commented Jun 9, 2021

pracucci left a comment

darrenjaneczek commented Jun 10, 2021

osg-grafana left a comment

09jvilla commented Jun 13, 2021 •

edited

Loading

pracucci left a comment

pracucci Jun 10, 2021

pracucci Jun 10, 2021

pracucci Jun 18, 2021

pracucci Jun 18, 2021

pracucci Jun 18, 2021

pracucci Jun 18, 2021

pracucci Jun 18, 2021

pracucci Jun 18, 2021

pracucci Jun 18, 2021

pracucci Jun 18, 2021

pracucci Jun 18, 2021 •

edited

Loading

pracucci Jun 18, 2021

darrenjaneczek Jun 21, 2021

pracucci Jun 21, 2021

pracucci left a comment

	* [ENHANCEMENT] Added documentation text panels and descriptions to Reads and Writes dashboards.
	* [ENHANCEMENT] Added documentation text panels and descriptions to Reads and Writes dashboards. #324

	Display the amount of time that it’s taken to generate a single compacted block.
	Rate of blocks that are generated as a result of a compaction operation.

feature: Add text boxes and descriptions to reads and writes dashboards #324

feature: Add text boxes and descriptions to reads and writes dashboards #324

Conversation

darrenjaneczek commented Jun 9, 2021

pracucci left a comment

Choose a reason for hiding this comment

darrenjaneczek commented Jun 10, 2021

osg-grafana left a comment

Choose a reason for hiding this comment

09jvilla commented Jun 13, 2021 • edited Loading

pracucci 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

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

pracucci Jun 18, 2021 • edited Loading

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

pracucci left a comment

Choose a reason for hiding this comment

09jvilla commented Jun 13, 2021 •

edited

Loading

pracucci Jun 18, 2021 •

edited

Loading