Improve error logging for DCM operations#101
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves observability for the DCM admin API by adding more explicit error logging around failed persistence operations and failed http.ResponseWriter → XResponseWriter type assertions.
Changes:
- Add structured error logs (with entity IDs and error details) for failed create/update/delete operations across multiple DCM settings services.
- Add explicit error logs when
http.ResponseWritercannot be cast to*xwhttp.XResponseWriterin several handlers/controllers. - Introduce a
logrusimport indcmformula_service.goto support the new structured logs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| adminapi/dcm/vod_settings_service.go | Adds structured error logs for VOD settings DB write/delete failures. |
| adminapi/dcm/test_page_controller.go | Logs when ResponseWriter type assertion fails in test page handler. |
| adminapi/dcm/logupload_settings_service.go | Adds structured error logs for log upload settings DB write/delete failures. |
| adminapi/dcm/logrepo_settings_service.go | Adds structured error logs for log repo settings DB write/delete failures. |
| adminapi/dcm/device_settings_service.go | Adds structured error logs for device settings DB write/delete failures. |
| adminapi/dcm/dcmformula_service.go | Adds structured error logs for formula/rule persistence failures (and adds logrus import). |
| adminapi/dcm/dcmformula_handler.go | Logs when ResponseWriter type assertion fails in several handlers; logs on priority update persistence failure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| xwhttp "github.com/rdkcentral/xconfwebconfig/http" | ||
| "github.com/rdkcentral/xconfwebconfig/rulesengine" | ||
| re "github.com/rdkcentral/xconfwebconfig/rulesengine" | ||
| "github.com/rdkcentral/xconfwebconfig/shared/logupload" | ||
| log "github.com/sirupsen/logrus" |
There was a problem hiding this comment.
The import block includes the same package path twice: github.com/rdkcentral/xconfadmin/common is imported both unaliased (common) and as xcommon. Go does not allow duplicate imports of the same path, so this will fail to compile. Remove one of these imports and update references to use the remaining import name consistently.
| for _, entry := range changedDcmRules { | ||
| entry.(*logupload.DCMGenericRule).Updated = util.GetTimestamp() | ||
| if err := db.GetCachedSimpleDao().SetOne(db.TABLE_DCM_RULE, entry.GetID(), entry); err != nil { | ||
| log.WithFields(log.Fields{"entity_id": entry.GetID(), "error": err}).Error("failed to create dcm formula") |
There was a problem hiding this comment.
This log message is misleading: the failing SetOne call is persisting a mix of new and re-prioritized existing rules after reorganization, not strictly "create". Consider using wording like "failed to persist dcm rule changes" (and/or include operation context) so logs are accurate during priority rebalancing failures.
| log.WithFields(log.Fields{"entity_id": entry.GetID(), "error": err}).Error("failed to create dcm formula") | |
| log.WithFields(log.Fields{"entity_id": entry.GetID(), "error": err}).Error("failed to persist dcm rule changes") |
This pull request improves error logging throughout the DCM (Dynamic Configuration Management) admin API services. It adds detailed log messages wherever critical operations fail, especially when database updates or deletions do not succeed, or when type assertions for response writers fail. This will make debugging and monitoring much easier by providing more context in log outputs.
The most important changes are:
Enhanced Error Logging for Database Operations:
CreateDcmRule,UpdateDcmRule,DeleteDcmFormulabyId, and their equivalents for device, log, and VOD settings. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]Improved Error Logging for Response Writer Type Assertions:
http.ResponseWritertoXResponseWriterin various handler functions, making it clear where and why the failure occurred. [1] [2] [3] [4] [5]Dependency Update:
logruslogging library indcmformula_service.goto support structured logging.