diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b7db7d7..9fbdaad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * issue#710: Fixing Typo in thold_daemons.service File * issue#714: Increase the Name column to 255 characters * issue#719: Plugin Disabled due to mix of string and int +* issue#815: Keep counter sample values and timestamps synchronized, recover from backward sample clocks, preserve alert state while samples are unavailable, and fail closed when expression sources cannot be resolved * issue: All Columns checkd on Thresholds page * issue: Special character previous value handling broken on data query indexes with special characters diff --git a/README.md b/README.md index 46310dce..2c208fbc 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,24 @@ and become familiar with its settings. From there, you can provide overall control of thold, and set defaults for things like Email bodies, weekend exemptions, alert log retention, logging, etc. +Counter, derive, and absolute rate thresholds preserve the previous value and +timestamp together when a poll has no numeric sample, preventing the next rate +from using mismatched interval data. A gap of more than two effective sampling +intervals (the greater of the RRD step and poller interval), or the configured +RRD heartbeat when available, is treated as unknown while the current sample +starts a fresh baseline instead of producing a stale rate. Thresholds with an +unknown rate remain eligible for the next poll, but that poll preserves the +existing alert state and writes a warning instead of treating the missing +sample as a restoral. Warnings are emitted only when the threshold enters the +unavailable state. A backward sample clock is re-anchored without calculating a +rate for that cycle, so later samples can recover normally. Expression +thresholds require a numeric sibling value in the current poll and use its +cached DSStats rate, with an RRD fallback, even when the sibling has no +threshold row. This preserves COUNTER and DERIVE rate units without adding an +rrdtool process per expression during normal operation. They fail closed when +the current sibling value is unavailable. Gauge readings remain valid across +such a gap. + As with much of Cacti, settings should be documented in line with the actual setting. If you find that any of these settings are ambiguous, please create a pull request with your proposed changes. diff --git a/includes/polling.php b/includes/polling.php index c5c2c1d6..35afde9a 100644 --- a/includes/polling.php +++ b/includes/polling.php @@ -138,7 +138,7 @@ function thold_poller_output(&$rrd_update_array) { td.cdef, td.local_data_id, td.data_template_rrd_id, td.lastread, UNIX_TIMESTAMP(td.lasttime) AS lasttime, td.oldvalue, td.data_source_name AS name, dtr.data_source_type_id, - dtd.rrd_step, dtr.rrd_maximum + dtd.rrd_step, dtr.rrd_maximum, dtr.rrd_heartbeat FROM thold_data AS td LEFT JOIN data_template_rrd AS dtr ON dtr.id = td.data_template_rrd_id @@ -148,7 +148,8 @@ function thold_poller_output(&$rrd_update_array) { AND td.local_data_id IN($local_data_ids)"); if (cacti_sizeof($tholds)) { - $sql = []; + $sql = []; + $status_sql = []; foreach ($tholds as $thold_data) { thold_debug("Checking Threshold: Name: '" . $thold_data['thold_name'] . "', Graph: '" . $thold_data['local_graph_id'] . "'"); @@ -206,17 +207,13 @@ function thold_poller_output(&$rrd_update_array) { } } - // This stores the raw value into the data source and is important for - // Counters, where calculating the difference is important. - // The unset case is problematic and may lead to false triggering - // events. So, in those cases, we will store the 'oldvalue'. - if (isset($item[$thold_data['name']])) { - $rawvalue = $item[$thold_data['name']]; - } else { - $rawvalue = $thold_data['oldvalue']; - } + $sample_rows = thold_polling_sample_row($thold_data, $item, $currentval, $currenttime); - $sql[] = '(' . $thold_data['id'] . ', 1, ' . db_qstr($currentval) . ', FROM_UNIXTIME(' . $currenttime . '), ' . db_qstr($rawvalue) . ')'; + if ($sample_rows['sample_row'] !== null) { + $sql[] = $sample_rows['sample_row']; + } elseif ($sample_rows['status_row'] !== null) { + $status_sql[] = $sample_rows['status_row']; + } } if (cacti_sizeof($sql)) { @@ -233,13 +230,30 @@ function thold_poller_output(&$rrd_update_array) { oldvalue = VALUES(oldvalue)'); } - // accommodate deleted tholds - db_execute('DELETE FROM thold_data WHERE local_data_id = 0'); + } + + if (cacti_sizeof($status_sql)) { + foreach (array_chunk($status_sql, 400) as $chunk) { + $placeholders = implode(', ', array_fill(0, cacti_sizeof($chunk), '(?, ?, ?)')); + $params = []; - if (db_affected_rows() > 0) { - set_config_option('time_last_change_thold', time()); + foreach ($chunk as $row) { + $params[] = $row['id']; + $params[] = $row['tcheck']; + $params[] = $row['lastread']; + } + + db_execute_prepared('INSERT INTO thold_data + (id, tcheck, lastread) + VALUES ' . $placeholders . ' + ON DUPLICATE KEY UPDATE + tcheck = VALUES(tcheck), + lastread = VALUES(lastread)', + $params); } } + + thold_polling_cleanup(cacti_sizeof($sql) || cacti_sizeof($status_sql)); } return $rrd_update_array; diff --git a/tests/Unit/GetCurrentValueTest.php b/tests/Unit/GetCurrentValueTest.php index f467b9bc..85c0a087 100644 --- a/tests/Unit/GetCurrentValueTest.php +++ b/tests/Unit/GetCurrentValueTest.php @@ -96,6 +96,12 @@ public function testMissingDataSourceNamesReturnsZero(): void { $this->rrdReturns([]); $this->assertSame(0, get_current_value(4, 'traffic_in')); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); + $this->rrdReturns([]); + $this->assertSame('', get_current_value(4, 'traffic_in', 0, '')); } /** @@ -105,6 +111,12 @@ public function testMissingValuesReturnsZero(): void { $this->rrdReturns(['data_source_names' => ['traffic_in']]); $this->assertSame(0, get_current_value(4, 'traffic_in')); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); + $this->rrdReturns(['data_source_names' => ['traffic_out'], 'values' => [['1700000000' => 20.0]]]); + $this->assertSame('', get_current_value(4, 'traffic_in', 0, '')); } /** diff --git a/tests/Unit/TholdCalculatePercentTest.php b/tests/Unit/TholdCalculatePercentTest.php index 4266fa39..3231ff32 100644 --- a/tests/Unit/TholdCalculatePercentTest.php +++ b/tests/Unit/TholdCalculatePercentTest.php @@ -80,8 +80,8 @@ public function testZeroDenominatorGivesZeroRatherThanDividingByZero(): void { /** * @return void */ - public function testNonNumericDenominatorGivesZero(): void { - $this->assertSame(0, $this->percent('U')); + public function testNonNumericDenominatorYieldsTheNoValueSentinel(): void { + $this->assertSame('', $this->percent('U')); } /** diff --git a/tests/Unit/TholdGetCurrentvalTest.php b/tests/Unit/TholdGetCurrentvalTest.php index 4c1472cf..7cfbaf5b 100644 --- a/tests/Unit/TholdGetCurrentvalTest.php +++ b/tests/Unit/TholdGetCurrentvalTest.php @@ -38,12 +38,14 @@ public static function setUpBeforeClass(): void { */ private function threshold(array $overrides = []) { return $overrides + [ + 'id' => 9, + 'thold_id' => 9, 'local_data_id' => 4, 'name' => 'traffic_in', 'data_source_type_id' => self::COUNTER, 'rrd_step' => 300, 'rrd_maximum' => 0, - 'lasttime' => 0, + 'lasttime' => 1700000000, 'oldvalue' => 100, ]; } @@ -81,6 +83,34 @@ public function testAbsoluteDividesTheReadingByTheStep(): void { $this->assertEqualsWithDelta(2, $this->currentValue($thold, 600), 1.0e-9); } + /** + * @return void + */ + public function testAbsoluteRejectsStaleAndOutOfOrderIntervals(): void { + foreach ([1000 + 86400, 900] as $sample_time) { + $thold = $this->threshold(['data_source_type_id' => self::ABSOLUTE, 'lasttime' => 1000]); + $reindexed = [4 => ['traffic_in' => 600]]; + $time_reindexed = [4 => $sample_time]; + $item = []; + $currenttime = 0; + + $this->assertSame('', thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); + } + } + + /** + * @return void + */ + public function testGaugeRemainsValidAcrossAGap(): void { + $thold = $this->threshold(['data_source_type_id' => self::GAUGE, 'lasttime' => 1000]); + $reindexed = [4 => ['traffic_in' => 42]]; + $time_reindexed = [4 => 1000 + 7 * 86400]; + $item = []; + $currenttime = 0; + + $this->assertSame(42, thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); + } + /** * @return void */ @@ -106,10 +136,10 @@ public function testCounterTreatsAPreviousReadingOfZeroAsReal(): void { /** * @return void */ - public function testCounterWithNoPreviousReadingYieldsZero(): void { - $thold = $this->threshold(['oldvalue' => '']); + public function testCounterWithNoPreviousReadingYieldsUnknown(): void { + $thold = $this->threshold(['lasttime' => 0, 'oldvalue' => '']); - $this->assertSame(0, $this->currentValue($thold, 600)); + $this->assertSame('', $this->currentValue($thold, 600)); } /** @@ -119,7 +149,7 @@ public function testCounterWithNoPreviousReadingYieldsZero(): void { * @return void */ public function testThirtyTwoBitWrapUsesTheCorrectModulus(): void { - $thold = $this->threshold(['oldvalue' => 4294967290, 'rrd_step' => 1]); + $thold = $this->threshold(['oldvalue' => 4294967290, 'rrd_step' => 1, 'lasttime' => 1700000299]); $this->assertEqualsWithDelta(11, $this->currentValue($thold, 5), 1.0e-9); } @@ -128,7 +158,7 @@ public function testThirtyTwoBitWrapUsesTheCorrectModulus(): void { * @return void */ public function testSixtyFourBitWrapUsesTheCorrectModulus(): void { - $thold = $this->threshold(['oldvalue' => '18446744073709551610', 'rrd_step' => 1]); + $thold = $this->threshold(['oldvalue' => '18446744073709551610', 'rrd_step' => 1, 'lasttime' => 1700000299]); $this->assertEqualsWithDelta(11, $this->currentValue($thold, 5), 1.0e-9); } @@ -140,7 +170,7 @@ public function testSixtyFourBitWrapUsesTheCorrectModulus(): void { * @return void */ public function testSixtyFourBitWrapAcceptsScientificNotation(): void { - $thold = $this->threshold(['oldvalue' => '1.8446744073709552E+19', 'rrd_step' => 1]); + $thold = $this->threshold(['oldvalue' => '1.8446744073709552E+19', 'rrd_step' => 1, 'lasttime' => 1700000299]); $this->assertEqualsWithDelta(5, $this->currentValue($thold, 5), 1.0e-9); } @@ -173,4 +203,509 @@ public function testMissingDataSourceYieldsTheNoValueSentinel(): void { $this->assertSame('', thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); } + + /** + * @return void + */ + public function testLowerUpperCombinationRejectsUnknownInputs(): void { + $thold = ['local_data_id' => 4, 'upper_ds' => 'upper']; + + $this->assertSame('', thold_calculate_lower_upper($thold, '', [4 => ['upper' => 5]])); + $this->assertSame('', thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 'U']])); + $this->assertSame('', thold_calculate_lower_upper($thold, 7, [4 => []])); + $this->assertEqualsWithDelta((5 * 4294967296) + 7, thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 5]]), 1.0e-9); + $this->assertEqualsWithDelta((2147483648.0 * 4294967296) + 7, thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 2147483648]]), 1); + $this->assertEqualsWithDelta((4294967295.0 * 4294967296) + 7, thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 4294967295]]), 1); + $this->assertSame('', thold_calculate_lower_upper($thold, 7, [4 => ['upper' => -1]])); + $this->assertSame('', thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 4294967296]])); + } + + /** + * @return void + */ + public function testCdefAndNestedExpressionPreserveUnknownValues(): void { + $this->assertSame('', thold_build_cdef(1, '', 4, 5)); + $this->assertSame( + ['sample_row' => "(9, 1, '', FROM_UNIXTIME(1700000300), '700')", 'status_row' => null], + thold_polling_sample_row($this->threshold(), ['traffic_in' => 700], '', 1700000300) + ); + + $nested = $this->threshold([ + 'lasttime' => 1000, + 'rrd_heartbeat' => 600, + ]); + CactiStubs::willReturn('db_fetch_row_prepared', $nested); + $outer = $this->threshold(['expression' => '|ds:traffic_in|', 'lastread' => 2]); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1900]; + + $this->assertSame('', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + $this->assertSame([], CactiStubs::$log); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', $nested); + $time_reindexed[4] = 1300; + $this->assertEqualsWithDelta(2, thold_calculate_expression($outer, '', $reindexed, $time_reindexed), 1.0e-9); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', ['data_source_type_id' => self::COUNTER]); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); + CactiStubs::willReturn('rrdtool_function_fetch', [ + 'data_source_names' => ['traffic_in'], + 'values' => [['1700000000' => 2.0]], + ]); + $this->assertEqualsWithDelta(2.0, thold_calculate_expression($outer, '', $reindexed, $time_reindexed), 1.0e-9); + $this->assertSame([], CactiStubs::$log); + + CactiStubs::reset(); + CactiStubs::$configOptions['dsstats_enable'] = 'on'; + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', ['data_source_type_id' => self::COUNTER]); + CactiStubs::willReturn('db_fetch_cell_prepared', 3.5); + $this->assertEqualsWithDelta(3.5, thold_calculate_expression($outer, '', $reindexed, $time_reindexed), 1.0e-9); + $this->assertSame([], CactiStubs::callsTo('rrdtool_function_fetch')); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', ['data_source_type_id' => self::GAUGE]); + $this->assertSame('700', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + $this->assertSame([], CactiStubs::callsTo('rrdtool_function_fetch')); + + foreach ([ + [], + ['data_source_names' => ['traffic_out'], 'values' => [['1700000000' => 2.0]]], + ] as $missing_fetch) { + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', ['data_source_type_id' => self::COUNTER]); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); + CactiStubs::willReturn('rrdtool_function_fetch', $missing_fetch); + $this->assertSame('', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + } + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', []); + $this->assertSame('', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + $reindexed = []; + $this->assertSame('', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + $this->assertStringContainsString('expression source traffic_in is unavailable', CactiStubs::$log[0]); + $log_call = CactiStubs::callsTo('cacti_log')[0]; + $this->assertSame('THOLD', $log_call['params'][2]); + $this->assertSame(POLLER_VERBOSITY_MEDIUM, $log_call['params'][3]); + } + + /** + * @return void + */ + public function testMissedSampleCarriesTheValueAndTimestampTogether(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100]); + + $missed = thold_sample_persistence($thold, [], 1300); + $this->assertSame(['lasttime' => 1000, 'oldvalue' => 100], $missed); + $this->assertSame($missed, thold_sample_persistence($thold, ['traffic_in' => 'U'], 1300)); + $this->assertSame($missed, thold_sample_persistence($thold, ['traffic_in' => 'nan'], 1300)); + $this->assertSame($missed, thold_sample_persistence($thold, ['traffic_in' => ''], 1300)); + $this->assertSame( + ['lasttime' => 1600, 'oldvalue' => 700], + thold_sample_persistence($thold, ['traffic_in' => 700], 1600) + ); + $this->assertSame( + ['lasttime' => 1600, 'oldvalue' => '700'], + thold_sample_persistence($thold, ['traffic_in' => '700'], 1600) + ); + + $thold['lasttime'] = $missed['lasttime']; + $thold['oldvalue'] = $missed['oldvalue']; + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1600]; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta( + 1, + thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + } + + /** + * @return array + */ + public static function emptyMaximumProvider() { + return [ + 'integer zero' => [0, 1009000000100, 1009000000100 / 600], + 'empty string' => ['', 1009000000100, 1009000000100 / 600], + 'null maximum' => [null, 1009000000100, 1009000000100 / 600], + 'unknown maximum' => ['U', 1009000000100, 1009000000100 / 600], + 'unresolved if speed' => ['|query_ifSpeed|', 1009000000100, 1009000000100 / 600], + 'explicit maximum' => [20000000, 1015000000000, 1015000000000 / 600], + ]; + } + + /** + * @dataProvider emptyMaximumProvider + * + * @param int|string|null $maximum + * @param int $reading + * @param float $expected + * + * @return void + */ + public function testMultiIntervalDeltaScalesTheResetGuard($maximum, $reading, $expected): void { + if ($maximum === '|query_ifSpeed|') { + CactiStubs::willReturn('db_fetch_row_prepared', ['host_id' => 1, 'snmp_query_id' => 2, 'snmp_index' => 'eth0']); + CactiStubs::willReturn('db_fetch_cell_prepared', ''); + } + + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 1000000000000, 'rrd_maximum' => $maximum]); + $reindexed = [4 => ['traffic_in' => $reading]]; + $time_reindexed = [4 => 1600]; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta($expected, thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime), 1.0e-9); + } + + /** + * @return void + */ + public function testMultiIntervalWrapUsesTheWholeElapsedTime(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 4294967290, 'rrd_maximum' => 0]); + $reindexed = [4 => ['traffic_in' => 5]]; + $time_reindexed = [4 => 1600]; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta( + 11 / 600, + thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + } + + /** + * @return void + */ + public function testWrapResetGuardUsesTheEffectiveSampleInterval(): void { + CactiStubs::$configOptions['poller_interval'] = 300; + $thold = $this->threshold([ + 'lasttime' => 1700000000, + 'oldvalue' => 1000, + 'rrd_step' => 60, + 'rrd_maximum' => 0, + ]); + + $this->assertEqualsWithDelta(999 / 300, $this->currentValue($thold, 999), 1.0e-9); + } + + /** + * @return void + */ + public function testStaleCounterAndDeriveSamplesAreDiscarded(): void { + foreach ([self::COUNTER, self::DERIVE] as $type) { + $thold = $this->threshold(['data_source_type_id' => $type, 'lasttime' => 1000, 'oldvalue' => 100]); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1000 + 7 * 86400]; + $item = []; + $currenttime = 0; + + $this->assertSame('', thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); + } + } + + /** + * @return array + */ + public static function invalidRrdStepProvider() { + return [ + 'zero' => [0], + 'null' => [null], + 'non-numeric' => ['invalid'], + ]; + } + + /** + * @dataProvider invalidRrdStepProvider + * + * @param mixed $rrd_step + * + * @return void + */ + public function testInvalidRrdStepFallsBackToThePollerInterval($rrd_step): void { + CactiStubs::$configOptions['poller_interval'] = 300; + $thold = $this->threshold([ + 'data_source_type_id' => self::ABSOLUTE, + 'lasttime' => 0, + 'rrd_step' => $rrd_step, + ]); + + $this->assertEqualsWithDelta(2, $this->currentValue($thold, 600), 1.0e-9); + } + + /** + * @return void + */ + public function testEvaluationCadenceMayExceedTheRrdStep(): void { + CactiStubs::$configOptions['poller_interval'] = 300; + $thold = $this->threshold(['rrd_step' => 60]); + + $this->assertEqualsWithDelta(2, $this->currentValue($thold, 700), 1.0e-9); + } + + /** + * @return void + */ + public function testRrdHeartbeatControlsGapAcceptanceWithASafeFloor(): void { + $accepted = $this->threshold(['lasttime' => 1000, 'rrd_heartbeat' => 1800]); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1900]; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta( + 600 / 900, + thold_get_currentval($accepted, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + + $floored = $accepted; + $floored['rrd_heartbeat'] = 120; + $time_reindexed[4] = 1300; + $this->assertEqualsWithDelta( + 2, + thold_get_currentval($floored, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + + $time_reindexed[4] = 1700; + $this->assertSame('', thold_get_currentval($floored, $reindexed, $time_reindexed, $item, $currenttime)); + } + + /** + * @return void + */ + public function testNonNumericSampleTimeUsesTheRrdStep(): void { + $thold = $this->threshold(); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 'U']; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta(2, thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime), 1.0e-9); + } + + /** + * @return void + */ + public function testFirstAbsoluteSampleUsesTheValidatedPollerInterval(): void { + CactiStubs::$configOptions['poller_interval'] = 300; + $thold = $this->threshold([ + 'data_source_type_id' => self::ABSOLUTE, + 'lasttime' => 0, + 'rrd_step' => 0, + ]); + + $this->assertEqualsWithDelta(2, $this->currentValue($thold, 600), 1.0e-9); + } + + /** + * @return void + */ + public function testOutOfOrderCounterAndDeriveSamplesAreUnknown(): void { + foreach ([self::COUNTER, self::DERIVE] as $type) { + $thold = $this->threshold(['data_source_type_id' => $type, 'lasttime' => 1700000400]); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1700000300]; + $item = []; + $currenttime = 0; + + $this->assertSame('', thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); + } + + $this->assertSame( + ['lasttime' => 1700000300, 'oldvalue' => 700], + thold_sample_persistence($thold, ['traffic_in' => 700], 1700000300) + ); + $this->assertSame( + ['lasttime' => 1700000400, 'oldvalue' => 100], + thold_sample_persistence($thold, ['traffic_in' => 700], 1700000400) + ); + + CactiStubs::reset(); + $this->assertSame([ + 'sample_row' => "(9, 1, '', FROM_UNIXTIME(1700000300), '700')", + 'status_row' => null, + ], thold_polling_sample_row($thold, ['traffic_in' => 700], '', 1700000300)); + $this->assertCount(1, CactiStubs::$log); + $this->assertStringContainsString('clock moved backwards', CactiStubs::$log[0]); + + CactiStubs::reset(); + $this->assertTrue(thold_daemon_persist_sample($thold, ['traffic_in' => 700], '', 1700000300)); + $this->assertCount(1, CactiStubs::$log); + $this->assertStringContainsString('clock moved backwards', CactiStubs::$log[0]); + $call = end(CactiStubs::$calls); + $this->assertSame([1, '', 1700000300, 700, 9], $call['params']); + + $reanchored = $this->threshold(['lasttime' => 1700000300, 'oldvalue' => 700]); + $reindexed = [4 => ['traffic_in' => 1000]]; + $time_reindexed = [4 => 1700000600]; + $item = []; + $currenttime = 0; + $this->assertEqualsWithDelta( + 1, + thold_get_currentval($reanchored, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + } + + /** + * @return void + */ + public function testDeriveWithAnInvalidPriorValueIsUnknown(): void { + $thold = $this->threshold([ + 'data_source_type_id' => self::DERIVE, + 'oldvalue' => 'U', + ]); + + $this->assertSame('', $this->currentValue($thold, 700)); + } + + /** + * @return void + */ + public function testDaemonPersistsThePairWithBoundParameters(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100]); + + $this->assertTrue(thold_daemon_persist_sample($thold, [], '', 1300)); + $call = end(CactiStubs::$calls); + $this->assertStringContainsString('lasttime = FROM_UNIXTIME(?)', $call['sql']); + $this->assertSame([1, '', 1000, 100, 9], $call['params']); + + CactiStubs::reset(); + $this->assertTrue(thold_daemon_persist_sample($thold, ['traffic_in' => 700], 2, 1600)); + $call = end(CactiStubs::$calls); + $this->assertSame([1, 2, 1600, 700, 9], $call['params']); + } + + /** + * @return void + */ + public function testDaemonPropagatesPersistenceFailure(): void { + CactiStubs::willReturn('db_execute_prepared', false); + $this->assertFalse(thold_daemon_persist_sample( + $this->threshold(['lasttime' => 1000]), + [], + '', + 1300 + )); + + CactiStubs::reset(); + CactiStubs::willReturn('db_execute_prepared', false); + $this->assertFalse(thold_daemon_persist_sample( + $this->threshold(['lasttime' => 0]), + [], + '', + 1300 + )); + } + + /** + * @return void + */ + public function testNeverSampledThresholdLeavesTheTimestampPairUntouched(): void { + $thold = $this->threshold(['lasttime' => 0, 'oldvalue' => null]); + + $this->assertSame( + ['lasttime' => 0, 'oldvalue' => null], + thold_sample_persistence($thold, ['traffic_in' => 'U'], 1300) + ); + $this->assertTrue(thold_daemon_persist_sample($thold, ['traffic_in' => 'U'], '', 1300)); + $call = end(CactiStubs::$calls); + $this->assertStringNotContainsString('FROM_UNIXTIME', $call['sql']); + $this->assertStringNotContainsString('oldvalue', $call['sql']); + $this->assertSame([1, '', 9], $call['params']); + + CactiStubs::reset(); + $this->assertSame( + ['sample_row' => null, 'status_row' => ['id' => 9, 'tcheck' => 1, 'lastread' => '']], + thold_polling_sample_row($thold, ['traffic_in' => 'U'], '', 1300) + ); + $this->assertSame([], CactiStubs::$calls); + } + + /** + * @return void + */ + public function testPollerBuildsTheSamePersistedPair(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100]); + + $this->assertSame([ + 'sample_row' => "(9, 1, '2', FROM_UNIXTIME(1000), '100')", + 'status_row' => null, + ], thold_polling_sample_row($thold, [], 2, 1300)); + $this->assertSame([ + 'sample_row' => "(9, 1, '2', FROM_UNIXTIME(1600), '700')", + 'status_row' => null, + ], thold_polling_sample_row($thold, ['traffic_in' => 700], 2, 1600)); + $this->assertSame([ + 'sample_row' => "(9, 1, '2', FROM_UNIXTIME(1000), '')", + 'status_row' => null, + ], thold_polling_sample_row($this->threshold(['lasttime' => 1000, 'oldvalue' => null]), [], 2, 1300)); + } + + /** + * @return void + */ + public function testMissingPersistenceKeysFailClosed(): void { + $this->assertSame( + ['lasttime' => 0, 'oldvalue' => null], + thold_sample_persistence([], ['traffic_in' => 700], 1600) + ); + $this->assertFalse(thold_daemon_persist_sample([], ['traffic_in' => 700], 2, 1600)); + $this->assertSame( + ['sample_row' => null, 'status_row' => null], + thold_polling_sample_row([], ['traffic_in' => 700], 2, 1600) + ); + } + + /** + * @return void + */ + public function testUnavailableSampleLogsOnlyOnTheStateTransition(): void { + $thold = $this->threshold([ + 'lastread' => 12, + 'name_cache' => 'Traffic in', + ]); + + thold_polling_sample_row($thold, [], '', 1700000300); + $this->assertCount(1, CactiStubs::$log); + $log_call = CactiStubs::callsTo('cacti_log')[0]; + $this->assertSame('THOLD', $log_call['params'][2]); + $this->assertSame(POLLER_VERBOSITY_MEDIUM, $log_call['params'][3]); + + $thold['lastread'] = ''; + thold_polling_sample_row($thold, [], '', 1700000600); + $this->assertCount(1, CactiStubs::$log); + } + + /** + * @return void + */ + public function testPollerCleanupRunsForEitherBatchType(): void { + thold_polling_cleanup(false); + $this->assertSame([], CactiStubs::$calls); + + CactiStubs::willReturn('db_affected_rows', 1); + thold_polling_cleanup(true); + $this->assertSame('db_execute_prepared', CactiStubs::$calls[0]['fn']); + $this->assertStringContainsString('local_data_id = 0', CactiStubs::$calls[0]['sql']); + $this->assertArrayHasKey('time_last_change_thold', CactiStubs::$configOptions); + } } diff --git a/tests/Unit/ThresholdHiLowCharacterizationTest.php b/tests/Unit/ThresholdHiLowCharacterizationTest.php index 85eb1004..398230ae 100644 --- a/tests/Unit/ThresholdHiLowCharacterizationTest.php +++ b/tests/Unit/ThresholdHiLowCharacterizationTest.php @@ -237,9 +237,14 @@ public function testMaintenanceWindowSuppressesNotification(): void { * @return void */ public function testUnknownReadingEmitsNoAlert(): void { - $outcome = $this->bounded(['lastread' => 'U'])->poll(); + $outcome = $this->bounded([ + 'lastread' => 'U', + 'thold_alert' => 2, + 'thold_fail_count' => 3, + ])->poll(); $this->assertSame(0, $outcome->mailCount()); + $this->assertNull($outcome->persistedAlertState()); } /** diff --git a/tests/Unit/ThresholdTimeBasedCharacterizationTest.php b/tests/Unit/ThresholdTimeBasedCharacterizationTest.php index e5a87fd5..b76e8507 100644 --- a/tests/Unit/ThresholdTimeBasedCharacterizationTest.php +++ b/tests/Unit/ThresholdTimeBasedCharacterizationTest.php @@ -134,9 +134,14 @@ public function testAcknowledgedThresholdDoesNotMailOnBreach(): void { * @return void */ public function testUnknownReadingEmitsNoAlert(): void { - $outcome = $this->bounded(['lastread' => 'U'])->poll(); + $outcome = $this->bounded([ + 'lastread' => 'U', + 'thold_alert' => 2, + 'thold_fail_count' => 3, + ])->poll(); $this->assertSame(0, $outcome->mailCount()); + $this->assertNull($outcome->persistedAlertState()); } /** diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 6c4a8e4f..e0b2991f 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -65,6 +65,10 @@ require_once __DIR__ . '/Helpers/ThresholdOutcome.php'; require_once __DIR__ . '/Helpers/ThresholdScenario.php'; +if (!defined('POLLER_VERBOSITY_MEDIUM')) { + define('POLLER_VERBOSITY_MEDIUM', 3); +} + /* * base_path has to point at the Cacti root two levels above this plugin: * thold_functions.php builds include paths from it at runtime. @@ -250,6 +254,7 @@ function __esc($text) { if (!function_exists('cacti_log')) { function cacti_log($message, $output = false, $environ = 'CMDPHP', $level = 0) { CactiStubs::$log[] = $message; + CactiStubs::record('cacti_log', '', [$message, $output, $environ, $level]); } } @@ -494,6 +499,14 @@ function rrdtool_function_interface_speed($data_local) { } } +if (!function_exists('substitute_snmp_query_data')) { + function substitute_snmp_query_data($value, $host_id, $snmp_query_id, $snmp_index) { + CactiStubs::record('substitute_snmp_query_data', (string) $value, [$host_id, $snmp_query_id, $snmp_index]); + + return CactiStubs::nextReturn('substitute_snmp_query_data', $value); + } +} + if (!function_exists('get_timeinstate')) { function get_timeinstate($host) { return CactiStubs::nextReturn('get_timeinstate', '1 day'); diff --git a/thold_functions.php b/thold_functions.php index 47526a76..2bd53824 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -837,9 +837,186 @@ function thold_counter_wrap_delta($oldvalue, $newvalue) { return (4294967296 - $oldvalue) + $newvalue; } +/** + * Whether a valid current sample predates the stored sample clock. + * + * @param array $thold_data + * @param array $item + * @param int $currenttime + * + * @return bool + */ +function thold_sample_clock_moved_backward(array $thold_data, array $item, $currenttime) { + $name = (string) ($thold_data['name'] ?? ''); + $currenttime = (int) $currenttime; + $lasttime = (int) ($thold_data['lasttime'] ?? 0); + + return $name !== '' + && $currenttime > 0 + && $lasttime > 0 + && $currenttime < $lasttime + && isset($item[$name]) + && is_numeric($item[$name]); +} + +/** + * Persist a raw sample and its timestamp as one causal pair. + * + * `$thold_data` must provide `name`, `lasttime`, and `oldvalue`; absent values + * fail closed to an unavailable prior sample. + * + * @param array $thold_data + * @param array $item + * @param int $currenttime + * + * @return array{lasttime:mixed,oldvalue:mixed} + */ +function thold_sample_persistence(array $thold_data, array $item, $currenttime) { + $name = (string) ($thold_data['name'] ?? ''); + $currenttime = (int) $currenttime; + + $lasttime = (int) ($thold_data['lasttime'] ?? 0); + + if ($name !== '' && $currenttime > 0 && $currenttime !== $lasttime && isset($item[$name]) && is_numeric($item[$name])) { + if (thold_sample_clock_moved_backward($thold_data, $item, $currenttime)) { + cacti_log(sprintf( + 'WARNING: Threshold %s sample clock moved backwards; re-anchoring its value and timestamp.', + $thold_data['id'] ?? ($thold_data['thold_id'] ?? 'unknown') + ), false, 'THOLD', POLLER_VERBOSITY_MEDIUM); + } + + return ['lasttime' => $currenttime, 'oldvalue' => $item[$name]]; + } + + return [ + 'lasttime' => $lasttime, + 'oldvalue' => $thold_data['oldvalue'] ?? null, + ]; +} + +/** + * Log only the transition from a numeric result to an unavailable result. + * + * @param array $thold_data + * @param mixed $currentval + * + * @return void + */ +function thold_log_unavailable_transition(array $thold_data, $currentval) { + if (is_numeric($currentval) || !is_numeric($thold_data['lastread'] ?? null)) { + return; + } + + cacti_log(sprintf( + 'WARNING: Threshold %s (%s) current sample is unavailable; preserving its alert state.', + $thold_data['id'] ?? ($thold_data['thold_id'] ?? 'unknown'), + $thold_data['name_cache'] ?? ($thold_data['thold_name'] ?? ($thold_data['name'] ?? 'unknown')) + ), false, 'THOLD', POLLER_VERBOSITY_MEDIUM); +} + +/** + * Persist one daemon sample without manufacturing a zero SQL timestamp. + * + * @param array $thold_data + * @param array $item + * @param mixed $currentval + * @param int $currenttime + * + * @return bool + */ +function thold_daemon_persist_sample(array $thold_data, array $item, $currentval, $currenttime) { + $id = (int) ($thold_data['thold_id'] ?? 0); + $tcheck = 1; + + if ($id <= 0) { + return false; + } + + $sample = thold_sample_persistence($thold_data, $item, $currenttime); + + if (!thold_sample_clock_moved_backward($thold_data, $item, $currenttime)) { + thold_log_unavailable_transition($thold_data, $currentval); + } + + if ($sample['lasttime'] <= 0) { + return db_execute_prepared('UPDATE thold_data + SET tcheck = ?, lastread = ? + WHERE id = ?', + [$tcheck, $currentval, $id]); + } + + return db_execute_prepared('UPDATE thold_data + SET tcheck = ?, lastread = ?, + lasttime = FROM_UNIXTIME(?), oldvalue = ? + WHERE id = ?', + [$tcheck, $currentval, $sample['lasttime'], $sample['oldvalue'], $id]); +} + +/** + * Build one pure poller batching result for sample or status-only updates. + * + * @param array $thold_data + * @param array $item + * @param mixed $currentval + * @param int $currenttime + * + * @return array{sample_row:string|null,status_row:array{id:int,tcheck:int,lastread:mixed}|null} + */ +function thold_polling_sample_row(array $thold_data, array $item, $currentval, $currenttime) { + $id = (int) ($thold_data['id'] ?? 0); + $tcheck = 1; + + if ($id <= 0) { + return ['sample_row' => null, 'status_row' => null]; + } + + $sample = thold_sample_persistence($thold_data, $item, $currenttime); + + if (!thold_sample_clock_moved_backward($thold_data, $item, $currenttime)) { + thold_log_unavailable_transition($thold_data, $currentval); + } + + if ($sample['lasttime'] <= 0) { + return [ + 'sample_row' => null, + 'status_row' => ['id' => $id, 'tcheck' => $tcheck, 'lastread' => $currentval], + ]; + } + + return [ + 'sample_row' => '(' . $id . ', ' . $tcheck . ', ' . db_qstr($currentval) + . ', FROM_UNIXTIME(' . $sample['lasttime'] . '), ' . db_qstr($sample['oldvalue']) . ')', + 'status_row' => null, + ]; +} + +/** + * Remove rows deleted during polling after either update batch ran. + * + * @param bool $has_updates + * + * @return void + */ +function thold_polling_cleanup($has_updates) { + if (!$has_updates) { + return; + } + + db_execute_prepared('DELETE FROM thold_data WHERE local_data_id = 0'); + + if (db_affected_rows() > 0) { + set_config_option('time_last_change_thold', time()); + } +} + function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexed, &$item, &$currenttime) { // adjust the polling interval by the last read, if applicable $currenttime = $rrd_time_reindexed[$thold_data['local_data_id']]; + $poller_interval = read_config_option('poller_interval'); + + if (!is_numeric($poller_interval) || $poller_interval <= 0) { + $poller_interval = 300; + } if ($thold_data['lasttime'] > 0) { if (is_numeric($currenttime)) { @@ -851,8 +1028,32 @@ function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexe $step = $thold_data['rrd_step']; } - if (empty($step)) { - $step = read_config_option('poller_interval'); + $elapsed_step = $step; + + if (!is_numeric($step) || $step <= 0) { + $step = $poller_interval; + } + + $rrd_step = is_numeric($thold_data['rrd_step']) && $thold_data['rrd_step'] > 0 + ? (float) $thold_data['rrd_step'] + : 0.0; + $sample_interval = max($rrd_step, (float) $poller_interval); + // Use a two-cycle floor so normal scheduler jitter does not discard the + // only usable pair even when an RRD heartbeat is tighter than poll cadence. + $rrd_heartbeat = is_numeric($thold_data['rrd_heartbeat'] ?? null) && $thold_data['rrd_heartbeat'] > 0 + ? max((float) $thold_data['rrd_heartbeat'], 2 * $sample_interval) + : 2 * $sample_interval; + $previous_sample_usable = $thold_data['lasttime'] > 0 + && is_numeric($elapsed_step) + && $elapsed_step > 0 + && $elapsed_step <= $rrd_heartbeat; + + if ($thold_data['lasttime'] > 0 && is_numeric($elapsed_step) && $elapsed_step > $rrd_heartbeat && function_exists('thold_debug')) { + thold_debug(sprintf( + 'Threshold sample gap of %s seconds exceeds the effective heartbeat of %s seconds.', + $elapsed_step, + $rrd_heartbeat + ), 'thold'); } $currentval = ''; @@ -864,7 +1065,7 @@ function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexe switch ($thold_data['data_source_type_id']) { case 2: // COUNTER // A previous reading of zero is a real reading, not a missing one. - if (is_numeric($thold_data['oldvalue']) && $thold_data['oldvalue'] !== '') { + if ($previous_sample_usable && is_numeric($thold_data['oldvalue']) && $thold_data['oldvalue'] !== '') { if ($item[$thold_data['name']] >= $thold_data['oldvalue']) { // Everything is Normal $currentval = $item[$thold_data['name']] - $thold_data['oldvalue']; @@ -873,7 +1074,7 @@ function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexe $currentval = thold_counter_wrap_delta($thold_data['oldvalue'], $item[$thold_data['name']]); } - if (strpos($thold_data['rrd_maximum'], '|query_') !== false) { + if (strpos((string) $thold_data['rrd_maximum'], '|query_') !== false) { $data_local = db_fetch_row_prepared('SELECT * FROM data_local WHERE id = ?', @@ -898,25 +1099,34 @@ function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexe } } + $maximum_value = $thold_data['rrd_maximum'] ?? ''; + $rrd_maximum = is_numeric($maximum_value) ? (float) $maximum_value : 0.0; + // assume counter reset if greater than max value - if ($thold_data['rrd_maximum'] > 0 && ($currentval / $step) > $thold_data['rrd_maximum']) { + if ($rrd_maximum > 0 && ($currentval / $step) > $rrd_maximum) { $currentval = $item[$thold_data['name']] / $step; - } elseif ($thold_data['rrd_maximum'] == 0 && $currentval > 4.25E+9) { + } elseif ($rrd_maximum === 0.0 && $currentval > 4.25E+9 * max(1, $step / $sample_interval)) { $currentval = $item[$thold_data['name']] / $step; } else { $currentval = $currentval / $step; } } else { - $currentval = 0; + $currentval = ''; } break; case 3: // DERIVE - $currentval = ($item[$thold_data['name']] - $thold_data['oldvalue']) / $step; + if ($previous_sample_usable && is_numeric($thold_data['oldvalue'])) { + $currentval = ($item[$thold_data['name']] - $thold_data['oldvalue']) / $step; + } else { + $currentval = ''; + } break; case 4: // ABSOLUTE - $currentval = $item[$thold_data['name']] / $step; + $currentval = ($thold_data['lasttime'] <= 0 || $previous_sample_usable) + ? $item[$thold_data['name']] / $step + : ''; break; case 1: // GAUGE @@ -972,7 +1182,7 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_ td.host_id, td.cdef, td.local_data_id, td.data_template_rrd_id, td.lastread, UNIX_TIMESTAMP(td.lasttime) AS lasttime, td.oldvalue, dtr.data_source_name as name, - dtr.data_source_type_id, dtd.rrd_step, dtr.rrd_maximum + dtr.data_source_type_id, dtd.rrd_step, dtr.rrd_maximum, dtr.rrd_heartbeat FROM thold_data AS td LEFT JOIN data_template_rrd AS dtr ON dtr.id = td.data_template_rrd_id @@ -982,34 +1192,64 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_ AND td.local_data_id = ?', [$dsname, $thold['local_data_id']]); - $value = ''; - - if (cacti_sizeof($thold_item)) { - $item = []; - $currenttime = 0; - $value = thold_get_currentval($thold_item, $rrd_reindexed, $rrd_time_reindexed, $item, $currenttime); - } + if (!cacti_sizeof($thold_item)) { + $current_sample = $rrd_reindexed[$thold['local_data_id']][$dsname] ?? ''; - // Previous returns 'U' after device recovers. Try alternate - if (empty($value) || $value == 'U') { - if (read_config_option('dsstats_enable') == 'on') { - $value = db_fetch_cell_prepared('SELECT calculated - FROM data_source_stats_hourly_last + if (is_numeric($current_sample)) { + $source = db_fetch_row_prepared('SELECT data_source_type_id + FROM data_template_rrd WHERE local_data_id = ? - AND rrd_name = ?', + AND data_source_name = ?', [$thold['local_data_id'], $dsname]); + + if (cacti_sizeof($source) && $source['data_source_type_id'] == 1) { + $value = $current_sample; + } elseif (cacti_sizeof($source)) { + $value = ''; + + if (read_config_option('dsstats_enable') == 'on') { + $value = db_fetch_cell_prepared('SELECT calculated + FROM data_source_stats_hourly_last + WHERE local_data_id = ? + AND rrd_name = ?', + [$thold['local_data_id'], $dsname]); + } + + if (!is_numeric($value) || $value == -90909090909) { + $value = get_current_value($thold['local_data_id'], $dsname, 0, ''); + } + } else { + $value = ''; + } + + if (is_numeric($value)) { + $expression[$key] = $value; + + continue; + } } - if (empty($value) || $value == 'U' || $value == '-90909090909') { - $value = get_current_value($thold['local_data_id'], $dsname); + if (is_numeric($thold['lastread'] ?? null)) { + cacti_log(sprintf( + 'WARNING: Threshold %s expression source %s is unavailable for local data ID %s.', + $thold['id'] ?? 'unknown', + $dsname, + $thold['local_data_id'] ?? 'unknown' + ), false, 'THOLD', POLLER_VERBOSITY_MEDIUM); } + + return ''; } - $expression[$key] = $value; + $item = []; + $currenttime = 0; + $value = thold_get_currentval($thold_item, $rrd_reindexed, $rrd_time_reindexed, $item, $currenttime); - if ($expression[$key] == '') { - $expression[$key] = '0'; + if (!is_numeric($value)) { + return ''; } + + $expression[$key] = $value; } elseif (strpos($item, '|') !== false) { // Remove invalid characters $item = str_replace('\\', '', $item); @@ -1269,7 +1509,9 @@ function thold_calculate_percent($thold, $currentval, $rrd_reindexed) { // forced the percentage to zero and kept a low threshold alerting. $t = $rrd_reindexed[$thold['local_data_id']][$thold['percent_ds']]; - if (is_numeric($t) && $t != 0) { + if (!is_numeric($t)) { + $currentval = ''; + } elseif ($t != 0) { $currentval = ($currentval / $t) * 100; } else { $currentval = 0; @@ -1284,12 +1526,21 @@ function thold_calculate_percent($thold, $currentval, $rrd_reindexed) { function thold_calculate_lower_upper($thold, $currentval, $rrd_reindexed) { $ds = $thold['upper_ds']; - if (isset($rrd_reindexed[$thold['local_data_id']][$ds])) { - $t = $rrd_reindexed[$thold['local_data_id']][$thold['upper_ds']]; - $currentval = ($t << 32) + $currentval; + if (!is_numeric($currentval)) { + return ''; } - return $currentval; + if (!isset($rrd_reindexed[$thold['local_data_id']][$ds])) { + return ''; + } + + $t = $rrd_reindexed[$thold['local_data_id']][$thold['upper_ds']]; + + if (!is_numeric($t) || $t < 0 || $t > 4294967295) { + return ''; + } + + return ((float) $t * 4294967296) + $currentval; } function get_allowed_thresholds($sql_where = '', $order_by = 'td.name', $sql_limit = '', &$total_rows = 0, $user_id = 0, $graph_id = 0) { @@ -2323,6 +2574,11 @@ function thold_check_threshold(&$thold_data) { return; } + // An unavailable sample is not evidence that an active alert recovered. + if (!is_numeric($thold_data['lastread'])) { + return; + } + $alert_exempt = read_config_option('alert_exempt'); // check for exemptions $weekday = date('l'); @@ -4676,8 +4932,8 @@ function thold_cdef_select_usable_names() { } function thold_build_cdef($cdef, $value, $local_data_id, $data_template_rrd_id) { - if ($value == '') { - $value = 0; + if (!is_numeric($value)) { + return ''; } $oldvalue = $value; @@ -4939,7 +5195,7 @@ function thold_rrd_last($local_data_id) { return trim($last_time_entry); } -function get_current_value($local_data_id, $data_template_rrd_id, $cdef = 0) { +function get_current_value($local_data_id, $data_template_rrd_id, $cdef = 0, $missing_value = 0) { // get the information to populate into the rrd files if (function_exists('boost_check_correct_enabled') && boost_check_correct_enabled()) { boost_process_poller_output($local_data_id); @@ -4968,7 +5224,7 @@ function get_current_value($local_data_id, $data_template_rrd_id, $cdef = 0) { // Return Blank if the data source is not found (Newly created?) if (!isset($result['data_source_names'])) { - return 0; + return $missing_value; } // array_search() reports a miss as false. Testing for null let the miss @@ -4978,7 +5234,7 @@ function get_current_value($local_data_id, $data_template_rrd_id, $cdef = 0) { // Return Blank if the value was not found (Cache Cleared?) if ($idx === false || !isset($result['values'][$idx]) || !cacti_sizeof($result['values'][$idx])) { - return 0; + return $missing_value; } $value = array_values($result['values'][$idx])[0]; diff --git a/thold_process.php b/thold_process.php index 40078c24..7edf3e7d 100644 --- a/thold_process.php +++ b/thold_process.php @@ -207,22 +207,11 @@ $currentval = ''; } - // Carry the previous value forward when this cycle has no reading; - // storing a timestamp here corrupts the next delta calculation. - if (isset($item[$thold_data['name']])) { - $rawvalue = $item[$thold_data['name']]; - } else { - $rawvalue = $thold_data['oldvalue']; - } - thold_daemon_debug(sprintf('Checked Name:%s, Graph:%s, Value:%s, Time:%s', $thold_data['thold_name'], $thold_data['local_graph_id'], $currentval, $currenttime), $thread); - db_execute_prepared('UPDATE thold_data - SET tcheck = 1, lastread = ?, - lasttime = FROM_UNIXTIME(?), oldvalue = ? - WHERE id = ?', - [$currentval, $currenttime, $rawvalue, $thold_data['thold_id']] - ); + if (!thold_daemon_persist_sample($thold_data, $item, $currentval, $currenttime)) { + thold_daemon_debug(sprintf('Failed to persist threshold sample for ID %s.', $thold_data['thold_id']), $thread); + } } $tholds = thold_get_thresholds_tholdcheck($thread, $start_time); @@ -370,7 +359,7 @@ function thold_get_thresholds_precheck($thread, $start_time) { td.data_template_rrd_id, td.lastread, UNIX_TIMESTAMP(td.lasttime) AS lasttime, td.oldvalue, dtr.data_source_name AS name, dtr.data_source_type_id, - dtd.rrd_step, dtr.rrd_maximum + dtd.rrd_step, dtr.rrd_maximum, dtr.rrd_heartbeat FROM plugin_thold_daemon_data AS tdd INNER JOIN thold_data AS td ON td.id = tdd.id @@ -391,7 +380,7 @@ function thold_get_thresholds_precheck($thread, $start_time) { td.data_template_rrd_id, td.lastread, UNIX_TIMESTAMP(td.lasttime) AS lasttime, td.oldvalue, dtr.data_source_name AS name, dtr.data_source_type_id, - dtd.rrd_step, dtr.rrd_maximum + dtd.rrd_step, dtr.rrd_maximum, dtr.rrd_heartbeat FROM plugin_thold_daemon_data AS tdd INNER JOIN thold_data AS td ON td.id = tdd.id