RDKDEV-1442, RDKBDEV-3388 Add documentation for rdklogger - #58
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new, generic component-level README under docs/ to document RDK Logger’s architecture, runtime control flow, configuration model, and integration points for consumers of librdkloggers.
Changes:
- Introduces
docs/README.mdwith detailed design/flow diagrams (Mermaid), module breakdown, and configuration guidance. - Documents dynamic log-level control via
rdklogctrl(UDP control path) and the debug.ini override chain.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (5)
docs/README.md:119
- This synchronization bullet says the main logging path relies on Log4C’s own thread safety for priority checks, but
rdk_dbg_priv_log_msg()takesgLoggingMutexbefore callinglog4c_category_is_priority_enabled()/log4c_category_vlog(). The doc should reflect thatgLoggingMutexserializes Log4C category access in the logging path.
- **Synchronization**: Two `pthread_mutex_t` instances are used — `gInitMutex` guards the initialization state flag and `rdk_logger_ext_init` path; `gLoggingMutex` guards extended initialization of per-module appenders. The main logging path relies on Log4C's own thread safety for category priority checks.
docs/README.md:438
- For consistency with the rest of the doc (and to reflect what
rdk_logger_level_from_string()accepts), it would help to note thatWARNINGis also accepted as an alias ofWARNin this “Available levels” list.
# Available levels: FATAL, ERROR, WARN, NOTICE, INFO, DEBUG, TRACE, NONE
docs/README.md:169
- In the “Initialization to Active State” sequence diagram,
rdk_dbg_priv_config()is shown being called beforerdk_dbg_priv_init(), but the actual implementation (rdk_logger_init()insrc/rdk_logger_init.c) callsrdk_dbg_priv_init()first and only then callsrdk_dbg_priv_config(debugConfigFile). This ordering matters because custom layout/appender types are registered beforelog4c_init()and before parsingdebug.ini.
Note over Logger: Check /opt/debug.ini via access()
Logger->>FS: access(DEBUG_INI_OVERRIDE_PATH_1)
alt /opt/debug.ini accessible
FS-->>Logger: File found
Logger->>PrivCore: rdk_dbg_priv_config("/opt/debug.ini")
docs/README.md:255
- The request-processing diagram claims
rdk_dbg_priv_log_msg()does avsnprintfstep, but the implementation directly callslog4c_category_vlog(cat, log4cPriority, format, args)without pre-formatting into a buffer. Keeping this step in the diagram misrepresents the runtime behavior.
PrivLog->>PrivLog: vsnprintf into format buffer
PrivLog->>Log4C: log4c_category_vlog(cat, log4cPriority, format, args)
docs/README.md:418
- The document alternates between
WARNINGandWARNfor the same log level. Sincerdk_logger_level_from_string()accepts both strings, consider documenting both forms here to avoid confusion for users editingdebug.ini.
This issue also appears on line 438 of the same file.
| `LOG.RDK.<MODULE>` | string (log level) | Inherits `LOG.RDK.DEFAULT` | Sets the log level for a specific named module. Accepted values: `FATAL`, `ERROR`, `WARNING`, `NOTICE`, `INFO`, `DEBUG`, `TRACE`, `NONE`. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (3)
docs/README.md:120
- The threading model says the main logging path relies on Log4C thread safety for category priority checks, but
rdk_dbg_priv_log_msg()andrdk_logger_is_logLevel_enabled()both takegLoggingMutexbefore callinglog4c_category_is_priority_enabled(). This sentence should reflect that the mutex serializes those checks too.
- **Synchronization**: Two `pthread_mutex_t` instances are used — `gInitMutex` guards the initialization state flag and `rdk_logger_ext_init` path; `gLoggingMutex` guards extended initialization of per-module appenders. The main logging path relies on Log4C's own thread safety for category priority checks.
docs/README.md:135
- The lifecycle summary says "Shutdown" releases resources, but
rdk_logger_deinit()only callsrdk_dyn_log_deinit()(closes the UDP socket) and does not tear down the Log4C backend or resetisLogInited. This can mislead readers about what deinit actually does.
The component transitions through the following states during its lifecycle: **Initializing** (mutex acquired, check `isLogInited` flag) → **LoadingConfig** (parse the selected debug.ini, populate Log4C category priorities) → **InitializingBackend** (register custom layouts and appenders, call `log4c_init()`) → **StartingDynamicControl** (open UDP socket on port 12035, bind to `127.255.255.255`) → **Active** (processing `RDK_LOG()` calls, servicing runtime control messages) → **Shutdown** (close UDP socket, release resources).
docs/README.md:196
- This bullet claims log messages are suppressed when initialization fails, but the logging APIs don’t check
isLogInited(it’s only used in init/deinit). The docs should avoid asserting suppression that the implementation doesn’t enforce.
- **Configuration File Not Found**: If the selected debug.ini cannot be opened, `rdk_logger_parse_config()` returns `RDK_FAILURE`, initialization of RDK Logger fails, and `isLogInited` remains false. Log messages issued before a successful initialization are suppressed.
|
Added file-based logging support throughout the documentation |
RDKDEV-1442, RDKBDEV-3388
Reason for Change:
To add a generic component Documentation for rdklogger.
Fix:
Added the README.md documentation
Signed-off-by: gourivarma3