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

ui: use metric name as custom chart title #81182

Merged
merged 1 commit into from
May 16, 2022

Conversation

tbg
Copy link
Member

@tbg tbg commented May 11, 2022

Custom charts were previously difficult to read because the graph didn't
include any indication of what was being graphed. This is now rectified
by printing the metrics name. This isn't the most user friendly thing
(ideally we would also include the actual title, and make the help text
available) but with my limited TSX knowledge this is solving 90% of the
problem with a 61 character change.

Related to #81035.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg tbg requested a review from a team May 11, 2022 01:19
@tbg tbg marked this pull request as ready for review May 11, 2022 01:19
@tbg tbg requested a review from erikgrinaker May 13, 2022 14:07
@tbg
Copy link
Member Author

tbg commented May 13, 2022

I need to fix some whitespace but other than that, this works.

Copy link
Collaborator

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

What whitespace do you need adjusted? It looked "fine" to me.
Screenshot 2022-05-13 at 10-18-31 Custom Chart Debug Cockroach Console

@tbg tbg force-pushed the cust-chart-title branch from 12f3d99 to 9ce19c2 Compare May 13, 2022 15:21
Custom charts were previously difficult to read because the graph didn't
include any indication of what was being graphed. This is now rectified
by printing the metrics name. This isn't the most user friendly thing
(ideally we would also include the actual title, and make the help text
available) but with my limited TSX knowledge this is solving 90% of the
problem with a 61 character change.

Related to cockroachdb#81035.

Release note: None
@tbg tbg force-pushed the cust-chart-title branch from 9ce19c2 to e46aef2 Compare May 13, 2022 17:12
@tbg
Copy link
Member Author

tbg commented May 13, 2022

bors r=dhartunian,erikgrinaker

It's beautiful:

image

@craig
Copy link
Contributor

craig bot commented May 13, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented May 13, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented May 14, 2022

Build failed:

@tbg
Copy link
Member Author

tbg commented May 16, 2022

    python_1     | psql: error: private key file "/certs/client.root.key" must be owned by the current user or root

I think this has been fixed now since bors has managed to merge something in the meantime. Here goes...

bors r=dhartunian,erikgrinaker

@craig
Copy link
Contributor

craig bot commented May 16, 2022

Build failed:

@tbg
Copy link
Member Author

tbg commented May 16, 2022

#73980
bors r=dhartunian,erikgrinaker

@craig
Copy link
Contributor

craig bot commented May 16, 2022

Build succeeded:

@craig craig bot merged commit 4fd43e7 into cockroachdb:master May 16, 2022
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.

4 participants