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

Metricbeat overview dashboard for Zookeeper #10379

Merged
merged 5 commits into from
Jan 30, 2019

Conversation

sayden
Copy link
Contributor

@sayden sayden commented Jan 28, 2019

I realized that we didn't have any dashboard for Zookeeper so I took advantage that I had all setup done to build one.

Just very key metrics were included, a total of 5 charts using 7 metrics:

  • Alive connections
  • Percentaje between open file descriptors and max file descriptors
  • Latency
  • Approximate data size
  • Packets received / sent

image

@sayden sayden added enhancement review Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Jan 28, 2019
@sayden sayden self-assigned this Jan 28, 2019
@sayden sayden requested a review from a team as a code owner January 28, 2019 17:06
@sayden sayden added the needs_backport PR is waiting to be backported to other branches. label Jan 28, 2019
@kaiyan-sheng
Copy link
Contributor

@sayden just curious :-) why the alive connections diagram is not just lines connecting each data point(like the packet received/sent)? What does the bottom green part mean?

@ruflin
Copy link
Member

ruflin commented Jan 29, 2019

Great to see a dashboard for zookeeper. Seems like this needs a make update.

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

The json file is in the wrong directory. I moved it locally for testing and then everything loads as expected and graphs are shown.

Could you also add the screenshot you have above to the module docs like we do in other modules?

@@ -0,0 +1,477 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

This file belongs into the dashboard directory inside 7, otherwise import does not work.

@sayden
Copy link
Contributor Author

sayden commented Jan 29, 2019

@sayden just curious :-) why the alive connections diagram is not just lines connecting each data point(like the packet received/sent)? What does the bottom green part mean?

When there's a single value, using an Area chart instead of a line chart make it looks a bit more dynamically beautiful 😄 Some charting libraries and products uses Area charts by default because of this.

When using 2 values like packets sent / received, this is not an option.

@jsoriano
Copy link
Member

@sayden awesome dashboard. Only a couple of small suggestions.
You may want to rearrange the graphs to put three graphs in a row so there is not a third row with a single graph.
For the file descriptors visualization, would it be possible to show the percentage in left axis and the absolute number in the right one? It can be useful to know the exact number of descriptors used.

@sayden sayden requested a review from a team as a code owner January 29, 2019 12:32
@sayden sayden force-pushed the feature/mb/zookeeper-overview-dashboard branch from e5bb695 to 907c246 Compare January 29, 2019 12:35
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM. My focus on testing was to check if dashboards are loaded correctly and if some data shows up which works.

@sayden
Copy link
Contributor Author

sayden commented Jan 29, 2019

For the file descriptors visualization, would it be possible to show the percentage in left axis and the absolute number in the right one? It can be useful to know the exact number of descriptors used.

@jsoriano As I wrote in the first message, the percentaje of file descriptors is a ratio between the max file descriptors and the used ones. If you want, for example, used file descriptors on the right axis, it is just an absolute number with no real meaning because you'll have to rely on the percentaje anyways. My question is: isn't it going to be too much information? I mean that I even did the visualization for the used file descriptors but then I realized that to see 1000 or 100000 wasn't very useful without knowing the max file descriptors. Just to clarify, Zookeeper only return max and used file descriptors.

@sayden
Copy link
Contributor Author

sayden commented Jan 29, 2019

This is what I mean: 26k file descriptors.... oh! that's a lot! Actually only a 0.003% of max configured 😄

image

@jsoriano
Copy link
Member

it is just an absolute number with no real meaning because you'll have to rely on the percentage anyways.

For this very reason any other value in this dashboard would have no real meaning 😃

Percentages are useful for capacity reasons, agree, but absolute values are also useful for other things, e.g. to identify problems like "with X file descriptors or Y active connections average latency increases", this is harder to do with percentages. Also think for example that you want to reduce the maximum number of file descriptors, it is also easier to decide a proper value if you have absolute values, and it is also easier to compare two different configurations if you have the absolute values.

This is what I mean: 26k file descriptors.... oh! that's a lot! Actually only a 0.003% of max configured smile

26k in an idle zookeeper? is this value correct? what does zookeeper do so many file descriptors? 🤔

And please start both scales from zero.

@jsoriano
Copy link
Member

Oh, and a cosmetic detail, please make the three graphs in the second row to have the same width.

@sayden
Copy link
Contributor Author

sayden commented Jan 29, 2019

I have adjusted the size and set the number format to 2 decimal places (by default it was 3, that's why I thought it had 26.000 opened file descriptors)

image

I didn't understand what do you mean with And please start both scales from zero. They are automatically scaled, do you want to fix the lower bounds to zero?

Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

looks great 👍

@ruflin
Copy link
Member

ruflin commented Jan 30, 2019

The dashboard screenshots keep getting nicer and nicer. @sayden Can you make sure that the screenshot used in the docs is from the "final" dashboard?

@jsoriano
Copy link
Member

I didn't understand what do you mean with And please start both scales from zero. They are automatically scaled, do you want to fix the lower bounds to zero?

Yes, please.

@sayden
Copy link
Contributor Author

sayden commented Jan 30, 2019

Updated screenshot with changes addressed :)

image

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Great 👍

@sayden sayden merged commit 34fecd4 into elastic:master Jan 30, 2019
sayden added a commit to sayden/beats that referenced this pull request Feb 1, 2019
@sayden sayden added v6.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants