sensu-plugins / sensu-plugins-memory-checks

This plugin provides native memory instrumentation for monitoring and metrics collection, including: memory usage via `free` and `vmstat`, including metrics. Note that this plugin may have cross-platform issues.
http://sensu-plugins.io
MIT License
15 stars 50 forks source link

Check Ram includes cache rather than checking memory in use #34

Closed AirCombat closed 7 years ago

AirCombat commented 8 years ago

Check Ram includes cache rather than checking memory in use

josephholsten commented 8 years ago

This is a rather annoying difference between the check-memory.sh and check-ram.rb. Unfortunately, I don't understand the vmstat gem well enough to pull out the buffer and cache memory.

majormoses commented 8 years ago

From what I remember from when I rewrote this check and the comment I left for myself I the output will not be the same as free. Here are the details: https://github.com/threez/ruby-vmstat/issues/4 . I can look some time this week to see if there is anything new available in order to better represent what people are expecting...

majormoses commented 8 years ago

I dont think that this behavior will change because every *nix based system has a slightly different way of outputting memory information. This is the most common approach that I have seen across linux and unix platforms. I do think that we should call some of this out in the readme. @sstarcher what do you think?

sstarcher commented 8 years ago

To support multiple *nix systems it would require having logic for each system that differs. Whatever is done it should not break the current systems it works on.

majormoses commented 8 years ago

@sstarcher there are too many places that you can get memory information from and depending on your OS type, distribution, and even versions the data will be displayed so parsing out from a shellout is dangerous. This is the best attempt I saw that was made readily available with ruby. To be clear its not "broken" on the platforms it currently runs on it returns not incorrect data that some might not expect or agree with. I dont see how we can make everyone happy without a massive amount of effort and have lots of logic to check things os type, distro, version of util, etc and offer every option.

I feel like we have 2 options:

  1. have separate checks based on the implementation type: free, meminfo and then the user can choose the implementation they like and all of the support can be much simpler to support.
  2. have 1 massive check script that based on args and have that logic determine this. This just feels like this is a worst version of the previous option.
sstarcher commented 8 years ago

Certainly from a user's perspective the section option is preferable, but much harder to support.

majormoses commented 8 years ago

@sstarcher I agree that the second might be nicer from the user perspective but I just dont see it as feasible unless someone wants to donate a ton of time for this. I think its hard enough to maintain the functionality separately. I think we should make a major release where we rename the checks to follow a pattern something like this:

check-memory-free.rb check-memory-meminfo.rb check-memory-vmstat.rb

And then we can start incrementally making each one as good as each use case and time allows. people will get the format that they expect.

AirCombat commented 7 years ago

The thing is, I'm not using some niche distro, I'm using RHEL/CentOS. A solution that covers RHEL/CentOS and Debian/Ubuntu based should cover the vast majority of users.

majormoses commented 7 years ago

@AirCombat I understand what you are saying but its not quite that simple as even within RHEL, centos, debian, etc. versions memory is reported differently using using solutions that rely on free as it has changed how it calculates and displays between versions. If we were to switch all to use /proc/meminfo or using something like vmstat the data will not 100% match the information returned from free which is apparent what many users expect. I think the best solution is to provide multiple checks with different implementations and let any implementation specific logic for each os, distro, and tool versions be self contained. This gives the ability to choose which implementation you want to use and we can work on more specific edge cases as far as versions etc.

majormoses commented 7 years ago

I have not looked through it enough but I think we might be able to have a happy medium for linux users: https://github.com/threez/ruby-vmstat/issues/16 @AirCombat would that work for you? I can put togethor something when I have some time if it does.

AirCombat commented 7 years ago

Sounds like a plan!

mikeeaton83 commented 7 years ago

@majormoses - Did you get around to doing anything yet?

majormoses commented 7 years ago

Sorry I have been busy and this fell off my radar. I am at Elasticon this week so I probably won't have time to take a look before the weekend.

majormoses commented 7 years ago

@if-meaton ok I lied and took a quick look and it was quite easy to do. I have not done a lot of platform testing yet; please feel free to test and let me know.

majormoses commented 7 years ago

@AirCombat can you please test my branch?

AirCombat commented 7 years ago

Sorry to say this but since last week I am in a new job and not using Sensu for the time being. Will let you know but you probably won't hear from me for a few months. I don't really have a platform to test it on. I recommend trying it on CentOS 1611 which is the latest rolling build.

I originally had this issue on 1511.

majormoses commented 7 years ago

I have tested it so far only on ubuntu 16.04 I am sure it would work on any linux system with a new enough kernel 3.14 What I have not done testing on yet is stuff like osx and freebsd.

majormoses commented 7 years ago

@AirCombat unfortunate for sensu community but understandable, I wish you well in your new position!

mikeeaton83 commented 7 years ago

@majormoses Hey, sorry the boxes I'm having this issue on run 3.10

majormoses commented 7 years ago

ok I unfortunately don't have a great solution for you atm without an upgrade. Will continue to keep my eyes and ears open...