diff --git a/lib/api_aggregate.php b/lib/api_aggregate.php index fba05ff3ae..25fd11f1b0 100644 --- a/lib/api_aggregate.php +++ b/lib/api_aggregate.php @@ -409,20 +409,20 @@ function aggregate_graphs_insert_graph_items(int $_new_graph_id, int $_old_graph $prepend = false; $prepend_cnt++; } elseif (str_contains($save['text_format'], ':current:')) { - if ($_total_type == AGGREGATE_TOTAL_TYPE_ALL || $_total_type == AGGREGATE_TOTAL_TYPE_SIMILAR) { - // All so use sum functions + if ($_total_type == AGGREGATE_TOTAL_TYPE_ALL) { $save['text_format'] = str_replace(':current:', ':aggregate_sum:', $save['text_format']); + } elseif ($_total_type == AGGREGATE_TOTAL_TYPE_SIMILAR) { + $save['text_format'] = str_replace(':current:', ':aggregate_current:', $save['text_format']); } else { - // Similar to separate - $save['text_format'] = str_replace(':current:', ':current:', $save['text_format']); + cacti_log(__FUNCTION__ . ' unhandled total_type ' . $_total_type . ' for :current: in text_format', true, 'AGGREGATE', POLLER_VERBOSITY_DEBUG); } } elseif (str_contains($save['text_format'], ':max:')) { - if ($_total_type == AGGREGATE_TOTAL_TYPE_ALL || $_total_type == AGGREGATE_TOTAL_TYPE_SIMILAR) { - // All so use sum functions - $save['text_format'] = str_replace(':max:', ':aggregate_sum:', $save['text_format']); + if ($_total_type == AGGREGATE_TOTAL_TYPE_ALL) { + $save['text_format'] = str_replace(':max:', ':aggregate_sum_peak:', $save['text_format']); + } elseif ($_total_type == AGGREGATE_TOTAL_TYPE_SIMILAR) { + $save['text_format'] = str_replace(':max:', ':aggregate_current_peak:', $save['text_format']); } else { - // Similar to separate - $save['text_format'] = str_replace(':max:', ':max:', $save['text_format']); + cacti_log(__FUNCTION__ . ' unhandled total_type ' . $_total_type . ' for :max: in text_format', true, 'AGGREGATE', POLLER_VERBOSITY_DEBUG); } } } @@ -1428,14 +1428,14 @@ function aggregate_handle_ptile_type(array $member_graphs, array $skipped_items, $pparts = explode(':', $parts[1]); if (isset($pparts[3])) { - if ($_total_type == AGGREGATE_TOTAL_TYPE_ALL || $_total_type == AGGREGATE_TOTAL_TYPE_SIMILAR) { - // All so use sum functions + if ($_total_type == AGGREGATE_TOTAL_TYPE_ALL) { $pparts[3] = str_replace('current', 'aggregate_sum', $pparts[3]); $pparts[3] = str_replace('max', 'aggregate_sum_peak', $pparts[3]); - } else { - // Similar to separate + } elseif ($_total_type == AGGREGATE_TOTAL_TYPE_SIMILAR) { $pparts[3] = str_replace('current', 'aggregate_current', $pparts[3]); $pparts[3] = str_replace('max', 'aggregate_current_peak', $pparts[3]); + } else { + cacti_log(__FUNCTION__ . ' unhandled total_type ' . $_total_type . ' for pparts[3] in text_format', true, 'AGGREGATE', POLLER_VERBOSITY_DEBUG); } switch($pparts[3]) { @@ -1510,14 +1510,14 @@ function aggregate_handle_ptile_type(array $member_graphs, array $skipped_items, $pparts = explode(':', $parts[1]); if (isset($pparts[3])) { - if ($_total_type == AGGREGATE_TOTAL_TYPE_ALL || $_total_type == AGGREGATE_TOTAL_TYPE_SIMILAR) { - // All so use sum functions + if ($_total_type == AGGREGATE_TOTAL_TYPE_ALL) { $pparts[3] = str_replace('current', 'aggregate_sum', $pparts[3]); $pparts[3] = str_replace('max', 'aggregate_sum_peak', $pparts[3]); - } else { - // Similar to separate + } elseif ($_total_type == AGGREGATE_TOTAL_TYPE_SIMILAR) { $pparts[3] = str_replace('current', 'aggregate_current', $pparts[3]); $pparts[3] = str_replace('max', 'aggregate_current_peak', $pparts[3]); + } else { + cacti_log(__FUNCTION__ . ' unhandled total_type ' . $_total_type . ' for pparts[3] in value', true, 'AGGREGATE', POLLER_VERBOSITY_DEBUG); } switch($pparts[3]) { diff --git a/tests/Unit/AggregatePtileFormatIntegrationTest.php b/tests/Unit/AggregatePtileFormatIntegrationTest.php new file mode 100644 index 0000000000..e94bcee4e7 --- /dev/null +++ b/tests/Unit/AggregatePtileFormatIntegrationTest.php @@ -0,0 +1,229 @@ +toBe('95th Percentile|nth_percentile:95:traffic_in:aggregate_current'); +}); + +test('COMMENT pipeline: SIMILAR max produces aggregate_current_peak', function () { + $input = '95th Percentile|nth_percentile:95:traffic_in:max'; + $result = process_ptile_pparts($input, AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($result)->toBe('95th Percentile|nth_percentile:95:traffic_in:aggregate_current_peak'); +}); + +test('COMMENT pipeline: ALL current produces aggregate_sum', function () { + $input = '95th Percentile|nth_percentile:95:traffic_in:current'; + $result = process_ptile_pparts($input, AGGREGATE_TOTAL_TYPE_ALL); + + expect($result)->toBe('95th Percentile|nth_percentile:95:traffic_in:aggregate_sum'); +}); + +test('COMMENT pipeline: ALL max produces aggregate_sum_peak', function () { + $input = '95th Percentile|nth_percentile:95:traffic_in:max'; + $result = process_ptile_pparts($input, AGGREGATE_TOTAL_TYPE_ALL); + + expect($result)->toBe('95th Percentile|nth_percentile:95:traffic_in:aggregate_sum_peak'); +}); + +// --- HRULE pipeline: same replacement pattern --- + +test('HRULE pipeline: SIMILAR current produces aggregate_current', function () { + $input = '#FF5722|nth_percentile:95:traffic_out:current'; + $result = process_ptile_pparts($input, AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($result)->toBe('#FF5722|nth_percentile:95:traffic_out:aggregate_current'); +}); + +test('HRULE pipeline: SIMILAR max produces aggregate_current_peak', function () { + $input = '#FF5722|nth_percentile:95:traffic_out:max'; + $result = process_ptile_pparts($input, AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($result)->toBe('#FF5722|nth_percentile:95:traffic_out:aggregate_current_peak'); +}); + +test('HRULE pipeline: ALL max produces aggregate_sum_peak', function () { + $input = '#FF5722|nth_percentile:95:traffic_out:max'; + $result = process_ptile_pparts($input, AGGREGATE_TOTAL_TYPE_ALL); + + expect($result)->toBe('#FF5722|nth_percentile:95:traffic_out:aggregate_sum_peak'); +}); + +// --- SIMILAR must never produce summed values --- + +test('SIMILAR COMMENT never produces aggregate_sum for current', function () { + $input = '95th Percentile|nth_percentile:95:traffic_in:current'; + $result = process_ptile_pparts($input, AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($result)->not->toContain('aggregate_sum:') + ->and($result)->not->toContain(':aggregate_sum'); +}); + +test('SIMILAR HRULE never produces aggregate_sum_peak for max', function () { + $input = '#FF5722|nth_percentile:95:traffic_in:max'; + $result = process_ptile_pparts($input, AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($result)->not->toContain('aggregate_sum_peak'); +}); + +// --- text_format pipeline: full string with ISP billing context --- + +test('text_format: SIMILAR with :current: in bandwidth label', function () { + $result = process_text_format('95th In :current: bps', AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($result)->toBe('95th In :aggregate_current: bps'); +}); + +test('text_format: SIMILAR with :max: in bandwidth label', function () { + $result = process_text_format('95th Peak :max: bps', AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($result)->toBe('95th Peak :aggregate_current_peak: bps'); +}); + +test('text_format: ALL with :current: in bandwidth label', function () { + $result = process_text_format('95th Total :current: bps', AGGREGATE_TOTAL_TYPE_ALL); + + expect($result)->toBe('95th Total :aggregate_sum: bps'); +}); + +test('text_format: ALL with :max: produces aggregate_sum_peak not aggregate_sum', function () { + // This is the secondary fix: ALL+max was using aggregate_sum instead of + // aggregate_sum_peak, mismatching the COMMENT/HRULE paths + $result = process_text_format('95th Peak :max: bps', AGGREGATE_TOTAL_TYPE_ALL); + + expect($result)->toBe('95th Peak :aggregate_sum_peak: bps') + ->and($result)->not->toContain(':aggregate_sum:'); +}); + +// --- Edge cases --- + +test('pparts with no pipe delimiter returns unchanged', function () { + $result = process_ptile_pparts('no pipes here', AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($result)->toBe('no pipes here'); +}); + +test('pparts with fewer than 4 colon parts returns unchanged', function () { + $result = process_ptile_pparts('label|nth_percentile:95:traffic_in', AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($result)->toBe('label|nth_percentile:95:traffic_in'); +}); + +test('unknown total type leaves pparts unchanged', function () { + $input = '95th Percentile|nth_percentile:95:traffic_in:current'; + $result = process_ptile_pparts($input, 99); + + expect($result)->toBe($input); +}); + +test('unknown total type leaves text_format unchanged', function () { + $result = process_text_format('95th :current: bps', 99); + + expect($result)->toBe('95th :current: bps'); +}); + +// --- Inbound vs Outbound: verify both data sources handled independently --- + +test('SIMILAR handles traffic_in and traffic_out independently', function () { + $in_result = process_ptile_pparts('In|nth_percentile:95:traffic_in:current', AGGREGATE_TOTAL_TYPE_SIMILAR); + $out_result = process_ptile_pparts('Out|nth_percentile:95:traffic_out:current', AGGREGATE_TOTAL_TYPE_SIMILAR); + + expect($in_result)->toBe('In|nth_percentile:95:traffic_in:aggregate_current') + ->and($out_result)->toBe('Out|nth_percentile:95:traffic_out:aggregate_current') + ->and($in_result)->not->toBe($out_result); +});