[ZBXNEXT-2969] Negative trigger function parameters Created: 2015 Sep 18  Updated: 2016 Oct 18  Resolved: 2016 Oct 18

Status: Closed
Project: ZABBIX FEATURE REQUESTS
Component/s: Server (S)
Affects Version/s: 3.0.0alpha2
Fix Version/s: 3.0.6, 3.2.2, 3.4.0alpha1

Type: Change Request Priority: Minor
Reporter: Glebs Ivanovskis (Inactive) Assignee: Unassigned
Resolution: Fixed Votes: 3
Labels: expressions, negative, triggerfunctions, usability
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Zip Archive is_time_suffix_unittests.zip    

 Description   

Currently all Zabbix trigger functions accept zero or positive numeric parameters because negative values do not really make much sense for them. However, ZBXNEXT-922 will introduce two new trigger functions for making predictions and support for negative parameters might be necessary to unleash their full potential.

Consider host:item.timeleft(...,...,threshold).
We have items which can become negative (recall free disk space percentage available to non-root user on FreeBSD) therefore negative can be threshold as well.
A simple generalization of our get_function_parameter_float() and additional check 0 <= percentage in evaluate_PERCENTILE() will suffice.

Beneath fancy name of host:item.forecast(...,...,time) is quite simple least squares fitting procedure, so someone might want to use this function for interpolation of item value between not so frequent checks for example.
Support for negative time would provide ability to calculate value of fitted expression at any time moment, both in the future and in the past.
Also, this would be useful for prediction method verification when we compare prediction with values we've already got. (Say, today is Wednesday, we make forecast for Tuesday based on Monday data and compare it with data gathered yesterday.)
Adding new "negative" flag to __get_function_parameter_uint31() is a way to implement this.



 Comments   
Comment by Glebs Ivanovskis (Inactive) [ 2016 Aug 23 ]

Also, it turns out that you can't use suffixes in "threshold" parameter of timeleft() trigger function.

Comment by Glebs Ivanovskis (Inactive) [ 2016 Aug 24 ]

Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBXNEXT-2969 revision 61897.

On the way improved frontend validation of fit and mode parameters of forecast() and timeleft() as well as mask parameter of band() in trigger expressions. Also made nodata(0) return an error, but not 100% about this change. We can revert it and revise nodata() separately in ZBX-11125.

Comment by Sandis Neilands (Inactive) [ 2016 Sep 01 ]

(1) [S] In __get_function_parameter_int() when casting unsigned integer to signed integer we rely on implementation-defined behavior. For now - at least add a comment explaining the situation. If there is time then changing the type either upwards or downwards would be preferable.

glebs.ivanovskis I tried to improve situation in r62285. RESOLVED

sandis.neilands CLOSED.

Comment by Sandis Neilands (Inactive) [ 2016 Sep 01 ]

(2) In get_function_parameter_int() pass ZBX_FLAG_SEC instead of literal 0 as it was done before. ZBX_FLAG_SEC might get redefined...

glebs.ivanovskis I think a better way to extract sec|#num parameter would look approximately like r62456.
RESOLVED

sandis.neilands CLOSED.

Comment by Sandis Neilands (Inactive) [ 2016 Sep 02 ]

Tested.

  • Frontend developers should take a look at the changed PHP file.
  • Regarding nodata(0) - documentation recommends not using values less than 30. Not sure if 0 should be explicitly handled in validation...
  • Documentation changes.
Comment by Sandis Neilands (Inactive) [ 2016 Sep 21 ]

(3) Now percentile() in server allows suffixed percentiles.

glebs.ivanovskis RESOLVED in r62718.

sandis.neilands Traditionally signed integers are not used for bitfield flags. Consider using fixed-length unsigned integer (unsigned char seems popular in our code).

sandis.neilands Also introduce a constant for 0.

glebs.ivanovskis Makes sense. RESOLVED in r62728 and r62730.

sandis.neilands CLOSED.

Comment by Sandis Neilands (Inactive) [ 2016 Sep 21 ]

(4) Define 6 and -1 as constants (defaults for queue 'from' and 'to' parameters). Use them in DCget_item_queue() as well as recv_getqueue(), get_value_internal() and any other places where you can find these magic numbers.

Actually you can open this as new ZBX since it wasn't introduced in this ZBXNEXT.

glebs.ivanovskis RESOLVED in r62715.

sandis.neilands Thanks, CLOSED.

Comment by Glebs Ivanovskis (Inactive) [ 2016 Sep 23 ]

(5) [D] Speaking of zabbix[queue,<from>,<to>], documentation says

Number of monitored items in the queue which are delayed by <from> to <to> seconds, inclusive.

Looking at the code I would say that items with delay of <to> seconds are not counted.

sandis.neilands Right. The logic for zabbix[queue, 0, 5] works as follows: don't include item if ((now - item's nextcheck < 0) or (now - item's nextcheck >= 5)).

zabbix[queue, 0, 5]   --> [0, 5) --> items delayed by 0, 1, 2, 3, 4 seconds.
zabbix[queue, 5, 10]  --> [5, 10) --> items delayed by 5, 6, 7, 8, 9 seconds.
zabbix[queue, 10, 15] --> [10, 15) -->  items delayed by 10, 11, 12, 13, 14 seconds.

If the check is used this way then it actually makes sense to exclude the last second from the range.

sandis.neilands However this begs the question - why is 6 the default instead of 5...

glebs.ivanovskis If next check is scheduled in the future, it is not a delay. Did you mean "in the past"? Regarding 5 and 6 - no idea. Maybe to compensate for 1 second poller sleep interval... Thank you for confirming my suspicion, I've corrected https://www.zabbix.com/documentation/3.2/manual/config/items/itemtypes/internal#supported_checks

sandis.neilands Yes, thanks for pointing that out. CLOSED.

glebs.ivanovskis Synced changes to 2.2, 2.4 and 3.0 documentation.

Comment by Sandis Neilands (Inactive) [ 2016 Sep 23 ]

Tested, frontend developers should take a look at the modified PHP file.

Comment by Alexander Vladishev [ 2016 Sep 27 ]

sandis.neilands, sub-issues (1) and (2) still open. Please check.

Comment by Alexander Vladishev [ 2016 Sep 27 ]

(6) [F] timeleft()

  • the third parameter accepts numbers with the leading plus character "+". I think, it must be prohibited
  • pattern modifier "u" is useless here
define('ZBX_PREG_NUMBER', '([\-+]?[0-9]+[.]?[0-9]*['.ZBX_BYTE_SUFFIXES.ZBX_TIME_SUFFIXES.']?)');

private function validateNumSuffix($param) {
    return preg_match('/^'.ZBX_PREG_NUMBER.'$/u', $param);
}

glebs.ivanovskis Removed "u" in r62832. Regarding "+" I'm not so sure... This would affect how frontend resolves positional macros in trigger names. We don't have a strict definition of constant. What should I see in trigger name "$1 $2 $3" if trigger expression is "{host:key.func()}=0+1-2"? Standard C functions strtod() and atof() allow optional plus sign, while JSON number definition does not. Different server side validation can't agree on what is and what is not a decimal number (ZBX-10784). So I wouldn't touch the plus in this issue and leave this problem for the future. Partially RESOLVED

glebs.ivanovskis Prohibited plus sign in r62839. RESOLVED

sasha Thanks! CLOSED

Comment by Alexander Vladishev [ 2016 Sep 27 ]

(7) [F] Have a look at my changes in r62820.

glebs.ivanovskis Thank you, accepted. CLOSED

Comment by Glebs Ivanovskis (Inactive) [ 2016 Sep 30 ]

Available in pre-3.0.6 r62906, pre-3.2.2 r62918, pre-3.3.0 (trunk) r62919.

Comment by Glebs Ivanovskis (Inactive) [ 2016 Sep 30 ]

(8) There were conflicts when I merged from 3.0 to 3.2 in:

  • src/libs/zbxserver/evalfunc.c
  • src/libs/zbxserver/expression.c
  • src/zabbix_server/poller/checks_calculated.c

is_double_suffix() tried to escape from me . sandis.neilands, please have a look.

sandis.neilands CLOSED.

Comment by Glebs Ivanovskis (Inactive) [ 2016 Oct 06 ]

Documented in:

sandis.neilands Several comments.

  • Which suffixes are supported for threshold (timeleft())?
    • The term "suffix" is not used in the trigger function documentation anywhere else.
    • Is "Unit symbols" page complete? How about KMGT suffixes?
  • The semantics of negative time are not described for forecast() and can be confusing.
    • sec, time_shift "go back in time" with positive number, time indicates past with negative number, future with positive number;
    • Description says: "Future value, max, min, delta or avg of the item."
    • Consider adding an example with short description.

Also please check if "Predictive trigger functions" page and the linked PDF have to be refreshed due to this change.

glebs.ivanovskis My answers.

  • That's right
    • Replaced "suffixes" with "unit symbols" and a link.
    • "Unit symbols" page looks good, except captions "Time unit suffixes" and "Prefix symbols" are a bit contradictory.
  • I disagree.
    • In forecast() description "Note that:" part describes precisely how time is used and what is calculated.
    • I would prefer concise and comprehensible description, even if it's not 100% precise. After all, "-future = past".
    • Example added.

"Predictive trigger functions" describes future prediction, I think there is no need for changes there. In PDF we can remove Note from 1.7, but I don't think it's an absolute necessity given that PDF has a link to this issue.

I will transfer latest changes from 3.0 docs to other versions as soon as you are happy with them.

sandis.neilands Thanks! CLOSED. Opened a separate ZBX for prefix/suffix issue in "Unit symbols" page: ZBX-11327.

glebs.ivanovskis Copied aforementioned changes from 3.0 to 3.2 and 3.4.

Comment by Sandis Neilands (Inactive) [ 2016 Oct 17 ]

The attached code tests is_time_suffix(). In order to test that function upon modification one hast to copy-paste it in function.c and add relevant stubs (if any).

Generated at Fri Mar 29 16:50:23 EET 2024 using Jira 9.12.4#9120004-sha1:625303b708afdb767e17cb2838290c41888e9ff0.