Community
Participate
Working Groups
OSGI based apps are not able to present important errors to the end-user. Everything is placed in the .log, for which the viewer is not always present. It should be possible to intercept logged events (so the app can notify the user about problems).
Created attachment 149431 [details] Fix proposition
The logging story in Eclipse is not a good one. I am loath to add another log listener. Closing as wontfix. But please keep in mind that we are looking to improve this story and really want to consolidate on a single logging API. The current thinking is to use the OSGi LogService to do that. In particular the extended log service we provide in Equinox.
Thomas, I completely understand your point of view and I want to assure you that I would not open that bug if I did not think it was necessary. But closing this bug actually leaves us in a pretty awkward situation. OSGI itself and many non-UI plugins will refer directly to the log, because that is an optimal error handling scenario for them. On the other hand you have all those plugins which heavily utilize IStatus that can be logged (OSGI style) or passed further to be properly handled. 'Properly handled' means either taking corrective actions (see bug 264196 as an example) or/and presenting it to the user. Many non-UI bundles does not allow to intercept an exception. It is logged and then passed to other layers, which usually try to log it again (besides taking any other actions), because they have no possibility to check if it had been done already. I believe that we have only two options right now to improve general error handling. 1. Expose logging innards to allow for better integration of external error handling frameworks. 2. Incorporate IStatus together with some sort of StatusHandling into main OSGI plugin. I have reopened this bug only for your attention - if I have not convinced you, feel free to close it. But if you have an idea how to write proper error handler which will server well UI and non-UI plugins, let me know ;-) Best regards, Chris
I'm fine with leaving this open, but I disagree with the solution of enhancing FrameworkLog. We need to consolidate into one and our current thinking is that that should be the LogService. CC'ing Simon for his awareness. Christopher, I recommend you work with the team to advocate this be added to the plan for next release. Otherwise it will likely not get the necessary focus.
(In reply to comment #4) > We need to consolidate into one and our current thinking is that > that should be the LogService. Doesn't this involve bringing the LogService classes into the org.eclipse.osgi bundle or do you have something different in mind?
(In reply to comment #5) > (In reply to comment #4) > > We need to consolidate into one and our current thinking is that > > that should be the LogService. > > Doesn't this involve bringing the LogService classes into the org.eclipse.osgi > bundle or do you have something different in mind? That is exactly what it would entail. From my point of view OSGi should probably have made the LogService a core service to begin with. There has always been a kind of boot strapping issue with the LogService when it is provided by a bundle on top because there could always be some other lower level bundle that needs to log something before a log service is available. So my proposal is to bring the extended log service from equinox (org.eclipse.equinox.log) into the core framework (org.eclipse.osgi) and unify our logging solution on top of that.
Created attachment 182916 [details] Move ExtendedLogService to core framwork (work in progress) Note this is work in progress and is not a complete solution yet. This patch copies the ExtendedLogService implementation from org.eclipse.equinox.log to org.eclipse.osgi. We will have to consider what to do with org.eclipse.equinox.log after this move. I don't want to maintain two copies of this code. At a minimum, in org.eclipse.equinox.log bundle, we should detect an ExtendedLogService registration from the system bundle and avoid registering another log service. But then I would like to freeze that bundle, deprecate it and eventually stop building it in new releases of equinox. The FrameworkLog implementation now front ends the ExtendedLogService using a logger name of "org.eclipse.equinox.logger". I ran into an issue of children FrameworkLogEntrys. The (Extended)LogEntry as no concept of children entries like FrameworkLogEntry does (or its companion IStatus). It would be awkward to add the child LogEntry concept to the (Extended)LogService. LogEntry objects are not constructed by logging clients. Instead they provide data to the LogService which is used to construct a LogEntry object. I don't think it is a good idea to try and enhance ExtendedLogService to allow us to somehow provide data for children LogEntries (or grandchildren etc.). It would end up a big mess. I worked around this in the FrameworkLog implementation by using the context object for the ExtendedLogEntry. I simply use the provided FrameworkLogEntry object as the context of the ExtendedLogEntry. The the EclipseLogWriter, which persists the log data to the .log file in a very specific format, provides the backend in the form of a LogListener. It treats the logger named "org.eclipse.equinox.logger" special and will look for a context of type FrameworkLogEntry and will simply log that to the log file like it used to. If the context object is anything other than a FrameworkLogEntry then the EclipseLogWriter will format the LogEntry into a form that is acceptable for the eclipse log (basically converting the data to a FrameworkLogEntry and logging it as normal). The EclipseLogWriter is also a LogFilter. It currently pays attention to the "org.eclipse.equinox.logger" logger name and the "eclipse.log.level" to determine what LogEntrys it will log. Currently it also pays attention to any LogEntry of type ERROR no matter what the logger name is. This likely will need to be configurable. I was left with a strange FrameworkLog service registration for the performance log. I am not sure how this is used or if this is even used anymore. I ended up front ending the ExtendedLogService with another FrameworkLog implementation for which uses a logger name of "org.eclipse.performance.logger" and another EclipseLogWriter that only writes to the performance log for "org.eclipse.performance.logger" logger names. This also adds complication for the "org.eclipse.equinox.logger" logger filter since it must ignore all "org.eclipse.performance.logger" loggers. I will have to double check that such a logger is still required. It definitely tests the ability to have multiple loggers and listeners that pay attention to different loggers. Much more testing is needed, but this definitely allows anyone to register a LogListener to listen to all log entries. The next steps to this solution involve plugging in the ILogListeners to try and receive events related to IStatus. In order to do this we may need to add a context concept to FrameworkLogEntry to allow upper layers to pass IStatus objects down to the listeners which then can pass the information back up to the ILogListeners. Another thing I would like to consider is allowing LogListeners to be registered as services instead of having to call LogReaderService add/removeLogListener ... methods. We likely would have to allow such registrations to also optionally implement LogFilter so that we can associate the LogFilter with the LogListener registration.
This patch also neglects section 101.6.4 of the compendium spec for LogService. This section outlines the Events that must be fired to the EventAdmin service for all log entries. Really wish we did not have to do this, it only adds more overhead to logging. It poses an issue for when we move the LogService to core since the core framework cannot easily fire events to event admin (would need many reflection tricks to do so).
Created attachment 183155 [details] continued work in progress This patch is work in progress still. It contains the end to end changes to Support ILogListeners and ILogs. One can now add a LogListener directly at the ExtendedLogReaderService level and get all log events for all bundles or for a specific bundle.
Created attachment 185356 [details] updated patch for HEAD
Simon I would appreciate a review of the code. Thanks.
+1 I think this is a high quality patch and I've had no problems running with it in my test environment. The only concern I have is around the use of synchronous listeners in common and core.runtime and even then I can buy that this is fine for now and we can maybe consider what's involved in going asynch later.
(In reply to comment #12) > +1 > I think this is a high quality patch and I've had no problems running with it > in my test environment. The only concern I have is around the use of > synchronous listeners in common and core.runtime and even then I can buy that > this is fine for now and we can maybe consider what's involved in going asynch > later. For the record, I went with async listeners in my initial implementation but unfortunately there are assumptions made in other parts of the system that the persistent eclipse log is written synchronously with each log entry before returning from a log method. I did not want to break this assumption for now. I agree we can look at changing this to async later. The final step required before releasing this is to write new logging testcases. Unfortunately the existing tests for all of the eclipse logging stuff is pretty minimal.
Created attachment 186141 [details] Updated patch and tests here is an updated patch - updated copyrights - small fix to use ExtendedLog service names for service look up - Conversion from LogEntry to IStatus when the LogEntry context does not provide an IStatus The tests demonstrate the three levels of listeners - ILog.addLogListener - Platform.addLogListener - LogService.addLogListener And demonstrate the different levels of logging - ILog.log - RuntimeLog.log - FrameworkLog.log - LogService.log
Created attachment 186308 [details] Final patch and tests I hope the final patch and tests. This patch adds back the event admin adapter which is required by the spec. I had to use reflection to do this. I also modified it a bit to be more lazy WRT getting the EventAdmin service so that we don't agressively cause the EventAdmin DS component to get created when no EventHandlers are around. I plan to release this for the weekend builds to get some testpasses before next weeks I-Build.
I released the latest patch with some minor tweaks for API tools and to do proper imports for the supplement bundle.
Changed title to reflect the solution that has been released.
*** Bug 286927 has been marked as a duplicate of this bug. ***
*** Bug 332152 has been marked as a duplicate of this bug. ***
FYI: This seems to be an incompatible change between 3.6.0 and 3.7.1. I am upgrading Netbinox and the removal of following lines from Framework.publishFrameworkEventPrivileged: // if the event is an error then it should be logged if (event.getType() == FrameworkEvent.ERROR) { FrameworkLog frameworkLog = adaptor.getFrameworkLog(); if (frameworkLog != null) frameworkLog.log(event); } breaks my hook which registered its own FrameworkLog. Its log method http://hg.netbeans.org/ergonomics/file/76c92828d69f/netbinox/src/org/netbeans/modules/netbinox/NetbinoxHooks.java#l263 is no longer called.
