Skip to content
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

Provide more control for how log messages are viewed in an application #58

Closed
petebankhead opened this issue Sep 16, 2023 · 1 comment · Fixed by #59
Closed

Provide more control for how log messages are viewed in an application #58

petebankhead opened this issue Sep 16, 2023 · 1 comment · Fixed by #59
Assignees

Comments

@petebankhead
Copy link
Member

I am struggling a bit to use LoggerManager, and I'm not sure if I'm approaching it correctly or if the API should be reconsidered.

The Goals

In QuPath, I would like to be able to:

  1. Access log message counts outside any log viewer control
  2. Create multiple controls that are populated by the same log messages
  3. Optionally clear the log messages in some controls, but not others
  4. Optionally specify that a control starts and stops displaying log messages

Specifically, for 3. and 4. I want to create textarea/richtext control that can independently have their contents cleared.
For example, the text-based log might show log messages only while a script is running (but not before or afterwards).

I think other applications may benefit from similar functionality, since it would give much more freedom in how log messages can be displayed.

The Problems

As far as I can see, LogViewer manages its own LogViewerModel, which is an implementation of LoggerListener. The LogViewerModel is package-private and not exposed anywhere through the public API.

This means I can't access a LoggerListener or LoggerManager.

If I create a new log viewer (of any type), I have no control over which LoggerManager will be used.

Based upon LoggerListener.getCurrentLoggerManager() and the use of a service loader, I expect that a new LoggerManager would be created for each viewer. This suggests more overhead.

It seems counter-intuitive to me that

  • a LoggerManager is returned via a default method defined in the LoggerListener interface (rather than a static method from the LoggerManager class, for example)
  • getCurrentLoggerManager() suggests that it is returning a singleton - but I don't think it is. 'Current' also suggests it might change, and the use of Optional implies that it is optional - but it isn't really (i.e. without it, log viewing won't work at all).

The Suggestions

  • Make it possible to access / create a LoggerManager separately from an UI control (and separate from an LoggerListener instance)
    • I suggest a static method List<LogManager> getAvailableLogManagers() that returns a list of managers with an active framework; then the calling code can decide which to use (or act if the number returned isn't 1)
  • Pass the LoggerManager to the UI control during its creation
    • If none is provided, then the UI may request its own LoggerManager - but this shouldn't be the only option
  • Make it possible to access the LoggerManager via each log viewer implementation
  • Add a method LoggerManager.removeListener(LoggerListener)
  • Use the fact that log listeners can be added and removed to implement start/stop logging options within the log viewer implementations
  • Add a method LoggerListener.rootLogLevelChanged(Level oldLevel, Level newLevel)
    • This may have a default, do-nothing implementation if you want LoggerListener to remain a functional interface
  • Provide access to the log message counts outside of LogViewer

What do you think?

@Rylern
Copy link
Contributor

Rylern commented Sep 18, 2023

I suggest a static method List getAvailableLogManagers() that returns a list of managers with an active framework; then the calling code can decide which to use (or act if the number returned isn't 1)

The number of active framework will always be 0 or 1 (SFL4J automatically discards additional active frameworks), so isn't it better to keep using an Optional instead of a List?

Add a method LoggerListener.rootLogLevelChanged(Level oldLevel, Level newLevel). This may have a default, do-nothing implementation if you want LoggerListener to remain a functional interface

This method would need to be called from the logging implementations (e.g. LogbackManager). However, for the logback and reload4j loggers, I can't listen to changes of levels. I can listen to the level changes if they are done through the LoggerManager, but not if they are done using the underlying logger. Isn't it an issue?

The rest of the comments have been implemented in #59.

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 a pull request may close this issue.

2 participants