Skip to content

Add client logger - #17

Merged
jmw182 merged 12 commits into
cmake-buildfrom
client-logger
Nov 9, 2026
Merged

jmw182 merged 12 commits into
cmake-buildfrom
client-logger

Conversation

@jmw182

@jmw182 jmw182 commented Sep 1, 2026

Copy link
Copy Markdown
Member

Port the pyrtma client logger to the c++ rtma library, deprecating the external cc_logger library. Written with github copilot (GPT-5.6-terra).


Stack created with GitHub Stacks CLI • Give Feedback 💬

@jmw182
jmw182 force-pushed the client-logger branch 4 times, most recently from 84ae39a to 7725658 Compare September 1, 2026 19:12
@jmw182

jmw182 commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

I'm burning through copilot tokens on this first day of the month, but I'm considering building this on top of spdlog.

Response from copilot:

Yes, spdlog would improve several weak spots in the current implementation:

  • Reliable rotating-file sinks, including size limits and backup handling.
  • Better console formatting and platform-safe output.
  • Thread-safe sinks and optional asynchronous logging.
  • It already uses fmt, so it complements the dependency already added.

You would still need an RTMA-specific sink that converts spdlog::details::log_msg into MDF_RTMA_LOG and sends it through the owning RTMA_Module. You would also need a small wrapper to retain the PyRTMA-compatible API and behavior: enable_rtma, per-sink levels, dynamic client-name updates, child names, and disabling RTMA output after send failures.

So it is not substantially more complicated overall. The initial integration is more work, but it replaces hand-rolled file rotation and console code in RTMA_Logger.cpp, which are the parts most likely to accumulate platform and concurrency bugs.

I would prefer spdlog if this logger is intended to become a supported RTMA API. Keep the wrapper thin:

RTMA_Logger
  -> spdlog::logger
      -> stdout_color_sink_mt
      -> rotating_file_sink_mt
      -> rtma_sink_mt  // custom sink

The custom rtma_sink_mt should:

  1. Map level to MT_RTMA_LOG*.
  2. Populate the fixed MDF_RTMA_LOG layout and truncation rule.
  3. Skip sends when disconnected.
  4. Disable itself on send failure to prevent logging recursion.

The main tradeoff is dependency weight and exported ABI complexity. Since fmt is already a public dependency, adding spdlog is a reasonable next step, preferably pinned and fetched through CMake just as fmt is.

@jmw182

jmw182 commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

I'm not sure if integrating spdlog is worth the cost of time and/or tokens at the moment, but this branch needs some work to at least be on par with cc_logger. I had put some effort into the cc_logger formatting (approximating that of the python rich logger), but copilot does not appear to have copied much or any of that over (e.g. no use of color).

I updated quicklogger to use the logger, which would eliminate the need for the DEBUG_TEXT message.

@jmw182
jmw182 marked this pull request as ready for review October 28, 2026 20:45
@jmw182

jmw182 commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

I did some more iteration with copilot to bring this up to feature parity with cc_logger and fix some issues it found. Some fixes are likely overkill but could avoid future problems (e.g. loggers in multiple threads).

jmw182 added 12 commits October 29, 2026 17:51
commit was revised to fix copilot's whitespace (CRLF) mangling
Allow console output stream to be configured (defaults to stderr)
Allow colorized output to be disabled, including automatically when not outputting to a terminal
Add RTMA_LevelName and RTMA_LevelFromName functions
Fix log file name format
**Logger changes** (in `RTMA_Logger.cpp` and `RTMA_Logger.h`)
- **Locking:** every getter and setter now takes the mutex, and `GetLogName()` no longer deadlocks when `LogMessage` calls it.
- **File sink:** the file stays open, and a byte counter drives rotation at 3 MiB with three backups. I flush after every line. If the file can't be opened, written, or reopened after rotating, the sink turns itself off and prints one warning on the console stream.
- **RTMA send:** the payload is built under the lock but sent after releasing it. A per-thread guard stops a log call made during a send from sending again. A thrown send turns the RTMA sink off.
- **Dangling module:** `RTMA_Module`'s destructor calls a new `Detach()`, so a `Child()` logger that outlives the module no longer touches freed memory. It still logs to console and file.
- **Truncation:** only the RTMA payload is cut at 1023 characters. Console and file get the full message.
- **Time:** `MDF_RTMA_LOG.time` is now epoch seconds, taken once per call and shared with the console and file timestamps.
- **Windows color:** in AUTO mode the logger turns on virtual terminal processing once for the console stream, and skips color if that fails.

**Docs:** `RTMA.h` now says `RTMA_Module` and `SendMessage` are not thread-safe, and that logging to RTMA from several threads needs `Logger().SetRTMAEnabled(false)`.

**Tests:** `public_api_test.cpp` gained four cases:
- a child logger outliving its module;
- rotation over 11 MB, leaving `.1` to `.3` but no `.4`;
- a 2000-character message arriving whole with no escape codes;
- a short multi-threaded logging run.

`CMakeLists.txt` links `Threads::Threads` into the test.
@jmw182
jmw182 removed this pull request from stack #13 November 9, 2026 15:36
@jmw182
jmw182 added this pull request to stack #18 November 9, 2026 15:37
@jmw182
jmw182 merged commit 8044d6f into v4 Nov 9, 2026
6 checks passed
@jmw182
jmw182 deleted the client-logger branch November 9, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant