fix: aggregate 95th percentile uses SUM instead of MAX for SIMILAR#6843
Merged
TheWitness merged 11 commits intoCacti:developfrom Mar 24, 2026
Merged
Conversation
AGGREGATE_TOTAL_TYPE_SIMILAR was grouped with ALL in three locations, causing SIMILAR aggregate graphs to use aggregate_sum / aggregate_sum_peak (which sums inbound + outbound together) instead of aggregate_current / aggregate_current_peak (which computes percentiles per data source name). This made 95th percentile HRULEs on aggregate graphs display the sum of in+out instead of max(in, out), roughly doubling the reported value. Split each conditional into separate ALL and SIMILAR branches: - ALL: aggregate_sum / aggregate_sum_peak (sum all data sources) - SIMILAR: aggregate_current / aggregate_current_peak (per-source) Three locations in lib/api_aggregate.php: - text_format substitution in aggregate_graphs_insert_graph_items() - pparts replacement for COMMENT items in aggregate_handle_ptile_type() - pparts replacement for HRULE items in aggregate_handle_ptile_type() Fixes Cacti#6835 Ref Cacti#6470 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
- Add cacti_log debug traces for unhandled total_type values - Add 18 integration tests covering full COMMENT/HRULE pparts pipeline and text_format replacement for SIMILAR vs ALL total types Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The test helpers replicate production logic from api_aggregate.php because the production functions require DB/session state. Add a comment documenting this dependency and the lines that must stay in sync. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof
force-pushed
the
fix/aggregate-ptile-similar-6835
branch
from
e3b5a5d to
691bc05
Compare
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
TheWitness
previously approved these changes
Mar 22, 2026
Member
|
Is this ready to go? |
Contributor
Author
yes |
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
TheWitness
approved these changes
Mar 24, 2026
xmacan
approved these changes
Mar 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AGGREGATE_TOTAL_TYPE_SIMILARfromALLin three locations inlib/api_aggregate.phpaggregate_current/aggregate_current_peak(per-data-source percentiles) instead ofaggregate_sum/aggregate_sum_peak(sum of all sources)max(in, out)instead ofsum(in + out):max:text_format path now usesaggregate_sum_peak(was incorrectly usingaggregate_sum), aligning it with the COMMENT/HRULE pparts paths which already usedaggregate_sum_peakRoot cause
Three conditional blocks grouped SIMILAR with ALL:
text_format:current:/:max:replacement inaggregate_graphs_insert_graph_items()pparts[3]replacement for COMMENT items inaggregate_handle_ptile_type()pparts[3]replacement for HRULE items inaggregate_handle_ptile_type()All three used
aggregate_sumfor SIMILAR, which sums inbound + outbound traffic together before computing the 95th percentile. The correct behavior for SIMILAR isaggregate_current, which computes percentiles per data source name (in/out separately) and takes the max.Note: PR #6750 fixed the test stubs but the
api_aggregate.phpchange was accidentally dropped during review cleanup.Test plan
:max:percentile items render correctly after the secondary fixFixes #6835
Ref #6470