Skip to content

Logging Refactor#1751

Merged
ifrit98 merged 21 commits intostagingfrom
feature/logging-refactor
Mar 25, 2024
Merged

Logging Refactor#1751
ifrit98 merged 21 commits intostagingfrom
feature/logging-refactor

Conversation

@sepehr-opentensor
Copy link
Contributor

@sepehr-opentensor sepehr-opentensor commented Mar 19, 2024

This PR removes the loguru dependency and replaces the underlying logging mechanism with the standard Python built-in logging library for compatibility with 3rd party modules and libraries.

Making this change will grant visibility into these 3rd party libraries for performance evaluation and debugging, which is inherently necessary for future updates to bittensor and btcli. These changes also preserve the existing API, and as such, users should remain unaffected.

@sepehr-opentensor sepehr-opentensor marked this pull request as ready for review March 20, 2024 14:50
Sepehr Ghavam and others added 11 commits March 21, 2024 18:38
To enable and manage third party loggers, loguru needed to be replaced with the builtin logging library. To maintain functional parity and best manage the logging infra state, the new log manager was built using a state machine, in order to explicitly handle transitions between the various logger states.
Removed some commented code that will be replaced anyway.
In order to capture logs from processes, a QueueHandler must be added to the logger that is instantiated with the LoggingMachine queue. The file log, if enabled, uses the trace format output for all loggers regardless of higher level settings. This is to avoid modifying the formatter during program execution.
To enable stdout streaming of external process loggers within the main process, all log record handling has been moved to a single QueueListener owned by the LoggingMachine. This way, handlers can be added or removed as necessary. To ensure that state transitions are respected temporally, as the QueueListener operates in a separate thread, a Lock was added to control operations on anything related to the QueueListener.
Since underlying calls to the logger for the various log functions are also concurrent, and the QueueListener is running on a separate thread, these actions must be synchronized so that the order of operations for state setting and logging on either side are respected.
…tions.

Turns out, the only threaded component was the QueueListener, and instead of synchronizing the logger calls, the listener can simply be stopped during a state transition. Thankfully, the StateMachine API makes this change trivial.
Some house keeping to the LoggingMachine initialization, and added unit tests to ensure state transitions work as expected.
@sepehr-opentensor sepehr-opentensor force-pushed the feature/logging-refactor branch from 639755c to bcf5bc6 Compare March 21, 2024 22:39
Copy link
Contributor

@ifrit98 ifrit98 left a comment

Choose a reason for hiding this comment

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

LGTM!

@ifrit98 ifrit98 merged commit 9ec0687 into staging Mar 25, 2024
@ifrit98 ifrit98 mentioned this pull request Mar 25, 2024
@unconst unconst deleted the feature/logging-refactor branch March 27, 2024 20:04
@birkskyum
Copy link

This breaking change was introduced in a minor release

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