Skip to content

Fix #753: Enable CDEF/calculated data source value alerting - #798

Open
bmfmancini wants to merge 2 commits into
Cacti:developfrom
bmfmancini:fix/753-cdef-calculated-values-alerting
Open

Fix #753: Enable CDEF/calculated data source value alerting#798
bmfmancini wants to merge 2 commits into
Cacti:developfrom
bmfmancini:fix/753-cdef-calculated-values-alerting

Conversation

@bmfmancini

Copy link
Copy Markdown
Member

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() in thold_functions.php had two logic bugs:

1. Contradictory CDEF lookup condition

The original code:

if ($thold_data['data_type'] != 1 || empty($thold_data['cdef'])) {
    // Query graph items for a CDEF
    $cdef = db_fetch_cell_prepared(...);
}

if ($cdef !== false && $cdef > 0 && $thold_data['data_type'] == 1) {
    // Apply CDEF to threshold values
}

When a user explicitly configures a CDEF on a threshold (data_type=1, cdef set), the first if evaluates to false || false = false, so the DB query is skipped and $cdef remains false. The second if then checks $cdef !== false which fails, so the CDEF is never applied to the threshold values (thold_hi, thold_low, etc.).

This means the already-CDEF-transformed lastread was compared against raw threshold values, causing alerts to never fire (or fire incorrectly).

2. Double transformation of lastread

lastread is already CDEF-transformed by thold_poller_output() (or thold_process.php in daemon mode) before being stored in the database. thold_modify_values_by_cdef() then applied the CDEF to lastread again, producing an incorrect (double-transformed) value.

Fix

Restructured thold_modify_values_by_cdef():

  1. Use the explicitly configured CDEF when data_type=1 and cdef is set, falling back to auto-detection from graph items only when cdef is empty.
  2. Removed the redundant lastread transformationlastread is 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

  • Verified both the standard poller path (thold_poller_outputthold_check_threshold) and the daemon path (thold_process.phpthold_check_threshold) apply the CDEF to lastread before storage, confirming the double-transformation was real.
  • No new lint/compile errors introduced.

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.
Copilot AI lite review requested due to automatic review settings August 17, 2026 03:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

Thold cannot alert on CDEF/calculated data source values

2 participants