roachprod: prometheus query & list --spark #138252

srosenberg · 2025-01-04T02:15:25Z

Previously, prometheus Query and QueryRange
was available only to roachtest; i.e., outside
of roachprod. This PR refactors the code s.t.
both can easily query prometheus.

Inspired by this new roachprod trick, we add
a new list option, namely roachprod list --spark.
When enabled, a sparkline is rendered directly in the
terminal. (As of this time, only iterm2 is supported.)
The sparkline graph denotes the cluster aggregate
throughput of committed KV transactions, i.e.,
txn_commits.

Epic: none
Release note: None

cockroach-teamcity · 2025-01-04T02:15:35Z

This change is

blathers-crl · 2025-01-04T17:58:47Z

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

Previously, prometheus `Query` and `QueryRange` was available only to roachtest; i.e., outside of roachprod. This PR refactors the code s.t. both can easily query prometheus. Inspired by this new roachprod trick, we add a new list option, namely `roachprod list --spark`. When enabled, a sparkline is rendered directly in the terminal. (As of this time, only `iterm2` is supported.) The sparkline graph denotes the cluster aggregate throughput of committed KV transactions, i.e., `txn_commits`. Epic: none Release note: None

srosenberg · 2025-01-05T02:11:36Z

Below is an example of roachprod list either via --spark or export ROACHPROD_SPARK_LINE=true.

golgeek

Looks amazing 😻

golgeek · 2025-01-06T15:32:59Z

pkg/cmd/roachprod/cli/list.go

+			points := prometheus.Values(series)
+			img, err := sparkline.DrawSparkline(points, 250, 35)
+			if img != nil && err == nil {
+				if s, err := sparkline.ITermEncodePNGToString(img, ""); err != nil {


Nit: Wouldn't it be better to have a sparkline.DrawAsItermString(points, 250, 35) that combines both calls above?

golgeek · 2025-01-06T15:33:31Z

pkg/cmd/roachprod/cli/list.go

+	for _, labelValToSeries := range m {
+		for clusterName, series := range labelValToSeries {
+			points := prometheus.Values(series)
+			img, err := sparkline.DrawSparkline(points, 250, 35)


Nit: sparkline.Draw() should suffice

DarrylWong

Very cool!

DarrylWong · 2025-01-07T20:11:31Z

pkg/cmd/roachprod/cli/flags.go

@@ -200,6 +201,10 @@ func initListCmdFlags(listCmd *cobra.Command) {
 		"cost", "c", os.Getenv("ROACHPROD_COST_ESTIMATES") == "true",
 		"Show cost estimates",
 	)
+	listCmd.Flags().BoolVarP(&listSparkline,
+		"spark", "s", os.Getenv("ROACHPROD_SPARK_LINE") == "true",
+		"Show sparkline",


nit: This flag description could be fleshed out a bit. Something mentioning: Show sparkline graph of txn_commits over last 5 minutes. Currently only supported on iterm?

Sure thing, I'll add some text to describe the flag.

DarrylWong · 2025-01-07T20:20:07Z

pkg/cmd/roachprod/cli/list.go

+)
+
+const (
+	clusterQueryTemplate = "sum(rate(txn_commits{cluster=~\"$CLUSTERS\"}[5m])) by (cluster)"


Why txn_commits vs say cpu or qps? Mostly just curious on the envisioned use.

The choice is definitely not entirely random :) Allow me to explain. CPU utilization isn't a good proxy for what the DB might be doing under the hood; e.g., an increase/decrease in transaction throughput may not move the needle wrt cpu, if it's mostly IO-bound. Also, a cpu has many cores, and we don't (yet) have recording rules in prometheus; i.e., the above query would be more expensive. By qps, I suppose you mean the throughput of executed sql statements? We could do that. I chose the KV layer because technically there could be no sql, and yet there are KV transactions; e.g., range splits, other system tasks, like cluster initialization, would all be reflected in that metric.

In summary, txt_commits seems like a good indicator of the amount of KV work a cluster might be doing or not. (Incidentally, it's also usually linearly correlated with the gRPC throughput.)

Thanks for the explanation! Makes sense to me!

herkolategan

Nice! Eye Candy!

kvoli · 2025-01-09T14:56:14Z

Don't wait up for me to review, working through a bit of a backlog atm and this already appears more than well reviewed by test eng :)

nameisbhaskar

I have some minor comments.

nameisbhaskar · 2025-01-06T05:27:45Z

pkg/cmd/roachtest/tests/drt.go

@@ -14,6 +14,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/task"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/prometheus"
+	prominstaller "github.com/cockroachdb/cockroach/pkg/roachprod/prometheus/prominstaller"


nit: prominstaller is redundant.

nameisbhaskar · 2025-01-06T05:32:30Z

pkg/util/sparkline/iterm.go

@@ -0,0 +1,53 @@
+// Copyright 2024 The Cockroach Authors.


Suggested change

// Copyright 2024 The Cockroach Authors.

// Copyright 2025 The Cockroach Authors.

"nit"

Lol, stuck in the past :)

srosenberg force-pushed the sr/roachprod_prometheus_query branch 3 times, most recently from a8a7623 to 4026f8f Compare January 4, 2025 17:58

srosenberg force-pushed the sr/roachprod_prometheus_query branch 3 times, most recently from c3a3d97 to 62645c7 Compare January 5, 2025 00:49

srosenberg force-pushed the sr/roachprod_prometheus_query branch from 62645c7 to 646a40a Compare January 5, 2025 02:09

srosenberg changed the title ~~WIP~~ roachprod: prometheus query & list --spark Jan 5, 2025

srosenberg marked this pull request as ready for review January 5, 2025 02:10

srosenberg requested a review from a team as a code owner January 5, 2025 02:10

srosenberg requested review from herkolategan, DarrylWong, golgeek and kvoli and removed request for a team January 5, 2025 02:10

srosenberg mentioned this pull request Jan 5, 2025

roachprod: add executing roachtest to list #138263

Open

golgeek approved these changes Jan 6, 2025

View reviewed changes

DarrylWong approved these changes Jan 7, 2025

View reviewed changes

herkolategan approved these changes Jan 8, 2025

View reviewed changes

kvoli removed their request for review January 9, 2025 14:56

nameisbhaskar reviewed Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

roachprod: prometheus query & list --spark #138252

roachprod: prometheus query & list --spark #138252

srosenberg commented Jan 4, 2025 •

edited

Loading

cockroach-teamcity commented Jan 4, 2025

blathers-crl bot commented Jan 4, 2025

srosenberg commented Jan 5, 2025

golgeek left a comment

golgeek Jan 6, 2025

golgeek Jan 6, 2025

DarrylWong left a comment

DarrylWong Jan 7, 2025

srosenberg Jan 7, 2025

DarrylWong Jan 7, 2025

srosenberg Jan 7, 2025

DarrylWong Jan 9, 2025

herkolategan left a comment

kvoli commented Jan 9, 2025

nameisbhaskar left a comment

nameisbhaskar Jan 6, 2025

nameisbhaskar Jan 6, 2025

srosenberg Jan 10, 2025

	// Copyright 2024 The Cockroach Authors.
	// Copyright 2025 The Cockroach Authors.

roachprod: prometheus query & list --spark #138252

Are you sure you want to change the base?

roachprod: prometheus query & list --spark #138252

Conversation

srosenberg commented Jan 4, 2025 • edited Loading

cockroach-teamcity commented Jan 4, 2025

blathers-crl bot commented Jan 4, 2025

srosenberg commented Jan 5, 2025

golgeek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarrylWong 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

herkolategan left a comment

Choose a reason for hiding this comment

kvoli commented Jan 9, 2025

nameisbhaskar 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

srosenberg commented Jan 4, 2025 •

edited

Loading