Skip to content

Add bulk change data source profile functionality with automatic RRD rebuild #6495

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.

Sign up for GitHub

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

Open
terrellgf wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Add bulk change data source profile functionality with automatic RRD rebuild #6495

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
191b017
Update data_sources.php
terrellgf Dec 23, 2025
00862be
Update data_sources.php
TheWitness Jan 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions data_sources.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
3 => __('Enable'),
4 => __('Change Device'),
5 => __('Reapply Suggested Names'),
6 => __('Change Data Source Profile')
];

$actions = api_plugin_hook_function('data_source_action_array', $actions);
Expand Down Expand Up @@ -456,6 +457,67 @@ function form_actions() {
api_reapply_suggested_data_source_data($selected_items[$i]);
update_data_source_title_cache($selected_items[$i]);
}
} elseif (get_nfilter_request_var('drp_action') == '6') { // change data source profile
get_filter_request_var('data_source_profile_id');

$new_profile = db_fetch_row_prepared('SELECT * FROM data_source_profiles WHERE id = ?',
array(get_request_var('data_source_profile_id')));

if (!empty($new_profile)) {
$rrd_changes = 0;

for ($i=0;($i<cacti_count($selected_items));$i++) {
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

Loop formatting is inconsistent with the codebase conventions. Throughout this file, loops use the pattern for ($i = 0; ($i < cacti_count($selected_items)); $i++) with spaces around operators, but this line has inconsistent spacing for ($i=0;($i<cacti_count($selected_items));$i++). Please update to match the established pattern seen in lines 444, 448, and 456.

Copilot uses AI. Check for mistakes.
// Get current step value
$current_step = db_fetch_cell_prepared('SELECT rrd_step FROM data_template_data WHERE local_data_id = ?',
array($selected_items[$i]));

// Update all database tables
db_execute_prepared('UPDATE data_template_data
SET data_source_profile_id = ?, rrd_step = ?
WHERE local_data_id = ?',
array(get_request_var('data_source_profile_id'), $new_profile['step'], $selected_items[$i])
);

Comment on lines +462 to +480
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

Missing validation that the selected data_source_profile_id exists and is valid before fetching it from the database. While db_fetch_row_prepared() returns an empty array for non-existent IDs (checked at line 466), it would be more robust to validate that get_request_var('data_source_profile_id') is a positive integer first. The get_filter_request_var() call on line 461 validates it as an integer but doesn't ensure it's positive or exists in the database.

Suggested change
$new_profile = db_fetch_row_prepared('SELECT * FROM data_source_profiles WHERE id = ?',
array(get_request_var('data_source_profile_id')));
if (!empty($new_profile)) {
$rrd_changes = 0;
for ($i=0;($i<cacti_count($selected_items));$i++) {
// Get current step value
$current_step = db_fetch_cell_prepared('SELECT rrd_step FROM data_template_data WHERE local_data_id = ?',
array($selected_items[$i]));
// Update all database tables
db_execute_prepared('UPDATE data_template_data
SET data_source_profile_id = ?, rrd_step = ?
WHERE local_data_id = ?',
array(get_request_var('data_source_profile_id'), $new_profile['step'], $selected_items[$i])
);
$profile_id = intval(get_request_var('data_source_profile_id'));
if ($profile_id > 0) {
$new_profile = db_fetch_row_prepared('SELECT * FROM data_source_profiles WHERE id = ?',
array($profile_id));
if (!empty($new_profile)) {
$rrd_changes = 0;
for ($i=0;($i<cacti_count($selected_items));$i++) {
// Get current step value
$current_step = db_fetch_cell_prepared('SELECT rrd_step FROM data_template_data WHERE local_data_id = ?',
array($selected_items[$i]));
// Update all database tables
db_execute_prepared('UPDATE data_template_data
SET data_source_profile_id = ?, rrd_step = ?
WHERE local_data_id = ?',
array($profile_id, $new_profile['step'], $selected_items[$i])
);

Copilot uses AI. Check for mistakes.
db_execute_prepared('UPDATE data_template_rrd
SET rrd_heartbeat = ?
WHERE local_data_id = ?',
array($new_profile['heartbeat'], $selected_items[$i])
);

db_execute_prepared('UPDATE poller_item
SET rrd_step = ?
WHERE local_data_id = ?',
array($new_profile['step'], $selected_items[$i])
);
Comment on lines +474 to +491
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The implementation does not handle remote pollers. When data source profiles are changed, the updates must also be replicated to remote pollers. The existing codebase pattern for similar operations (such as api_data_source_enable at lib/api_data_source.php:385 and api_data_source_change_host at lib/api_data_source.php:586) shows that after updating data_template_data, data_template_rrd, and poller_item tables locally, you should:

  1. Fetch the device_id/poller_id from the data source
  2. Use poller_push_to_remote_db_connect() to get the remote connection
  3. Execute the same UPDATE queries on the remote poller database

Without this, remote pollers will have stale configuration data and the profile change won't take effect on those pollers.

Copilot uses AI. Check for mistakes.
Comment on lines +474 to +491
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The operation updates the database tables before checking if the RRD step actually changed. If the new profile has the same step value as the current one, the database updates are unnecessary. Consider checking $current_step != $new_profile['step'] before performing any database updates to avoid unnecessary writes and improve performance in bulk operations.

Copilot uses AI. Check for mistakes.

update_poller_cache($selected_items[$i], true);
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The update_poller_cache() function is called with the commit parameter set to true for each individual data source in the loop. This causes immediate commits for every iteration. Looking at the utility.php implementation (line 164), update_poller_cache can be called without commit and accumulated, then batch-committed using poller_update_poller_cache_from_buffer() after the loop completes. This pattern is used in repopulate_poller_cache() (lib/utility.php:67) and would significantly improve performance when updating many data sources.

Copilot uses AI. Check for mistakes.

// Handle RRD file if step changed
if ($current_step != $new_profile['step']) {
$rrd_path = get_data_source_path($selected_items[$i], true);
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The get_data_source_path() function is called inside a loop for each data source individually. This could be inefficient for bulk operations with many data sources. Consider batching RRD path lookups if possible, or at minimum, only call this function when the step value has changed (move it inside the if condition at line 496).

Copilot uses AI. Check for mistakes.
if (file_exists($rrd_path)) {
// Backup with timestamp and delete
$backup = $rrd_path . '.bak_' . date('Ymd-His');
if (copy($rrd_path, $backup)) {
unlink($rrd_path);
$rrd_changes++;
cacti_log("RRD backed up and removed: $rrd_path -> $backup", false, 'DATASOURCE');
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

Missing error handling for the file copy operation. If copy() fails but doesn't throw an error (e.g., due to disk space or permission issues), the code silently continues without incrementing rrd_changes or logging the failure. The spikekill implementation (lib/spikekill.php:451-457) demonstrates proper error handling by checking the copy result and setting an error if it fails. Consider logging a warning when copy fails so administrators are aware the backup didn't succeed.

Suggested change
cacti_log("RRD backed up and removed: $rrd_path -> $backup", false, 'DATASOURCE');
cacti_log("RRD backed up and removed: $rrd_path -> $backup", false, 'DATASOURCE');
} else {
/* Log a warning so administrators are aware the backup did not succeed */
cacti_log("WARNING: Failed to backup RRD file '$rrd_path' to '$backup'", false, 'DATASOURCE');

Copilot uses AI. Check for mistakes.
}
}
}
}

raise_message(1);

if ($rrd_changes > 0) {
$_SESSION['sess_messages']['custom_info'] = array(
'message' => sprintf(__('%d RRD files were backed up and will be recreated with new step value at next polling.'), $rrd_changes),
'type' => 'info'
);
}
Comment on lines +512 to +517
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

Direct modification of $_SESSION is not following established Cacti patterns for user messages. Throughout the codebase, user messages are handled via the raise_message() function. This custom session message approach is inconsistent and may not integrate properly with Cacti's message display system. Consider using raise_message() with a custom message code, or verify that the custom_info session pattern is supported by the message display functions.

Copilot uses AI. Check for mistakes.
} else {
raise_message(2);
}
} else {
api_plugin_hook_function('data_source_action_execute', get_nfilter_request_var('drp_action'));
}
Expand Down Expand Up @@ -590,6 +652,20 @@ function form_actions() {
'pmessage' => __('Click \'Continue\' to Reapply Suggested Names for the following Data Sources.'),
'scont' => __('Reapply Suggested Names for Data Source'),
'pcont' => __('Reapply Suggested Names for Data Sources')
],
6 => [
'smessage' => __('Click \'Continue\' to Change the Data Source Profile for the following Data Source.'),
'pmessage' => __('Click \'Continue\' to Change the Data Source Profile for the following Data Sources.'),
'scont' => __('Change Data Source Profile'),
'pcont' => __('Change Data Source Profiles'),
'extra' => [
'data_source_profile_id' => [
'method' => 'drop_sql',
'title' => __('New Data Source Profile'),
'default' => '',
'sql' => 'SELECT id, name FROM data_source_profiles ORDER BY name'
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The SQL query should include a WHERE clause to filter out any inactive or deleted profiles if such a status exists in the data_source_profiles table. Review the data_source_profiles schema to determine if there are status flags that should be considered when presenting profile options to users.

Suggested change
'sql' => 'SELECT id, name FROM data_source_profiles ORDER BY name'
'sql' => 'SELECT id, name FROM data_source_profiles WHERE active="on" ORDER BY name'

Copilot uses AI. Check for mistakes.
]
]
]
]
];
Expand Down