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

Add nats dashboard #10235

Merged
merged 5 commits into from Jan 28, 2019
Merged

Add nats dashboard #10235

merged 5 commits into from Jan 28, 2019

Conversation

ChrsMark
Copy link
Member

This PR adds an overview dashboard for nats module as proposed on #10071.

Co-Authored-By: Stamatis Katsaounis katsaouniss@gmail.com
Co-Authored-By: Michael Katsoulis michaelkatsoulis88@gmail.com

@ChrsMark ChrsMark requested review from a team as code owners January 22, 2019 07:27
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@ChrsMark
Copy link
Member Author

screenshot from 2019-01-21 12-47-24

@ruflin here is something we created.

TBH I'm not confident enough about the beauty of the dashboard, but this is the overview we find meaningful for our purposes.

@ruflin
Copy link
Member

ruflin commented Jan 22, 2019

Thanks, this is great. I'm good with the dashboards as is but still need to test it on my end.

Some suggestions:

  • Should we use one box per number instead of multiple in one box?
  • It seems Kibana does not know it's a byte value. Probably we are missing format: bytes somewhere in the fields.yml. If we have this in, Kibana would convert the value to MB / KB etc. which would be nicer I think.

@ruflin ruflin added module review Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Jan 22, 2019
@ChrsMark
Copy link
Member Author

ChrsMark commented Jan 22, 2019

Nice! Will update it according to the suggestions!

PS: @ruflin to make your life easier with manual testing:
you can spin up a nats container and feed it will artificial traffic using nats benchmarks (https://github.com/nats-io/go-nats/tree/master/examples/nats-bench) written in Go lang. I run the benchmark as: ./nats-bench -np 50 -ns 50 -n 1000000 -ms 16 16 foo in order to produce significant traffic. Feel free to ping if you have trouble with this.

@sayden
Copy link
Contributor

sayden commented Jan 22, 2019

Hi @ChrsMark 🙂 Amazing contribution, dashboards are very important for each module.

I think the dashboard wasn't exported correctly. I get this when trying to open it:

image

Also, I cannot open each visualization. It just hangs when I clicked them, first time I see something similar.

Can you tell us how did you export the dashboard?

Thanks!

@ChrsMark
Copy link
Member Author

ChrsMark commented Jan 22, 2019

Hmm, this is an error on my end. I had the visualizations already in Kibana from previous step so when I reloaded the dashboard with make import-dashboards everything was fine.

Let me tune the visualization IDs.

@sayden, I followed the steps below:

  1. ./metricbeat export dashboard -id 7fea2930-478e-11e7-b1f0-cb29bac6bf8b >> Metricbeat-nats.json
  2. I had to remove some stuff like searchSourceJSON fields and replace some machine created IDs with named ones.
  3. make import-dashboards

Is this the normal workflow?

@ChrsMark
Copy link
Member Author

ChrsMark commented Jan 22, 2019

Finally I made it work smoothly with:
MODULE=nats ID=c359a6c0-1e45-11e9-a1b4-79a7ae42ab61 mage exportDashboard,
@sayden

Change regarding fields (conversion to bytes) will be addressed on another commit. (cc: @skatsaounis, @MichaelKatsoulis )

@jsoriano
Copy link
Member

@ChrsMark this is looking great 🙂

As an extra suggestion, I'd propose to use more line/area graphs and less gauges and numbers, for example for the number of consumers we could have a graph like the one for subscriptions. Also, for the volume of messages we could have graphs with two scales, one for number of messages and another one for its size in bytes, and something similar for cpu usage.

I think gauges are great for dashboards that are not usually operated, like the ones that can be seen in a screen in the wall 🙂 But when operating or investigating an incidence in an specific period of time I find graphs with the evolution of the values much more useful.

@ChrsMark
Copy link
Member Author

I think most comments addressed!

screenshot from 2019-01-23 01-03-04

@ruflin
Copy link
Member

ruflin commented Jan 23, 2019

As #10249 was merged this will require a rename of the dir 6 to 7.

@sayden
Copy link
Contributor

sayden commented Jan 23, 2019

Awesome @ChrsMark The chart can be loaded already.

As a general workflow, a user will load the dashboard by doing the setup with metricbeat setup. All dashboards are loaded into Kibana with this command. This is the workflow that must be tested to ensure everything is working when importing the dashboard.

Few minor things left that I didn't see/realized before (sorry):

  • A description on the dashboard is missing.
  • The names of the visualizations don't follow naming conventions. For example NATS IO should be IO [Metricbeat NATS]. Nats Memory should be Memory [Metricbeat NATS]. Take a look at the rest of visualization charts to see more examples. You did it correctly with the dashboard 👍

Finally, but I'm not fully sure why, some index patterns are missing. I don't have any good suggestion:

image

@ruflin do you have any idea of why some of the index pattern fields have not been uploaded?

Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Overall looks very good. Left some comments

@sayden
Copy link
Contributor

sayden commented Jan 23, 2019

To fix the conflicts you'll need to rebase over master and then push the changes in those files after running make update

@skatsaounis
Copy link
Contributor

Hi all,

I have made the rebase locally and also worked on your comments.

Tomorrow I am going to push a new patch which also contains changes to some field types (eg. long instead of integer etc) based on NATS output.

ChrsMark and others added 4 commits January 24, 2019 10:48
Signed-off-by: Chris <chrismarkou92@gmail.com>

Co-Authored-By: Stamatis Katsaounis <katsaouniss@gmail.com>
Co-Authored-By: Michael Katsoulis <michaelkatsoulis88@gmail.com>
Signed-off-by: Chris <chrismarkou92@gmail.com>
Signed-off-by: Chris <chrismarkou92@gmail.com>
Signed-off-by: Chris <chrismarkou92@gmail.com>
@skatsaounis
Copy link
Contributor

Hi all,

I rebased to today's master branch and also updated some fields and the dashboard as well.

Below, you can see a screenshot with the new changes:
screenshot from 2019-01-24 11-46-41

Signed-off-by: Stamatis Katsaounis <katsaouniss@gmail.com>
Co-Authored-By: Christos Markou <chrismarkou92@gmail.com>
Co-Authored-By: Michael Katsoulis <michaelkatsoulis88@gmail.com>
@skatsaounis
Copy link
Contributor

@ruflin Hi Travis is green. Can you trigger Jenkins as well? :)

@ruflin
Copy link
Member

ruflin commented Jan 24, 2019

jenkins, test this

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.

Tested this locally and seems to work as expected. Unfortunately my graphs don't look as nice as I don't that nice data ;-)

@ruflin ruflin merged commit 8349e70 into elastic:master Jan 28, 2019
@ruflin
Copy link
Member

ruflin commented Jan 28, 2019

@skatsaounis Any chance you could open a follow up PR to the docs with the above dashboard images so it shows up here? https://www.elastic.co/guide/en/beats/metricbeat/master/metricbeat-module-nats.html Would be nice to show to users in advance what kind of dashboard they get.

@skatsaounis
Copy link
Contributor

Hi @ruflin. Thank you for merging this PR. I am going to update the docs; no problem at all :) I will mention you to the new PR when it will be ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Metricbeat Metricbeat module review Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants