Skip to content

Logging/log emitter - #90

Merged
manish-wekan merged 7 commits into
nitrocloudofficial:developfrom
Prateep07:logging/log-emitter
Oct 28, 2026
Merged

manish-wekan merged 7 commits into
nitrocloudofficial:developfrom
Prateep07:logging/log-emitter

Conversation

@Prateep07

@Prateep07 Prateep07 commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Add a process-wide log_emitter with subscribe(callback) -> unsubscribe.
  • Forward structured records from NitroStack loggers to subscribers, including level, message, timestamp, logger name, and extra fields.
  • Deliver records through bounded per-subscriber buffers so slow callbacks don’t block logging. Drop the oldest queued record when a buffer is full.
  • Isolate subscriber exceptions and stop delivery on unsubscribe.
  • Preserve FileLogger output configuration and capture HTTP logger records when propagation is disabled.

closes #72

Testing

  • Logging tests: passed.
  • Full suite: 834 passed, 4 failed.
  • An untouched copy of HEAD also had failures: 826 passed, 5 failed. The shared failures involved missing wheel (bdist_wheel), installed NitroStack metadata reporting 0.4.0 while pyproject.toml declares 0.4.2, and intermittent OAuth socket errors.
  • Fixed the HTTP logging test’s handler cleanup; it passes in the current run.

@AadarshM07

Copy link
Copy Markdown
Collaborator

Hi @Prateep07, thanks for the PR
Could you rebase this PR with the develop branch and remove the additional commit dependencies? Also, it would be great if you could follow the Closes #<issue-number> format in the description. This will link the issue and automatically close it when the PR is merged.

@Prateep07
Prateep07 force-pushed the logging/log-emitter branch from 75a6bf9 to 6ebf6ba Compare October 26, 2026 14:38

@AadarshM07 AadarshM07 left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changes looks good to me, tests also passes. we will need to run the pipeline once.

@manish-wekan

Copy link
Copy Markdown
Collaborator

@Prateep07
Thanks, the emitter works well: the full suite passes on develop, with no duplicate records, and unsubscribe and raising subscribers behave as #72 asks.

One fix before merge: install_log_handler() runs at import and adds a handler to the nitrostack logger, which stops Python's lastResort stderr fallback.

As a result, NitroStack warnings logged before the first FileLogger exists now disappear (e.g. No widget HTML found for route … shows on develop but not on this PR).

Please install the handler lazily: add it on the first subscribe(), remove it after the last unsubscribe, and add a test that a nitrostack warning still reaches stderr when nothing has subscribed.

~sent from claude-code

@manish-wekan

manish-wekan commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks, the lazy install fixes the lastResort issue, and I verified that warnings reach stderr again with no subscriber. One gap left: when _enable_trace_logging() runs before the first subscribe() (the normal server-start order), install_log_handler("nitrostack.transports.http") returns early because nobody is subscribed yet.

Records from that logger, which has propagate=False, then never reach subscribers. Please record the requested logger names in install_log_handler even with no subscribers, have subscribe() attach to all of them, and add a test for the "trace enabled, then subscribe" order.

~sent from claude-code

…cribe

install_log_handler() returned early when nobody was subscribed, so a
logger with propagate=False (nitrostack.transports.http, set up by
_enable_trace_logging at server start) was never bridged once tooling
subscribed later. Remember registered logger names and attach to all of
them on subscribe.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@manish-wekan
manish-wekan merged commit a56aa78 into nitrocloudofficial:develop Oct 28, 2026
3 checks passed
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.

4 participants