Skip to content

Conversation

@Yogibaer75
Copy link
Contributor

One bug was encountered where the rule setting to not compute trend data is not respected.
Better said it is ignored and trend data is computed every time.

As the default setting for this param is True i only check if it is set.

Thank you for your interest in contributing to Checkmk!
Consider looking into Readme regarding process details.

General information

Please give a brief summary of the affected device, software or appliance.
Keep in mind that we are experts in monitoring, but we cannot be experts on all supported devices.
A little context will help us assess your proposed change.

Bug reports

Please include:

  • Your operating system name and version
  • Any details about your local setup that might be helpful in troubleshooting
  • Detailed steps to reproduce the bug
  • An agent output or SNMP walk
  • The ID of a submitted crash report for reference (if applicable)

Proposed changes

Sometimes it is hard for us to assess the quality of a fix.
While it may work for you, it is our job to ensure that it works for everybody.
These are some ways to help us:

  • What is the expected behavior?
  • What is the observed behavior?
  • If it's not obvious from the above: In what way does your patch change the current behavior?
  • Consider writing a unit test that would have failed without your fix.
  • Is this a new problem? What made you submit this PR (new firmware, new device, changed device behavior)?

@Yogibaer75 Yogibaer75 changed the title Bug: All df checks don't repsect the trend_perfdata setting Bug: All df checks don't respect the trend_perfdata setting Feb 10, 2023
@mo-ki mo-ki self-requested a review February 20, 2023 14:30
@mo-ki
Copy link
Member

mo-ki commented Feb 20, 2023

The corresponding setting reads "Enable generation of performance data from trends".
Your proposed changes would skip the computation and checking of the trends entirely.
In my opinion that would be wrong.
We should just see no metrics generated, which as far as I can see is the case. Are you experiencing something different?

@Yogibaer75
Copy link
Contributor Author

Yogibaer75 commented Feb 20, 2023

At the moment the behavior is a little bit different.
Inside this "utils/df.py" the default settings are.

TREND_DEFAULT_PARAMS: Mapping[str, Any] = {
    "trend_range": 24,
    "trend_perfdata": True,  # do send performance data for trends
}

That means trend perfdata is generated automatically for every df check.
If you now want to switch of the trend generation you create a rule with this setting.
image
Result of this rule is nothing. Trend perf data is generated regardless if there is a rule or not.

My if statement is all the time true, only if such a rule like in the screenshot is create it is false.

if params.get("trend_perfdata"):

@mo-ki
Copy link
Member

mo-ki commented Feb 21, 2023

I still think it is working 🤔
Here's what I'm doing:
No rules, default config:

OMD[heute]:~$ cmk -nvp --plugins df heute | grep "heute/tmp"
Filesystem /opt/omd/sites/heute/tmp Used: 0.03% - 4.83 MiB of 15.5 GiB, trend per 1 day 0 hours: +13.2 GiB, trend per 1 day 0 hours: +84.75%, Time left until disk full: 1 day 4 hours (fs_used=4.828125;12731.128124;14322.51914;0;15913.910156 fs_free=15909.082031;;;0; fs_used_percent=0.030339;80;90;0;100 fs_size=15913.910156;;;0; growth=0;;;; trend=13487.406377;;;0;663.07959 inodes_used=1217;3666564.9;3870262.95;0;4073961)

Growth and trend metrics are there. Create a rule as seen in your screenshot:

OMD[heute]:~$ cmk -nvp --plugins df heute | grep "heute/tmp"
Filesystem /opt/omd/sites/heute/tmp Used: 0.03% - 4.83 MiB of 15.5 GiB, trend per 1 day 0 hours: +13.2 GiB, trend per 1 day 0 hours: +85.25%, Time left until disk full: 1 day 4 hours (fs_used=4.828125;12731.128124;14322.51914;0;15913.910156 fs_free=15909.082031;;;0; fs_used_percent=0.030339;80;90;0;100 fs_size=15913.910156;;;0; inodes_used=1217;3666564.9;3870262.95;0;4073961)

Growth and trend metrics are gone.

@Yogibaer75
Copy link
Contributor Author

That is more weird than what i wanted. My change is than more - don't calculate trend data.
In your example the whole trend data is existing in the output. This is the useless part i don't want to see.
The output is generated by the "yield from size_trend".
For my understanding of every good check it should be that a calculated value is also given as performance data.
If you don't want the trend data - why calculate it?

@mo-ki
Copy link
Member

mo-ki commented Feb 21, 2023

That is more weird than what i wanted. [...] If you don't want the trend data - why calculate it?

You have a valid point there, of course. I can't tell you why it is this way. From my experience, it's probably one of these:

a) It is a mistake, and was always intended the way you expected it to be, but never did.
b) It is a mistake, was always intended the way you expected it to be, and broke at some point.
c) Some customer wanted exactly the option of not recording the values, but computing them.

In this case I would put my money on c), but I wasn't able to find it out in a quick research.
The problem is: There are users relying on the current behavior. (Nothing is so weird that no-one relies on just that, see https://xkcd.com/1172/)
For those users your change would silently disable the monitoring of the file system trend -- I don't think we should do that.

Now that I have written all this: I will close this PR for that reason. Your desire to not show the site trend is understandable, but I don't think we can do it using this option.

Still, as always: Thanks for taking the effort of digging into these things!

@mo-ki mo-ki closed this Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants