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: Add per-node option to Custom Graphs #23771

Merged
merged 1 commit into from
Mar 15, 2018

Conversation

mrtracy
Copy link
Contributor

@mrtracy mrtracy commented Mar 13, 2018

Adds "Breakdown" aggregator option to custom graphs, which will graph a
separate line for every node in the cluster for a given metric.

Also fixes a bug where you could not graph multiple lines of the same
named metric.

Release note: None

screen shot 2018-03-12 at 9 48 21 pm

@mrtracy mrtracy requested review from vilterp, couchand and a team March 13, 2018 01:49
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@mrtracy mrtracy changed the title ui: Add Breakout aggregator to Custom Graphs ui: Add Breakdown aggregator to Custom Graphs Mar 13, 2018
@mrtracy mrtracy force-pushed the mtracy/custom_graph_new_options branch from 4108c01 to 8592691 Compare March 13, 2018 01:50
@vilterp
Copy link
Contributor

vilterp commented Mar 14, 2018

This is cool — tried locally and it works good 👍 Starting to remind me of honeycomb.io's query editor:
image

The "breakdown" value of 999 seems a little weird; maybe we can wrap aggregation/breakdown in another type? Any thoughts @couchand?

Aside: reduces the need for #23733… Maybe we should link from the current graphs to "broken-down" graphs here.

@vilterp
Copy link
Contributor

vilterp commented Mar 14, 2018

Capturing in-person discussion: would prefer a separate column with a checkbox that says "per-node" (or something), instead of "breakdown" as an aggregation option with a sentinel value.

That'd be more discoverable to users, and gets rid of sentinel value awkwardness.

@mrtracy mrtracy force-pushed the mtracy/custom_graph_new_options branch from 8592691 to bd62bcd Compare March 14, 2018 21:55
@mrtracy
Copy link
Contributor Author

mrtracy commented Mar 14, 2018

Updated with suggestion (also updated commit message).

screen shot 2018-03-14 at 5 53 58 pm

@vilterp vilterp changed the title ui: Add Breakdown aggregator to Custom Graphs ui: Add per-node option to Custom Graphs Mar 14, 2018
Copy link
Contributor

@vilterp vilterp left a comment

Choose a reason for hiding this comment

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

Love it. LGTM pending lint fixes.

Adds "Per Node" option to custom graphs, which will graph a separate
line for every node in the cluster for a given metric.

Also fixes a bug where you could not graph multiple lines of the same
named metric.

Release note: None
@mrtracy mrtracy force-pushed the mtracy/custom_graph_new_options branch from bd62bcd to b96ab02 Compare March 15, 2018 00:00
@mrtracy mrtracy merged commit 052891a into cockroachdb:master Mar 15, 2018
@mrtracy mrtracy deleted the mtracy/custom_graph_new_options branch March 15, 2018 19:59
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.

3 participants