rpk: grafana-generate - support public metrics

[r-vasquez]

Cover letter

rpk generate grafana-dashboard have a summary section that has some metrics that don't exist in the new /public_metrics endpoint.

This PR creates a new summary section with the new metrics available in the /public_metrics endpoint.

Fixes #5646

Grafana dashboard example: here

Backport Required

• v22.2.x

UX changes

Old:

Now:

We are keeping backward compatibility for /metrics endpoint and added new panels / modified some aggregation criteria for the new endpoint only:

Node Up and Partitions panels

Now it queries redpanda_cluster_brokers and redpanda_cluster_partitions.

Latency of Kafka Request

We are splitting produce and consume requests in each percentile. We are querying redpanda_kafka_request_latency_seconds_bucket now.

Throughput

Is now calculed by sum(rate(redpanda_kafka_request_bytes_total[2m])) by (redpanda_request).

Changes that affect all panels

We are removing the shard label, filtering, and aggregation criteria in the /public_metrics endpoint because the new metrics don't have a shard label.

Release notes

Improvements

• rpk generate grafana-dashboard now supports /public_metrics endpoint.

[VladLazar]

I'll defer to someone with better go knowledge (mine's non-existent) for the code review.

Functionally it looks nice. There's a couple of improvements we could make:

• Remove the labels that never have a value from the legend. For all the panels in "Redpanda Summary" we can remove the redpanda_request label from the legent as it never has a value.
  Same for the panels in the "Internal RPC Latency" section. The panel in the "Throughput" section should only keep the "redpanda_request" label in the legend.
• Can we change the Y axis unit for panels that represent time (latency) to seconds?

@BenPope, @travisdowns could you take a look too? I generated some load between 14:20 and 14:25 UTC so the panels should display something around that time.

[BenPope]

• The intervals for rates should prefer[$__rate_interval] over a fixed period such as [2m], but you may need to set a min step of 1m.
• The units should always be set; e.g., it looks like the latency is off by some orders of magnitude, and should be set to seconds.
• I'd be tempted to also aggregate schema registry and HTTP Proxy errors by redpanda_status - it's in the legend, after all.
• In general I'd prefer rate over irate, at least for simple queries - I don't want to get into subqueries here.
• Memory stats have shard in the legend, but not in aggregation criteria (probably worth adding a new option that is cluster,instance,shard in the dropdown.
• The unit for scheduler stats is seconds per second - it should be percent (0..1), not ops/s.
• The storage section has rate queries, and I don't know what we're trying to measure. Probably worth combining them into a ratio of disk used %.
• Internal latency of schema registry / http proxy isn't internal, it's the request latency.

[travisdowns]

Nice to see support for public metrics. I had some feedback but Ben got all of it in his comments above (particularly important to fix the units for latency before it goes out).

Minor concern about how we detect the endpoint: we look for the string public_metrics in the URL is that right? Could it be a problem if someone is using URL-rewriting so that it does not appear or (less likely) they happen to have this string (e.g. in the hostname part) in the hostname so it appears spuriously? I think it's good enough and it this would only be something to consider changing if we have a better solution, but I'm not sure what that is: @VladLazar is there some strong indicator in the metrics results themselves to distinguish public from oldschool metrics?

[BenPope]

The new metrics all start with redpanda_, the old ones with vectorized_

[r-vasquez]

Force Push:

• Removed the labels that never have a value from the legend. (Summary and Internal RPC Latency section)
• Changed latency units to seconds.
• Changed interval for rates from a fixed [2m] to [$__rate_interval] with a minimum of [1m]
• Changed unit for scheduler stats to be a percent (0...1)

New Grafana example can be found here

[r-vasquez]

Single line change: it's changing because now I'm adding the default interval rate to be [1m]

[0x5d]

In hindsight I think it would have been better to create a golden file instead of putting all this in a JSON single-liner. it would make reviewing these changes easier.

[VladLazar]

Looking good. I think a few of Ben's comments still need addressing:

• Rename "Internal latency of request for schema_registry" to "Schema Registry Request Latency" and change unit to seconds
• Rename "Internal latency of rest_proxy" to "REST Proxy Request Latency" and change unit to seconds
• Replace the panels in the storage section with two new panels that display the ratio of disk available:
  • Disk Usage per Broker (the percentage of disk currently in use): 1 - (redpanda_storage_disk_free_bytes / redpanda_storage_disk_total_bytes)
• Aggregate the rest proxy and schema registry errors by redpanda_status. The queries should change like this:
  sum(...) by ($aggr_criteria, redpanda_status). Note the new label we aggregate by.

Also, I would replace the panels in the memory section with the following:

• Memory Usage per Broker (the percentage of memory currently in use): redpanda_memory_allocated_memory / (redpanda_memory_free_memory + redpanda_memory_allocated_memory)

[r-vasquez]

@VladLazar We take the name of the panel from the HELP of each metric, in the case of Internal Latency of Request for schema_registry and rest_proxy:

# HELP redpanda_schema_registry_request_latency_seconds Internal latency of request for schema_registry
# TYPE redpanda_schema_registry_request_latency_seconds histogram
redpanda_schema_registry_request_latency_seconds_sum{} 0

# HELP redpanda_rest_proxy_request_latency_seconds Internal latency of request for rest_proxy
# TYPE redpanda_rest_proxy_request_latency_seconds histogram
redpanda_rest_proxy_request_latency_seconds_sum{} 0

To avoid hardcoding this, can we change the response from /public_metrics ? lmk so I can make the change if needed.

Aggregate the rest proxy and schema registry errors by redpanda_status.

Is this just for the 2 panels or can we add the aggregation criteria for every other panel?

Replace the panels in the storage section with two new panels that display the ratio of disk available

Is it helpful to leave the ones that we have and add the 2 new panels?

Also, I would replace the panels in the memory section with the following

Same as above.

I'm asking all this because we will have a lot of custom hardcoded logic in rpk for public_metrics vs the way we handle /metrics endpoint's panels and want to make sure that I understand all 😃

[VladLazar]

@VladLazar We take the name of the panel from the HELP of each metric, in the case of Internal Latency of Request for schema_registry and rest_proxy:

If that's the case then let's leave it as is for now and we can change the description of the metrics in redpanda.

Is this just for the 2 panels or can we add the aggregation criteria for every other panel?

Just for those two panels. The others don't have this label attached to them.

Is it helpful to leave the ones that we have and add the 2 new panels?

I would just replace them. They'd display the same information, but in a different way. I think what a user actually
cares about is how much disk and memory are being used.

[twmb]

Should this be !strings.HasPrefix?

Also, is the counter vs. gauge thing a redpanda issue?

[travisdowns]

Yeah this looks like a bug. I think all of the repdanda_storage metrics in public_metrics should be gauges. Filed: #6316

[twmb]

Two non-blocking questions

[emaxerrno]

@tmgstevens may have thoughts on the default panels we show on /public_metrics i think CS found other panels to yield higher information/better signal.

[tmgstevens]

@tmgstevens may have thoughts on the default panels we show on /public_metrics i think CS found other panels to yield higher information/better signal.

This is an example of an ops dashboard that we've produced in CS which covers some of the things we'd be encouraging people to monitor. Unfortunately it seems that some of these things aren't available in public_metrics, so we may need a separate issue to add those things.
List as follows

1. Nodes Up
2. Uptime
3. No. Partitions
4. No. Topics
5. Leadership transfer rate (not present)
6. Under replicated partitions
7. Leaderless partitions
8. CPU Utilisation (not present)
9. Allocated Memory
10. Leadership balance
11. Currently active connections (not present)
12. Cluster info (build numbers, versions, etc) (not present)
13. Produce latency
14. Consumer latency
15. Storage bytes written (not present)
16. Storage bytes read (not present)
17. Network bytes received
18. Network bytes sent
19. Under-replicated partitions (by topic) (not present)
20. Leaderless partitions (list)
21. Under replicated partitions by cluster (not present)
22. Number of groups for which a node is a leader
23. Partition leadership per broker

[emaxerrno]

@r-vasquez ^^ see tristan's comments above.

[VladLazar]

@emaxerrno would it be a good idea to decouple the fix (this PR) and the improvements suggested by @tmgstevens?
I'm on-board with improving the generated dashboards as Tristan suggests, but lacking integration with Grafana for public_metrics is a regression when compared to the metrics endpoint and it's blocking adoption.

[BenPope]

5. Leadership transfer rate (not present)

Yep, not available.

8. CPU Utilisation (not present)

You may find these useful:

# HELP redpanda_cpu_busy_seconds_total Total CPU busy time in seconds
# TYPE redpanda_cpu_busy_seconds_total gauge
# HELP redpanda_scheduler_runtime_seconds_total Accumulated runtime of task queue associated with this scheduling group
# TYPE redpanda_scheduler_runtime_seconds_total counter

11. Currently active connections (not present)

I thought we had this, but apparently not.

12. Cluster info (build numbers, versions, etc) (not present)

I thought we had this, but apparently not.

15. Storage bytes written (not present)
16. Storage bytes read (not present)

Yep, not available.

19. Under-replicated partitions (by topic) (not present)

# HELP redpanda_kafka_under_replicated_replicas Number of under replicated replicas (i.e. replicas that are live, but not at the latest offest)
# TYPE redpanda_kafka_under_replicated_replicas gauge

21. Under replicated partitions by cluster (not present)

Can be derived from redpanda_kafka_under_replicated_replicas

[tmgstevens]

@emaxerrno would it be a good idea to decouple the fix (this PR) and the improvements suggested by @tmgstevens? I'm on-board with improving the generated dashboards as Tristan suggests, but lacking integration with Grafana for public_metrics is a regression when compared to the metrics endpoint and it's blocking adoption.

I would, however, suggest going through the list I mentioned and making sure that we've got as many of those things on the generated dashboard. For example - IIRC Under-replicated partitions isn't anywhere near the top of the page, but should be as it's really important to monitor.

[twmb]

We need to separate further dashboard improvements into a separate PR -- the scope of this PR was originally to migrate from our overly excessive /metrics to our new /public_metrics.

The best long term fix would be to remove the dashboard generation from pure-Go, rpk-only code and separate it to something that can be maintained and extended by all teams.

If we are ok with the current dashboards in this PR, we should merge this PR. I think the comments above indicate that the current dashboards are good, pending any disagreements, I think we should merge this by EOD.

We should have two followup issues, one tracking how to separate these dashboards from pure-Go code so that the dashboards can be maintained more broadly, and one to further improve the dashboards per @tmgstevens's suggestions above.

Lmk if there are any disagreements here, otherwise the plan is to merge EOD.

[0x5d]

lgtm besides unresolved comments.

[0x5d]

In hindsight I think it would have been better to create a golden file instead of putting all this in a JSON single-liner. it would make reviewing these changes easier.

[tmgstevens]

We need to separate further dashboard improvements into a separate PR -- the scope of this PR was originally to migrate from our overly excessive /metrics to our new /public_metrics.

The best long term fix would be to remove the dashboard generation from pure-Go, rpk-only code and separate it to something that can be maintained and extended by all teams.

If we are ok with the current dashboards in this PR, we should merge this PR. I think the comments above indicate that the current dashboards are good, pending any disagreements, I think we should merge this by EOD.

We should have two followup issues, one tracking how to separate these dashboards from pure-Go code so that the dashboards can be maintained more broadly, and one to further improve the dashboards per @tmgstevens's suggestions above.

Lmk if there are any disagreements here, otherwise the plan is to merge EOD.

I'm cool with that. I haven't managed to actually run the code anywhere, if someone can generate a dashboard for me then I'll happily test it, but generally, bar the comments above, LGTM

[r-vasquez]

Latest Grafana dashboard example: here

Force Push:

• More accurate string validation for both metrics endpoint and hack around redpanda_storage until we solve Some storage metrics should be guages not counters #6316
• graf README update with the new variable INTERVAL
• Typos

The improvements to the generated dashboard will be solved by #6382 so we can split the fix to make public_metric work and the improvements of the dashboard. 😄

[r-vasquez]

/backport v22.1.x : my mistake.

[vbotbuildovich]

Branch name "v22.1.x" not found.

Workflow run logs.

[r-vasquez]

/backport v22.2.x

[vbotbuildovich]

Branch name "v22.2.x" not found.

Workflow run logs.