pkg/cloud/server.go improvements

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

carverauto/serviceradar#28

mfreeman451 commented

2026-03-28 04:16:08 +00:00

Owner

Imported from GitHub.

Original GitHub issue: #65
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/issues/65
Original created: 2025-01-20T03:31:16Z

Error Handling: Error handling in some functions (e.g., periodicCleanup, checkInitialStates) could be improved by returning errors instead of just logging them. This would allow for better monitoring of background tasks.
Configuration: The default database path (defaultDBPath) is hardcoded. It would be better to make it configurable.
Constants: Constants like oneDay, oneWeek, nodeHistoryLimit, etc., could be made configurable or moved to a separate configuration package.
checkNeverReportedPollers(): The logic for sending an alert when a poller never reports could be refined. It currently sends an alert only once at startup. It might be useful to have periodic checks or a configurable timeout.
The getLastDowntime() function logs an error but returns time.Time{}, this will mask the error.
The checkNodeExists(), insertNewNode(), and updateExistingNode() could be defined on the db.DB type directly.
The statusUnknown constant could be made unexported (lowercase).
The querySQL variables should be named as constants.
The maxMessageSize constant could be made configurable.
getHostname() is defined but could just use os.Hostname() directly.
The function MonitorPollers() has a select block that has a time.After() case as well as the ticker. The time.After() logic would make more sense being added to the periodicCleanup() function.
The node monitoring logic has high coupling to alerts. If other notification methods are added in the future, it may be better to extract the alert generation logic.

Imported from GitHub. Original GitHub issue: #65 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/issues/65 Original created: 2025-01-20T03:31:16Z --- - [ ] Error Handling: Error handling in some functions (e.g., periodicCleanup, checkInitialStates) could be improved by returning errors instead of just logging them. This would allow for better monitoring of background tasks. - [ ] Configuration: The default database path (defaultDBPath) is hardcoded. It would be better to make it configurable. - [ ] Constants: Constants like oneDay, oneWeek, nodeHistoryLimit, etc., could be made configurable or moved to a separate configuration package. - [ ] checkNeverReportedPollers(): The logic for sending an alert when a poller never reports could be refined. It currently sends an alert only once at startup. It might be useful to have periodic checks or a configurable timeout. - [ ] The getLastDowntime() function logs an error but returns time.Time{}, this will mask the error. - [ ] The checkNodeExists(), insertNewNode(), and updateExistingNode() could be defined on the db.DB type directly. - [ ] The statusUnknown constant could be made unexported (lowercase). - [ ] The querySQL variables should be named as constants. - [ ] The maxMessageSize constant could be made configurable. - [ ] getHostname() is defined but could just use os.Hostname() directly. - [ ] The function MonitorPollers() has a select block that has a time.After() case as well as the ticker. The time.After() logic would make more sense being added to the periodicCleanup() function. - [ ] The node monitoring logic has high coupling to alerts. If other notification methods are added in the future, it may be better to extract the alert generation logic.

mfreeman451 closed this issue

2026-03-28 04:16:09 +00:00

Rows
Columns

pkg/cloud/server.go improvements #28