Closed ffays closed 1 year ago
mattrpav
commented
1 year ago This is a -1 for me -- logging in an application server is a fundamental capability that the application server must be able make opinionated decisions to service deployments. pax-logging allows the deployment of virtually any java application and dependency, since it supports all the relevant logger API's and impls.
I've yet to run across a use case where pax-logging's approach is problematic.
grgrzybek
commented
1 year ago Thanks @mattrpav for your view.
I agree. Adding to the main discussion at #518, I'll emphasize again - from pure OSGi point of view, you should deal with packages and you may get into a trap of thinking that a bundle is as equally important as a package. The practice shows that you have to give caesar what belongs to caesar and:
The point is that you can't avoid mixing packages in single bundle - both because there are such bundle around (have you look at the bundles that are supposed to provide javax.* packages? That's a mess!) and because you want some form of control of what you actually deploy.
Let's imagine for a while that we follow your (theoretically sane suggestion in the spirit of pure OSGi) advice and have pax-logging-api export only org.ops4j.pax.logging.* package. Then you'd have to deploy (assuming they are correct and not competing) twenty six other bundles:
Again, I'm not sure if you've noticed, but OSGi spirit tells you that you should craft your bundle without taking much care about the bundle version, but with strict control over your package versions.
So the OSGi way is to have:
com.example/my-service bundle (available from Maven for example) at version 1.0 with packages like:
com.example.api 1.0com.example.impl 1.0com.example/my-service bundle (available from Maven for example) at version 2.4 with packages like:
com.example.api 1.1com.example.impl 1.1The above example shows that the packages are fully compatible from consumer point of view even if the Maven version is completely different.
That's how for example these libraries are created:
See - major version upgrade, but some packages have only micro version difference:
org.osgi.util.tracker;version="1.5.2"org.osgi.util.tracker;version="1.5.3"On the other hand, the most popular (IMO) way to create OSGi bundles is org.apache.felix/mave-bundle-plugin which analyzes your jar and exports all (customizable) eligible packages with version coming from Maven version of the project. This is where you end up with tens of Guava libraries, exporting the same packages with new major version, but which are in fact compatible.
That's why clever people before me (I'm only a janitor of Pax Logging project...) decided to do what they did. I've accepted, appreciated and understood the pragmatic approach - you do what has to be done.
So no way - your suggestion would work in ideal world, where:
META-INF/MANIFEST.MF consciously without the defaultsBut fortunately (yes) we don't live in such ideal world ;)
ffays
commented
1 year ago thank you for enlightening me regarding the downside to have separate all implementations kept in separate bundles.
I still see a major downside of such approach: Let's imagine you have new Common Vulnerabilities and Exposures for one or several packages that are not part of this project and that are bundled in pax-logging-api. If the Single Responsibility principle is respected, then the maintainer just need to replace the bundle that is vulnerable with a new version with the fix. Whereas hereby, the maintainer will have to open an issue on pax-logging-api project to request a port!
Let's imagine for a while that we follow your (theoretically sane suggestion in the spirit of pure OSGi) advice and have pax-logging-api export only org.ops4j.pax.logging.* package. Then you'd have to deploy (assuming they are correct and not competing) twenty six other bundles:
To my opinion, not really 26 other bundles, all package imports can be made optional, this lets the decision to the packager which logger implementation it needs in the application, as in fact in real world no decent applications uses 8 different logger flavours at the same time...
grgrzybek
commented
1 year ago thank you for enlightening me regarding the downside to have separate all implementations kept in separate bundles.
I still see a major downside of such approach: Let's imagine you have new Common Vulnerabilities and Exposures for one or several packages that are not part of this project and that are bundled in pax-logging-api. If the Single Responsibility principle is respected, then the maintainer just need to replace the bundle that is vulnerable with a new version with the fix. Whereas hereby, the maintainer will have to open an issue on pax-logging-api project to request a port!
True - this was the case during Log4J apocalypse - I was releasing 5 Pax Logging versions a day for several days.
But I wouldn't switch to other model anyway ;) Similar (but much simpler) model is with your WAR files deployed to, say, Tomcat (or other app server) - how many not-yours libraries do you have in WEB-INF/lib? How many of them do you put to "common classloader" (in Tomcat), so your WAR is thin? Sometimes it's better to embed it, so you have more control.
But again - while for example, commons-logging uses /META-INF/services/org.apache.commons.logging.LogFactory service simply ships own org.apache.commons.logging.LogFactory implementation delegating to org.ops4j.pax.logging.PaxLoggingManager.
It's even more visible with Log4j1 API - if you switch from putting org.apache.log4j package inside pax-logging-api to separate log4j bundle, you'd load org.apache.log4j.Category from log4j1 and you'd end up with original org.apache.log4j.LogManager running its own static block to discover different things. We have to replace this behavior with one that allows you to redirect Log4j1 API calls to Logback backend! So we have to replace this class/package instead of asking Log4j1 maintainers (...) to accept our PR that does the OSGi magic.
Let's imagine for a while that we follow your (theoretically sane suggestion in the spirit of pure OSGi) advice and have pax-logging-api export only org.ops4j.pax.logging.* package. Then you'd have to deploy (assuming they are correct and not competing) twenty six other bundles:
To my opinion, not really 26 other bundles, all package imports can be made optional, this lets the decision to the packager which logger implementation it needs in the application, as in fact in real world no decent applications uses 8 different logger flavours at the same time...
True, but as I said - we not only want to give pax-logging-api access to some external packages - we have to replace some classes with ones that delegate to OSGi machinery (wiring, trackers, ...). If we simply use original classes from, say, JBoss Logging, we'd delegate to its behavior, which is mostly classpath scanning in search for available Logging implementations - but hey, there's no CLASSPATH in OSGi ;)
And also, for example you deploy two bundles that have (because of the default behavior of maven-bundle-plugin):
Import-Package: org.slf4j; version="[1.7,2.0)"Import-Package: org.slf4j; version="[2.0,3.0)"Actually from API point of view, there's no change between SLF4J 1 and 2. So pax-logging-api exports both versions, while with pure OSGi approach you'd have to install two slf4j-api bundles. But 1.7 would try to load org.slf4j.impl.StaticLoggerBinder from the CLASSPATH and 2.0 would search for /META-INF/services/org.slf4j.spi.SLF4JServiceProvider - mind that java.util.ServiceLoader in OSGi works reliably only with some bytecode TCCL magic done by Aries SPI-Fly...
Analysing the
Export-Packagedirective from theMANIFEST.MFofpax-logging-api-2.2.2.jarit appears that the Jar is bundled with many "foreign" classes that does not belong to org.ops4j.pax.logging projectI believe this is a bad practice, as it break the Single Responsibility principle, i.e. every class, module, or function in a program should have one responsibility/purpose in a program.
And hereby the library is taking over the responsibility of many other libraries.
Hereby below the list of "foreign" packages containing "foreign" classes: