Fix #753: Enable CDEF/calculated data source value alerting - #798
Fix #753: Enable CDEF/calculated data source value alerting#798bmfmancini wants to merge 2 commits into
Conversation
thold_modify_values_by_cdef() had two logic bugs that prevented CDEF-based thresholds from triggering: 1. Contradictory condition: The CDEF was only looked up from graph items when data_type != 1 OR cdef was empty, but only applied when data_type == 1. When a user explicitly set a CDEF on a threshold (data_type=1, cdef set), the lookup was skipped and $cdef stayed false, so the CDEF was never applied to the threshold values. 2. Double transformation: lastread was already CDEF-transformed by thold_poller_output (or thold_process.php for daemon mode) before being stored in the database. thold_modify_values_by_cdef() then applied the CDEF to lastread again, producing an incorrect value. The fix restructures the CDEF lookup to use the threshold's explicitly configured CDEF when data_type=1, falling back to auto-detection from graph items. It also removes the redundant lastread transformation, applying the CDEF only to the threshold values (thold_hi, thold_low, etc.) so the comparison between the already-transformed lastread and the threshold values is consistent.
somethingwithproof
left a comment
There was a problem hiding this comment.
The explicit-CDEF selection and removal of the second lastread transform need behavioral proof because this changes all CDEF threshold comparisons. Please add Cacti-Composer/Pest cases for an explicitly configured CDEF, graph-item auto-detection, no CDEF, and both poller/daemon lastread paths. Also remove the three unrelated threshold_value hunks from #789 and add a CHANGELOG entry. The current integration matrix is red.
|
All four checks fail before this plugin loads because the branch’s older workflow tests unpinned Cacti develop, which crashes in core with an undefined __() call. Maintainer edits are disabled, so I cannot apply the pinned-Cacti and bounded-retry workflow fix. Please enable maintainer edits or update/rebase the branch. |
Summary
Fixes #753
Problem
Thresholds configured against data sources that use CDEF (calculated) values do not trigger alerts. The threshold evaluation compares against the raw RRD value rather than the CDEF-transformed value, so alerts never fire when the threshold condition is only met after the CDEF calculation.
Root Cause
thold_modify_values_by_cdef()inthold_functions.phphad two logic bugs:1. Contradictory CDEF lookup condition
The original code:
When a user explicitly configures a CDEF on a threshold (
data_type=1,cdefset), the firstifevaluates tofalse || false = false, so the DB query is skipped and$cdefremainsfalse. The secondifthen checks$cdef !== falsewhich fails, so the CDEF is never applied to the threshold values (thold_hi,thold_low, etc.).This means the already-CDEF-transformed
lastreadwas compared against raw threshold values, causing alerts to never fire (or fire incorrectly).2. Double transformation of
lastreadlastreadis already CDEF-transformed bythold_poller_output()(orthold_process.phpin daemon mode) before being stored in the database.thold_modify_values_by_cdef()then applied the CDEF tolastreadagain, producing an incorrect (double-transformed) value.Fix
Restructured
thold_modify_values_by_cdef():data_type=1andcdefis set, falling back to auto-detection from graph items only whencdefis empty.lastreadtransformation —lastreadis already CDEF-transformed before storage. The CDEF is now applied only to the threshold values (thold_hi,thold_low,thold_warning_hi,thold_warning_low,time_hi,time_low, etc.) so the comparison is consistent.Testing
thold_poller_output→thold_check_threshold) and the daemon path (thold_process.php→thold_check_threshold) apply the CDEF tolastreadbefore storage, confirming the double-transformation was real.