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

Improve processes plugin #69

Closed
oli-g opened this issue Sep 2, 2016 · 12 comments
Closed

Improve processes plugin #69

oli-g opened this issue Sep 2, 2016 · 12 comments
Assignees

Comments

@oli-g
Copy link
Contributor

oli-g commented Sep 2, 2016

Hi @anryko

We're using your Grafana Dashboard in order to display the Collectd metrics we're gathering from our containers. It's nice and I thank you for your effort to maintain this project.

We would like to improve the processes plugin, and display a graph with RSS for selected process: Collectd documentation states that it's possible to select more detailed statistics by using the Process and ProcessMatch directives.

We are very keen to help on this one, maybe can you suggest us where to start having a look?

Thank you.

@anryko
Copy link
Owner

anryko commented Sep 3, 2016

Hi @oli-g

Glad you like it :).

I will be able to look in to this mid next week for you. I will have to enable this collectd processes plugin feature and check how data is structured in influxdb to know exactly what to do.

To start figuring it out on your own you should read Configuration HOWTO. Understand what additional data is saved in influxdb and how it is different from the data already matched by plugin configuration. Most likely there will be a separate metric. something like processes_rss. In that case you would need to add configuration to reflect this metric in the graph in to already defined processes plugin.

@anryko
Copy link
Owner

anryko commented Sep 7, 2016

Hey @oli-g

I added some initial support for extended processes metrics. Please check it out and tell me what you think. I can't make sense of ps_stacksize numbers. Maybe you will know what is wrong with numbers on this graph.
Unfortunately there is an aggregation bug and instance metrics are duplicated in merged graph. I have no idea why this happens at the moment :). Will have to investigate it later.

Looking forward to your feedback.

@oli-g
Copy link
Contributor Author

oli-g commented Sep 8, 2016

Wow! Good job, thank you! I'm going to have a look now and leave you some comments!

@oli-g
Copy link
Contributor Author

oli-g commented Sep 8, 2016

I added some minor comments to your commit. Additionally I can confirm you that I have merged graphs with duplicated metrics too, but just for Processes VM, Processes Stack Size, Processes RSS and Processes Code and Data graphs. Actually, for the first 3 graphs, I think that the aggregated version could be better, otherwise we would have 1 graphs with just 1 line for every process we're monitoring. What do you think?

Thank you for your effort!

@anryko
Copy link
Owner

anryko commented Sep 8, 2016

I agree that it is convenient to have it aggregated. Unless you have more than 26 processes per host. In that case it will not work because grafana doesn't allow more than 26 queries per graph (26 comes from A-Z letters used as id's).
What do you think? How likely it is that somebody who uses extended processes metrics enabled will run in to this limitation?
What is your take on Processes Stack Size? I see some huge numbers there that don't make sense... but maybe it's just me.

@anryko
Copy link
Owner

anryko commented Sep 9, 2016

Actually in Grafana3.1.1 there is no 26 query limit. It just shows empty space instead of a letter after 'Z'. Not sure if this is a bug or a feature :). This will permit to aggregate without this limitation.

@anryko
Copy link
Owner

anryko commented Sep 11, 2016

Hey @oli-g

I fixed that duplication bug and split the plugin in two. processes and process, because I can't do splinting graphs by instance and aggregating by instance in the same plugin :). Contradicting requirements. Now functionality is in part duplicated. In processes you can see all metrics per process and in process you see aggregated one metric graphs. If you don't need both, you can comment out undesired panels in plugin.

Looking forward to your feedback.

@oli-g
Copy link
Contributor Author

oli-g commented Sep 14, 2016

Hi @anryko

Thank you for your effort on this! The dashboard looks cool, but from your last commit, the graphs Processes State and Processes Fork Rate (which are defined here and here) are no longer displayed. Is this intended or is it a side effect of your change? Should't they belong to the process plugin, and not to the processes one?

Actually we need those graphs as well.

anryko added a commit that referenced this issue Sep 14, 2016
@anryko
Copy link
Owner

anryko commented Sep 14, 2016

Thanks for checking!
Fixed.

anryko added a commit that referenced this issue Sep 19, 2016
@anryko
Copy link
Owner

anryko commented Sep 19, 2016

Merged to master. If you have any other suggestions please let me know.
Thank you for your help!

@anryko anryko closed this as completed Sep 19, 2016
@oli-g
Copy link
Contributor Author

oli-g commented Sep 22, 2016

Thanks for merging it! I have another suggestion indeed, I'll open a new issue.

@anryko
Copy link
Owner

anryko commented Sep 22, 2016

Cool. Can't wait for it! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants