-
Notifications
You must be signed in to change notification settings - Fork 54
Revamp the LogService #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
One thing I did not do, and which needs testing: We want the ability to filter stdout and stderr console events based on the producer (i.e., generating class/package). We need to inspect the stack trace to figure that out. The LogService already has this feature, so probably, nothing new needs to be implemented. We can simply use the stdout and stderr log channels. However, the And one other needed improvement: the ConsoleService should be smart enough to recognize when an exception comes down the pipeline of stderr, and transform it into the associated exception of the emitted log message. Kinda nuts, but super useful to reconstruct failures. Definitely needs unit testing, though! |
|
Remaining action items:
|
We are going to add support for multiple Logger objects in the same SciJava context. The LogService instance will itself be the default Logger, but there will be others as well.
The default level went back and forth between INFO and WARN a couple of times, but it is currently INFO.
This change eliminates the various protected log methods of
AbstractLogService, in favor of a single method:
alwaysLog(int level, Object message, Throwable t)
All other logging methods in the Logger interface now have default
implementations which funnel to this method, which should make it
simpler to implement this interface.
This change also introduces new methods for logging information at any
arbitrary log level:
log(int level, Object message)
log(int level, Throwable t)
log(int level, Object message, Throwable t)
All level-specific logging methods now lean on these general-purpose log
methods, which do the level threshold check, and then call alwaysLog.
The logic in AbstractLogService is now generalized in a DefaultLogger implementation which implements Logger. The AbstractLogService now simply keeps an instance of DefaultLogger to which it delegates everything, apart from calls to alwaysLog, which is marks abstract. To better understand the motivation for this change, see next commits.
Now Logger extends the Named interface. This will be useful shortly.
We sometimes need Logger instances other than the LogService itself. All the logic of AbstractLogService is hereby migrated to a separate DefaultLogger class, instances of which are created on demand in response to LogService#channel(String) calls. Created Logger instances are stored in a hash by name. The AbstractLogService maintains a special instance of DefaultLogger as a channel called "default", which receives the log events fed to the LogService itself. Alternately, consuming code can call LogService#channel(String) to obtain a different channel to which information can be logged.
For performance reasons, we do a couple of tricky things: 1. We do not use a synchronize lock in the notifyListeners method; rather, we keep a cached copy of the listeners as an array reference and use that; see also 033a921. 2. In contrast to normal Java convention, we do not wrap the message details in a LogEvent. This tactic avoids the overhead of creating a new object every time a message is logged. With this API, it is now possible for interested parties to receive notification of any log messages of interest.
The LogLevel#prefix(int) method hasn't been released yet, so we can still modify the behavior before it goes gold. ;-)
I usually hate to do this, since it hurts startup time of a context. However, we really need to register as a listener for console events up front, so they can be forwarded to dedicated logger channels of the LogService (see next commit).
Specifically, we publish stdout messages to a dedicated "stdout" logger channel, and stderr messages to a dedicated "stderr" logger channel. Note that because console messages can contain partial lines, we use a simple heuristic to avoid publish stupid and/or empty log messages: 1) We accumulate console messages to a buffer; 2) Whenever we see a newline, we flush it; 3) Whenever the message ends in a newline, we trim one. See next commit for a test which illustrates why this works OK.
We now forward console messages to special logger channels of the LogService. This test ensures that that actually happens.
|
Hey @ctrueden, I'm sorry for starting a fork of this branch before contributing to this discussion. It was simply the workflow I used for the plot-service branch. But discussions are important so let's start: |
|
At first, I think we should clarify some user stories this branch is dedicated to:
|
|
What does not work:
|
|
Closing in favor of #255. |
As discussed with @fjug in Dresden:
LogServicenow sports a spiffy callback mechanism, so that you can register listeners for log events.Loggerchannels keyed by name, of which theLogServiceitself functions as the"default"one, preserving the convenience of the previous paradigm.ConsoleServicenow forwards all stdout and stderr messages to special channels of theLogServicenamed"stdout"and"stderr"respectively.@fjug Please review and test with your logging UI. I will merge once you sign off on it.