FreeBSD: Calculate memory stats using sysctl instead of relying on statgrab

[brd]

Enhancement to allow checkmk to calculate memory usage without relying on an external dependency.

[brd]

ping?

[mo-ki]

Hi Brad!
Thank you for your contribution!
Could you be a bit more explicit in the rationale why this is a good idea? If I understand correctly, sysctl is shipped with FreeBSD and statgrab is not?

[brd]

Exactly, gather more data with builtin tools incase statgrab is not available for some reason.

[mo-ki]

This is a somewhat hacky approach in the following sense:
You are using sysctl to mimic the output format of the statgrab_mem section, such that the output can be parsed by the same code. The more native way to go would be to create dedicated parsing code on server side:

• output a new section here, <<<sysctl_mem>>> (for instance). You can output the information in any format that is convenient
• add a new "agent section" on server side, parsing this new dataset. I think it should go here: https://github.com/Checkmk/checkmk/blob/master/cmk/base/plugins/agent_based/mem_used_sections.py
• by setting the same value for parsed_section_name you ensure that your section will be used by all plugins subscribing to the sections "mem_used".
• Don't forget to add a test! (even though the status quo is embarrasing: https://github.com/Checkmk/checkmk/blob/master/tests/unit/cmk/base/plugins/agent_based/test_mem_used_sections.py)

[brd]

I figured it would be easier to stay compatible with the existing way statgrab works to keep complexity to a minimum. Are you sure you'd rather it done this way?

[xbeaudouin]

Hello,
I would like to help about this patch.
I agree with the idea of <<<sysctl_mem>>> but there is some tools that relies on <<<statgrab_mem>>> and have "difficulties" to be updated to <<<sysctl_mem>>> if this section exists only on FreeBSD platform, which you know is less deployed than others.
How can I help?

[mo-ki]

Oh, no! I completely lost track of this :-( . I am so sorry!

I'm not sure if I understand your question. Generally, it should never be a problem if a section is not present in the output. As far as I can see, <<<statgrab_mem>>> is only mentioned in some places that are a relic from the time there was a dedicated plugin.

Maybe I should elaborate: These days (>2.0), we have one additional layer of abstraction: The difference between raw and parsed sections. For example: The check plugin "mem_used". It doesn't have an explicit sections argument, so it subscribes to exactly one section, by the same name as itself by default. That means, whenever there is a parsed section by the name mem_used, this plugin will be called.

Looking at it from the agent's end: The agent creates a raw section with the name statgrab_mem. The server receives this output, and picks the agent section plugin by that name. It now applies the parse function, and stores the result under the parsed section name.

Now we have a parsed section called "mem_used", and the plugin will be called.

In this instance we already have several sections that create a "mem_used" section in the end ("aix_memory", "solaris_mem", "statgrab_mem", "openbsd_mem"). The check plugin in end does not need to know where the parsed section originated from -- it will just do its thing.

The advantage of this approach is that you move a lot of the parsing logic into the python world on server side, which makes it much easier to test.

[xbeaudouin]

Hello.
Sorry for the late reply, I have looked about check_mk code. And I feel a bit puzzled.
On FreeBSD, the package check_mk_agent contains (mostly) only agents/check_mk_agent.freebsd and that's all. No plugins are installed or ported.
So I understand better the idea of @brd for this patch.
In a nutshell, do we need to install plugins to have memory information ?

[mo-ki]

That would be an option, but no, that is not what I meant. It is always a bit of a judgement call what goes directly into the agent, and what should be a plug-in, but I think memory is something that should unquestionably be monitored out of the box.

Im afraid it is quite confusing with all the different things we refer to as "plug-in". In this case only few are relevant (but need to be distinguished):

• "agent plug-ins": plugins to the agent, deployed on the monitored host, found in agents/plugins. Not wanted here.
• "agent section plugin": a plug-in to the check- and discovery engine, run on the monitoring server, found in cmk/base/plugins/agent_based or - preferably - cmk/plugins/<FAMILY>/agent_based. This is what we need.
• "check plug-in": also a plug-in to the check- and discovery engine, run on the server, found in same folder. By using the "parsed_section_name" feature we don't have to write it.

There are two files that need to be adjusted here I think:

1. The agent file (and only this needs to be deployed on the monitored host): I think the agent should try to create the new section sysctl_mem, and only if that fails fall back to statgrab_mem (a construct that can be seen here, but we don't need the condition and the logging).
2. The agent section plugins as linked above. This file does not need to be deployed to the monitored host, it is run by the server.

If you want, you could start by sending something unfinished as PR, so that I can try to guide you through the process via review step by step. This will require some back and forth, obviously.

[xbeaudouin]

Well, I cannot update @brd patch, but since sysctl is always here, I think we should present sysctl_mem even if statgrab_mem is here. I know the informations are quite redondant, but it will permit check_mk users to use either information they should have.
I have tried to add into freebsd-agent the work from @brd renamed sysctl_mem and it seems pretty good.
Do I have to open a new PR for this ?

[mo-ki]

I think that makes sense.

[TimotheusBachinger]

Dear Checkmk Contributor! Unfortunately, we had to re-write our git-repo history, rendering your PR auto-closed. We will therefore rebase your PR onto the current master and reopen it again. Sorry for the inconvenience.

[TimotheusBachinger]

Dear Contributor. Unfortunately, we learned that re-opening a PR which was force-rebased, is not possible (see isaacs/github#361). Therefore we kindly ask you to create a new PR for your change. We apologize for the circumstances and will implement technical measures to prevent such incidents in the future.