Repository navigation
Set default log level to WARNING in Equinox OSGi Logger - #1377
Conversation
|
@tjwatson could you please have a look? |
So probably we should use |
|
I too wondered about the info status because I do see info status logged for "normal logging"... |
2b9d45f to
f5ddd04
Compare
Ok, lets do that. It also fixes one of the two test failures without further changes. In the meantime I've also adapted the only remaining failing test, so the new build should be successful. @tjwatson could you please have a look at this? |
|
I think this change is going to remove details out of the eclipse log that we are use to seeing. I think the real problem here is the Equinox slf4j bundle and its use of the Equiniox logger named org.eclipse.core.internal.runtime.PlatformLogWriter.EQUINOX_LOGGER_NAME which has always had all levels enabled for persistent to the eclipse log by default. It is unclear to me why that line is not using the logger name passed in by the slf4j user. |
tjwatson
left a comment
There was a problem hiding this comment.
I don't think the right fix is to default the equinox logger named org.eclipse.equinox.logger to INFO as I think that may have unexpected behavior from before for eclipse plugins using the Eclipse Log class. Instead I think the Equinox slf4j bundle should use the logger names passed from the slf4j APIs so they can be configured independently of the org.eclipse.equinox.logger logger.
| protected final String name; | ||
|
|
||
| private LogLevel enabledLevel = LogLevel.TRACE; | ||
| private LogLevel enabledLevel = LogLevel.INFO; |
There was a problem hiding this comment.
I don't think this does anything useful as the default because this ends up getting set by the LoggerContext settings which, if not set at all then it defaults to WARN from
|
Note that the Eclipse logger writer currently only persists ERROR log entries from other logger names. See So if slf4j does use its own names then only errors will be persisted in the Eclipse log file no matter what, unless we change this. |
To clarify, the purpose of the So we just got into the situation, because previously spifly was active only when started (what usually is not the case by default) and our implementation is more lenient regarding lazy bundles (see osgi/osgi#973). So in general one would say everything works as expsected here anyways. The real culprit is that neither PDE nor P2 currently has a strategy to ask the user if there are multiple possible choices for a capability and then choose just one randomly, while e.g. a user could actually already influence this by explicitly choosing e.g. What I can think about to possibly improve the situation is that |
|
It is strange that the But now we need a threshold level configuration for equinox.slf4j? What should happen if that conflicts with the For the current release we likely will need to do what you suggest to get something done for the short-term. But I do think there is a better way we can get slf4j to map to the osgi logger and if there is already one that does map to the osgi logger then perhaps we just need to enhance the equinox log writer to properly listen to other logger names when configured. |
As said this is meant really as a bridge between Maybe there are other ways but at least this is consistent with how log messages appear when using logging from an usual eclipse plugin.
I would not say we strictly need it, as mentioned elsewhere, a user/product should simply choose what fits best, if
Just to clarify, as of today the user has no way to silence eg. jdt verus pde or egit or ... It just has shown that libraries using slf4j make much more use of TRACE messages than usual |
f5ddd04 to
3ae34b4
Compare
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
|
As discussed (also in private conversations) I've changed this PR to just introduce a This is similar to
|
Using a default default log level of 'TRACE' (or comparable) by default leads to very verbose logging in cases where the logger is not configured further. Other logging back-ends usually use higher log-levels by default too. This is for example similar to slf4j-simple, who uses the level INFO as default value for the default log-level.
9e01d81 to
89463c2
Compare
laeubi
left a comment
There was a problem hiding this comment.
Looks all fine to me and only changes the slf4j part so it seems safe.
Using a default default log level of
TRACE(or comparable) by default leads to very verbose logging in cases where the logger is not configured further. Other logging backends usually use higher log-levels by default too.For example slf4j.simple uses
INFOby default:https://github.com/qos-ch/slf4j/blob/69d28be9fba7a9c0072dfbac8cbee8e40bb965d5/slf4j-simple/src/main/java/org/slf4j/simple/SimpleLogger.java#L188-L189
This surfaced recently since
equinox.slf4jrecently became activated automatically and seems to be the preferred choice of the Equinox Resolver used for PDE target platforms, if no SLF4J logging backend is added to a runtime:Using a higher log-level by default would also make it more convenient in the cases a developer is happy with any backend and doesn't want to configure it further.
Fixes eclipse-platform/eclipse.platform#3022