roachprod-microbench: change sheet order#110448
roachprod-microbench: change sheet order#110448craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Previously metric units in the summary sheet, and on sheet tabs, were sorted alphabetically. Since `sec/op` is the most important metric it should be first. This change reverses the order to ensure it is. Epic: None Release note: None
|
This change is |
|
|
||
| // Sort sheets by name. | ||
| // Sort sheets by name in reverse order. This ensures `sec/op` is the first | ||
| // sheet and metric in the summary. |
There was a problem hiding this comment.
I think I see how this works, but if we add a new sheet (watts/sec) this will quietly break. Are we ok with that? My understanding is that we are since we will eventually be looking at Prometheus as source of truth.
herkolategan
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @DarrylWong, @rachitgsrivastava, and @renatolabs)
pkg/cmd/roachprod-microbench/google/service.go line 88 at r1 (raw file):
Previously, renatolabs (Renato Costa) wrote…
I think I see how this works, but if we add a new sheet (
watts/sec) this will quietly break. Are we ok with that? My understanding is that we are since we will eventually be looking at Prometheus as source of truth.
Correct, z-a order doesn't necessarily mean we'll get the "most important" first especially if custom metrics exist. Alternative might be to try and preserve the original ordering.
|
bors r=renatolabs |
|
Build failed: |
|
bors r=renatolabs |
|
Build failed: |
|
The upgrade acceptance test failed by finding a known issue in an older release -- see also #110702. I'll put a patch for it later today. Unrelated to this PR. bors retry |
|
Build succeeded: |
Previously metric units in the summary sheet, and on sheet tabs, were sorted alphabetically. Since
sec/opis the most important metric it should be first. This change reverses the order to ensure it is.Epic: None
Release note: None