-
Notifications
You must be signed in to change notification settings - Fork 16
Alarm grouping #241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Alarm grouping #241
Conversation
This feature allows users to group messeges based on similar text and host This is useful for when a host has been sending repeted alarms of the same type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request introduces a significant new feature to the syslog plugin enabling message grouping in the UI, along with new GitHub agent templates for development assistance and minor documentation improvements.
Key Changes:
- Adds message grouping functionality to aggregate duplicate syslog entries with occurrence counts, timestamps, and expandable detail views
- Introduces three new GitHub agent templates for code quality, PHP development, and MySQL/MariaDB administration
- Updates documentation with links to main Cacti repository and resources
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 27 comments.
Show a summary per file
| File | Description |
|---|---|
| syslog.php | Implements core grouping feature with SQL queries, UI controls, row counting logic, and JavaScript expand/collapse functionality |
| syslog.css | Adds styling for group toggle icons and detail row hover effects |
| functions.php | Modifies syslog_row_color to return severity class and add syslog-detail-row class |
| .gitignore | Removes .git* pattern exclusion |
| .github/ISSUE_TEMPLATE/copilot-instructions.md | Adds documentation links and clarifies syslog reception workflow |
| .github/ISSUE_TEMPLATE/agents/php-devloper.agent.md | New agent template for PHP development assistance |
| .github/ISSUE_TEMPLATE/agents/mysql-mariadb.agent.md | New agent template for database administration |
| .github/ISSUE_TEMPLATE/agents/code-quality.agent.md | New agent template for code quality assurance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - `db_last_insert_id()`: Retrieves the ID of the last inserted record. | ||
|
|
||
|
|
||
| ##web documentation |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after '##' in the heading. Should be '## Web documentation'.
| ##web documentation | |
| ## Web documentation |
| @@ -0,0 +1,39 @@ | |||
| --- | |||
| description: "This Custom agent acts as a quality assurance specialist, focusing on code quality, best practices, and maintainability." | |||
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in 'Custom' - should be lowercase 'custom' to maintain consistency with the description format in other agent files.
| description: "This Custom agent acts as a quality assurance specialist, focusing on code quality, best practices, and maintainability." | |
| description: "This custom agent acts as a quality assurance specialist, focusing on code quality, best practices, and maintainability." |
| --- | ||
|
|
||
| # Code Quality Specialist | ||
| You are a Code Quality Specialist agent. Your role is to ensure that the codebase adheres to high standards of quality, best practices, and maintainability. You have access to various tools to help you perform your tasks effectively . |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra space before the period in 'effectively .' - should be 'effectively.'
| You are a Code Quality Specialist agent. Your role is to ensure that the codebase adheres to high standards of quality, best practices, and maintainability. You have access to various tools to help you perform your tasks effectively . | |
| You are a Code Quality Specialist agent. Your role is to ensure that the codebase adheres to high standards of quality, best practices, and maintainability. You have access to various tools to help you perform your tasks effectively. |
|
|
||
| // Get individual messages for this group | ||
| $detail_messages = syslog_db_fetch_assoc("SELECT syslog.*, syslog_programs.program | ||
| FROM `" . $syslogdb_default . "`.`" . (($sm['mtype'] == 'main') ? 'syslog' : 'syslog_removed') . "` AS syslog | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` | ||
| ON syslog.program_id=syslog_programs.program_id | ||
| WHERE syslog.seq IN (" . implode(',', array_map('intval', $seq_array)) . ") | ||
| ORDER BY syslog.logtime DESC"); | ||
|
|
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The WHERE clause in this query uses raw seq values from the seq_list without prepared statement parameters. While array_map('intval', ...) provides type casting protection, this pattern should ideally use prepared statements for consistency with database best practices. Consider using syslog_db_fetch_assoc_prepared instead.
| // Get individual messages for this group | |
| $detail_messages = syslog_db_fetch_assoc("SELECT syslog.*, syslog_programs.program | |
| FROM `" . $syslogdb_default . "`.`" . (($sm['mtype'] == 'main') ? 'syslog' : 'syslog_removed') . "` AS syslog | |
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` | |
| ON syslog.program_id=syslog_programs.program_id | |
| WHERE syslog.seq IN (" . implode(',', array_map('intval', $seq_array)) . ") | |
| ORDER BY syslog.logtime DESC"); | |
| $seq_array = array_map('intval', $seq_array); | |
| // Get individual messages for this group using a prepared statement | |
| if (!empty($seq_array)) { | |
| $placeholders = implode(',', array_fill(0, count($seq_array), '?')); | |
| $sql = "SELECT syslog.*, syslog_programs.program | |
| FROM `" . $syslogdb_default . "`.`" . (($sm['mtype'] == 'main') ? 'syslog' : 'syslog_removed') . "` AS syslog | |
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` | |
| ON syslog.program_id=syslog_programs.program_id | |
| WHERE syslog.seq IN (" . $placeholders . ") | |
| ORDER BY syslog.logtime DESC"; | |
| $detail_messages = syslog_db_fetch_assoc_prepared($sql, $seq_array); | |
| } else { | |
| $detail_messages = array(); | |
| } | |
| $sql_where | ||
| ) AS rowcount"); | ||
| } elseif (get_request_var('removal') == -1) { | ||
| $total_rows = syslog_db_fetch_cell("SELECT COUNT(DISTINCT CONCAT(host_id, '|', message, '|', program_id, '|', facility_id, '|', priority_id)) |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CONCAT function with pipe delimiters for counting distinct groups could produce incorrect counts if the message text contains the pipe character '|'. This would cause messages to be grouped incorrectly. Consider using CONCAT_WS with a null byte separator or a multi-column COUNT DISTINCT approach if the database supports it.
| // Check if grouping is enabled | ||
| $grouping_enabled = isset_request_var('grouping') && get_request_var('grouping') == '1'; | ||
|
|
||
| if ($grouping_enabled) { | ||
| // When grouping, we need to get grouped results with counts | ||
| if (get_request_var('removal') == '-1') { | ||
| $query_sql = "SELECT | ||
| syslog.host_id, | ||
| syslog.message, | ||
| syslog.program_id, | ||
| syslog.facility_id, | ||
| syslog.priority_id, | ||
| syslog_programs.program, | ||
| 'main' AS mtype, | ||
| COUNT(*) AS occurrence_count, | ||
| MIN(syslog.logtime) AS first_logtime, | ||
| MAX(syslog.logtime) AS logtime, | ||
| MIN(syslog.seq) AS seq, | ||
| GROUP_CONCAT(syslog.seq ORDER BY syslog.logtime DESC SEPARATOR ',') AS seq_list | ||
| FROM `" . $syslogdb_default . "`.`syslog` | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` | ||
| ON syslog.program_id=syslog_programs.program_id " . | ||
| $sql_where . " | ||
| GROUP BY syslog.host_id, syslog.message, syslog.program_id, syslog.facility_id, syslog.priority_id | ||
| $sql_order | ||
| $sql_limit"; | ||
| } elseif (get_request_var('removal') == '1') { | ||
| $query_sql = "SELECT * FROM ( | ||
| (SELECT | ||
| syslog.host_id, | ||
| syslog.message, | ||
| syslog.program_id, | ||
| syslog.facility_id, | ||
| syslog.priority_id, | ||
| syslog_programs.program, | ||
| 'main' AS mtype, | ||
| COUNT(*) AS occurrence_count, | ||
| MIN(syslog.logtime) AS first_logtime, | ||
| MAX(syslog.logtime) AS logtime, | ||
| MIN(syslog.seq) AS seq, | ||
| GROUP_CONCAT(syslog.seq ORDER BY syslog.logtime DESC SEPARATOR ',') AS seq_list | ||
| FROM `" . $syslogdb_default . "`.`syslog` AS syslog | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` | ||
| ON syslog.program_id=syslog_programs.program_id " . | ||
| $sql_where . " | ||
| GROUP BY syslog.host_id, syslog.message, syslog.program_id, syslog.facility_id, syslog.priority_id | ||
| ) UNION (SELECT | ||
| syslog.host_id, | ||
| syslog.message, | ||
| syslog.program_id, | ||
| syslog.facility_id, | ||
| syslog.priority_id, | ||
| syslog_programs.program, | ||
| 'remove' AS mtype, | ||
| COUNT(*) AS occurrence_count, | ||
| MIN(syslog.logtime) AS first_logtime, | ||
| MAX(syslog.logtime) AS logtime, | ||
| MIN(syslog.seq) AS seq, | ||
| GROUP_CONCAT(syslog.seq ORDER BY syslog.logtime DESC SEPARATOR ',') AS seq_list | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` | ||
| ON syslog.program_id=syslog_programs.program_id " . | ||
| $sql_where . " | ||
| GROUP BY syslog.host_id, syslog.message, syslog.program_id, syslog.facility_id, syslog.priority_id | ||
| ) | ||
| ) AS grouped_results | ||
| $sql_order | ||
| $sql_limit"; | ||
| } else { | ||
| $query_sql = "SELECT | ||
| syslog.host_id, | ||
| syslog.message, | ||
| syslog.program_id, | ||
| syslog.facility_id, | ||
| syslog.priority_id, | ||
| syslog_programs.program, | ||
| 'remove' AS mtype, | ||
| COUNT(*) AS occurrence_count, | ||
| MIN(syslog.logtime) AS first_logtime, | ||
| MAX(syslog.logtime) AS logtime, | ||
| MIN(syslog.seq) AS seq, | ||
| GROUP_CONCAT(syslog.seq ORDER BY syslog.logtime DESC SEPARATOR ',') AS seq_list | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` AS syslog_programs | ||
| ON syslog.program_id=syslog_programs.program_id " . | ||
| $sql_where . " | ||
| GROUP BY syslog.host_id, syslog.message, syslog.program_id, syslog.facility_id, syslog.priority_id | ||
| $sql_order | ||
| $sql_limit"; | ||
| } | ||
| } else { | ||
| $query_sql = "SELECT syslog.*, syslog_programs.program, 'remove' AS mtype | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` AS syslog_programs | ||
| ON syslog.program_id=syslog_programs.program_id " . | ||
| $sql_where . " | ||
| $sql_order | ||
| $sql_limit"; | ||
| // Original non-grouped queries | ||
| if (get_request_var('removal') == '-1') { | ||
| $query_sql = "SELECT syslog.*, syslog_programs.program, 'main' AS mtype | ||
| FROM `" . $syslogdb_default . "`.`syslog` | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` | ||
| ON syslog.program_id=syslog_programs.program_id " . | ||
| $sql_where . " | ||
| $sql_order | ||
| $sql_limit"; | ||
| } elseif (get_request_var('removal') == '1') { | ||
| $query_sql = "(SELECT syslog.*, syslog_programs.program, 'main' AS mtype | ||
| FROM `" . $syslogdb_default . "`.`syslog` AS syslog | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` | ||
| ON syslog.program_id=syslog_programs.program_id " . | ||
| $sql_where . " | ||
| ) UNION (SELECT syslog.*, syslog_programs.program, 'remove' AS mtype | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` | ||
| ON syslog.program_id=syslog_programs.program_id " . | ||
| $sql_where . ") | ||
| $sql_order | ||
| $sql_limit"; | ||
| } else { | ||
| $query_sql = "SELECT syslog.*, syslog_programs.program, 'remove' AS mtype | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| LEFT JOIN `" . $syslogdb_default . "`.`syslog_programs` AS syslog_programs | ||
| ON syslog.program_id=syslog_programs.program_id " . | ||
| $sql_where . " | ||
| $sql_order | ||
| $sql_limit"; | ||
| } | ||
| } |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This complex nested query structure with multiple branching paths (grouping on/off, removal values -1/0/1) creates significant code duplication and maintainability challenges. The same SQL construction logic is repeated multiple times with minor variations. Consider extracting common query building logic into helper functions or using a query builder pattern to reduce duplication and make the code more maintainable.
| // Check if grouping is enabled for row count | ||
| $grouping_enabled = isset_request_var('grouping') && get_request_var('grouping') == '1'; | ||
|
|
||
| if ($grouping_enabled) { | ||
| // When grouping, count distinct groups instead of individual rows | ||
| if (get_request_var('removal') == 1) { | ||
| $total_rows = syslog_db_fetch_cell("SELECT SUM(totals) | ||
| FROM ( | ||
| SELECT COUNT(DISTINCT CONCAT(host_id, '|', message, '|', program_id, '|', facility_id, '|', priority_id)) AS totals | ||
| FROM `" . $syslogdb_default . "`.`syslog` AS syslog | ||
| $sql_where | ||
| UNION | ||
| SELECT COUNT(DISTINCT CONCAT(host_id, '|', message, '|', program_id, '|', facility_id, '|', priority_id)) AS totals | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| $sql_where | ||
| ) AS rowcount"); | ||
| } elseif (get_request_var('removal') == -1) { | ||
| $total_rows = syslog_db_fetch_cell("SELECT COUNT(DISTINCT CONCAT(host_id, '|', message, '|', program_id, '|', facility_id, '|', priority_id)) | ||
| FROM `" . $syslogdb_default . "`.`syslog` AS syslog | ||
| $sql_where | ||
| UNION | ||
| SELECT count(*) AS totals | ||
| $sql_where"); | ||
| } else { | ||
| $total_rows = syslog_db_fetch_cell("SELECT COUNT(DISTINCT CONCAT(host_id, '|', message, '|', program_id, '|', facility_id, '|', priority_id)) | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| $sql_where | ||
| ) AS rowcount"); | ||
| } elseif (get_request_var('removal') == -1) { | ||
| $total_rows = syslog_db_fetch_cell("SELECT count(*) | ||
| FROM `" . $syslogdb_default . "`.`syslog` AS syslog | ||
| $sql_where"); | ||
| $sql_where"); | ||
| } | ||
| } else { | ||
| $total_rows = syslog_db_fetch_cell("SELECT count(*) | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| $sql_where"); | ||
| // Original non-grouped row counting | ||
| if (get_request_var('removal') == 1) { | ||
| $total_rows = syslog_db_fetch_cell("SELECT SUM(totals) | ||
| FROM ( | ||
| SELECT count(*) AS totals | ||
| FROM `" . $syslogdb_default . "`.`syslog` AS syslog | ||
| $sql_where | ||
| UNION | ||
| SELECT count(*) AS totals | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| $sql_where | ||
| ) AS rowcount"); | ||
| } elseif (get_request_var('removal') == -1) { | ||
| $total_rows = syslog_db_fetch_cell("SELECT count(*) | ||
| FROM `" . $syslogdb_default . "`.`syslog` AS syslog | ||
| $sql_where"); | ||
| } else { | ||
| $total_rows = syslog_db_fetch_cell("SELECT count(*) | ||
| FROM `" . $syslogdb_default . "`.`syslog_removed` AS syslog | ||
| $sql_where"); | ||
| } | ||
| } |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the query construction, the row counting logic duplicates significant SQL construction for grouping/non-grouping cases and removal filter values. This duplication makes it harder to maintain consistency between the query and count logic. Consider consolidating this with the query building logic to ensure they stay in sync.
|
|
||
| ## The project in this repo calls on functions from the cacti project. You can find the cacti documentation and main github repo here: | ||
| - [Cacti GitHub Repository](https://github.com/Cacti/cacti/tree/1.2.x) | ||
| - [Cacti Documentation](https://www.github.com/Cacti/documentation) |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link URL has 'www.github.com' which is incorrect - GitHub's documentation is at 'github.com' without the 'www' subdomain. The correct URL should be https://github.com/Cacti/documentation.
| - [Cacti Documentation](https://www.github.com/Cacti/documentation) | |
| - [Cacti Documentation](https://github.com/Cacti/documentation) |
| 5. **Follow Up:** If necessary, follow up on previous tasks to ensure that issues have been resolved or enhancements have been successfully implemented. | ||
|
|
||
|
|
||
| ## Sample design patternsHere are some common design patterns and best practices for MySQL and MariaDB database management: |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after the heading '##' in 'Sample design patternsHere'. Should be '## Sample design patterns' with 'Here are...' on a new line or with proper spacing.
| print "<td class='left' style='padding-left:30px;'>" . $dm['logtime'] . "</td>"; | ||
| print "<td class='left'>" . (isset($hosts[$dm['host_id']]) ? $hosts[$dm['host_id']] : __('Unknown', 'syslog')) . "</td>"; | ||
| print "<td class='left'>" . $dm['program'] . "</td>"; | ||
| print "<td class='left syslogMessage'>" . filter_value(title_trim($dm[$syslog_incoming_config['textField']], get_request_var_request('trimval')), get_request_var('rfilter')) . "</td>"; | ||
| print "<td class='left'>" . (isset($facilities[$dm['facility_id']]) ? $facilities[$dm['facility_id']] : __('Unknown', 'syslog')) . "</td>"; | ||
| print "<td class='left'>" . (isset($priorities[$dm['priority_id']]) ? $priorities[$dm['priority_id']] : __('Unknown', 'syslog')) . "</td>"; |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new grouped message detail rows render database fields like logtime, host, program, message, facility, and priority directly into <td> elements without HTML-escaping, e.g. filter_value(title_trim($dm[$syslog_incoming_config['textField']], ...), get_request_var('rfilter')) is printed raw. Because syslog message and related fields originate from syslog traffic and can contain attacker-controlled content (including HTML/JS), expanding a grouped row can result in stored XSS in the browser. To mitigate this, ensure all untrusted fields in these detail rows are passed through html_escape (or an equivalent encoding) before output, or reuse the same safe rendering helper used for the main syslog table rows so that any HTML metacharacters are properly encoded.
| print "<td class='left' style='padding-left:30px;'>" . $dm['logtime'] . "</td>"; | |
| print "<td class='left'>" . (isset($hosts[$dm['host_id']]) ? $hosts[$dm['host_id']] : __('Unknown', 'syslog')) . "</td>"; | |
| print "<td class='left'>" . $dm['program'] . "</td>"; | |
| print "<td class='left syslogMessage'>" . filter_value(title_trim($dm[$syslog_incoming_config['textField']], get_request_var_request('trimval')), get_request_var('rfilter')) . "</td>"; | |
| print "<td class='left'>" . (isset($facilities[$dm['facility_id']]) ? $facilities[$dm['facility_id']] : __('Unknown', 'syslog')) . "</td>"; | |
| print "<td class='left'>" . (isset($priorities[$dm['priority_id']]) ? $priorities[$dm['priority_id']] : __('Unknown', 'syslog')) . "</td>"; | |
| print "<td class='left' style='padding-left:30px;'>" . html_escape($dm['logtime']) . "</td>"; | |
| print "<td class='left'>" . (isset($hosts[$dm['host_id']]) ? html_escape($hosts[$dm['host_id']]) : __('Unknown', 'syslog')) . "</td>"; | |
| print "<td class='left'>" . html_escape($dm['program']) . "</td>"; | |
| print "<td class='left syslogMessage'>" . filter_value(title_trim(html_escape($dm[$syslog_incoming_config['textField']]), get_request_var_request('trimval')), get_request_var('rfilter')) . "</td>"; | |
| print "<td class='left'>" . (isset($facilities[$dm['facility_id']]) ? html_escape($facilities[$dm['facility_id']]) : __('Unknown', 'syslog')) . "</td>"; | |
| print "<td class='left'>" . (isset($priorities[$dm['priority_id']]) ? html_escape($priorities[$dm['priority_id']]) : __('Unknown', 'syslog')) . "</td>"; |
This pull request introduces a major new feature to the syslog plugin: the ability to group duplicate syslog messages in the UI, making it easier to review and analyze large volumes of similar log entries. It also adds new agent templates for code quality, PHP development, and MySQL/MariaDB database administration, and includes minor documentation and style improvements.
Syslog Plugin: Message Grouping Feature
syslog-detail-rowclass for improved styling and hover effects. [1] [2]Documentation and Developer Experience
Agent Templates