You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "N1k145 (via GitHub)" <gi...@apache.org> on 2023/04/13 12:58:52 UTC
[PR] #1367 Added missing activation policy (logging-log4j2)
N1k145 opened a new pull request, #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429
Added the missing activation policy for #1367, this is required to start the bundle and get the bundle context in an eclipse OSGI environment.
## Checklist
* Base your changes on `2.x` branch if you are targeting Log4j 2; use `main` otherwise
* `./mvnw verify` succeeds (if it fails due to code formatting issues reported by Spotless, simply run `./mvnw spotless:apply` and retry)
* Non-trivial changes contain an entry file in the `src/changelog/.2.x.x` directory
* Tests for the changes are provided
* [Commits are signed](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) (optional, but highly recommended)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "HannesWell (via GitHub)" <gi...@apache.org>.
HannesWell commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1627755948
Just tested the recent snapshots that include the fix (I had less time than expected to test this before it was merged) and everything looks good to me as well.
Tested it with an application that uses log4j-2-core as back-end and the log2j-2.api as well as SLF4J and the bridge `log4j-slf4j2-impl` and in another one that uses the log4j-2.api and logback as backend and `log4j.to-slf4j` to redirect log2j to slf4j.
Only for the former application I had to keep the explicit activation for `org.apache.logging.log4j.core`, because otherwise nothing called the log4.core Activator and the necessary registrations of the provider are never done.
Maybe it would be worth to think about an approach that only relies on lazy activation so that one would not need to start any bundle explicitly, in another issue/PR. But this change already simplifies the setup.
Thanks @N1k145 and @ppkarwasz for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "N1k145 (via GitHub)" <gi...@apache.org>.
N1k145 commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1623088161
@HannesWell this is the issue we talked about, where the activation policies are missing for the automatic bundle start
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "N1k145 (via GitHub)" <gi...@apache.org>.
N1k145 commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1624011979
> What happens if someone puts `log4j-core` in start level 1 and `log4j-api` in start level 2? Will this solution work anyway or the bundles must be in the same start level?
In theory you do not need to modify the start levels.
You already have the implementation where the `api` searches for the `impl` OSGi Service. Which throws an exception in an eclipse environment as described in the linked issue.
Because Equinox, in contrast to Felix, does not activate every bundle by default.
With this change Equinox should activate the bundle when it is accessed and the exception should no longer be thrown as the bundle context is there and the `impl` service should already be registered as it is indexed in the startup. So the service can be found using the OSGi Service lookup.
So no need for any manual startlevel or autostart configuration and OSGi just has to do it's magic.
But feel free to test it, I only tested it in our quite large setup, maybe it behaves differently in other cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "HannesWell (via GitHub)" <gi...@apache.org>.
HannesWell commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1623111659
> @HannesWell this is the issue we talked about, where the activation policies are missing for the automatic bundle start
Thanks for the link.
This change looks good to me. This really simplifies using log4j-2 in OSGi/Eclipse-based applications. 👍🏽
Additionally the activation policy should also be added to the log4j-to-slf4j bundle:
https://github.com/apache/logging-log4j2/blob/83bba1bc322e80e7e95edbebc2383f2724dbe0de/log4j-to-slf4j/pom.xml#L98-L102
@ppkarwasz could you please have a look at this one?
@N1k145 what might be missing is a change-log entry. I think if you create one like in https://github.com/apache/logging-log4j2/pull/1375, this should speed-up the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "N1k145 (via GitHub)" <gi...@apache.org>.
N1k145 commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1623186920
@HannesWell thanks for the hint, I added the changelog entry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1623844345
@HannesWell and @N1k145,
I don't work with OSGI on a daily basis, so I need time to setup a testing environment. Since Hannes confirms that this solution works, I guess I could skip this step.
What happens if someone puts `log4j-core` in start level 1 and `log4j-api` in start level 2? Will this solution work anyway or the bundles must be in the same start level?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz merged PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "rgoers (via GitHub)" <gi...@apache.org>.
rgoers commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1623923793
We do have OSGi tests in the log4j-osgi module. Would it be possible to add a test there?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "HannesWell (via GitHub)" <gi...@apache.org>.
HannesWell commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1624092744
> @HannesWell and @N1k145,
>
> I don't work with OSGI on a daily basis, so I need time to setup a testing environment. Since Hannes confirms that this solution works, I guess I could skip this step.
I have not yet tried out this explicit change, but I remember from that the past that I tried something similar (if not the same) already for the same reason.
But I'll try this out with our projects tomorrow.
>
> What happens if someone puts `log4j-core` in start level 1 and `log4j-api` in start level 2? Will this solution work anyway or the bundles must be in the same start level?
As Niklas said, this should not cause harm. If you are interested in the details, chapter [4.4.6.2 `Lazy Activation Policy` of the OSGi specification](https://docs.osgi.org/specification/osgi.core/8.0.0/framework.lifecycle.html#i3270445) that covers it. If you want a little bit more context the entire [4.4.6 `Activation` chapter](https://docs.osgi.org/specification/osgi.core/8.0.0/framework.lifecycle.html#i3270394) probably makes it a bit more clear.
But to summaries it, this header mainly commands the running OSGi framework to delay the activation of a bundle (i.e. calling its `BundleActivator's start()` method to the time immediately before when the first class from it is loaded, after it has been started.
Additionally as far as I know, Equinox has the specialty (I don't know how other implementations like Felix behave in this regard) to put bundles that have the `Bundle-ActivationPolicy: lazy` declared into the STARTING state automatically, which has the effect that they are started as soon as the first class is loaded from them, without the need to start them explicitly. This also has the advantage that lazy activated bundles have a BundleContext, even if they not have been started (as Niklas also pointed out). This is for example stated in [Bundle.getBundleContext()](https://docs.osgi.org/specification/osgi.core/8.0.0/framework.api.html#org.osgi.framework.Bundle.getBundleContext--).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "HannesWell (via GitHub)" <gi...@apache.org>.
HannesWell commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1632015508
@ppkarwasz or @rgoers can you estimate when the next release for the 2-major will happen?
Thanks in advance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #1367 Added missing activation policy (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #1429:
URL: https://github.com/apache/logging-log4j2/pull/1429#issuecomment-1625204675
After some tests it appears that as long as `log4j-core` is in a start level lower or equal to the bundle that requires logging, everything works fine. I assume that `log4j-core` and `log4j-api` should be in one of the lowest start levels.
Moreover:
- on Equinox the `log4j-api` bundle starts always **before** `log4j-core`. All calls to `OsgiServiceLocator` succeed (even if the first is triggered by static initialization of the activator class),
- on Felix nothing seems to change: the order of activation depends on the order of declaration of the bundles.
Anyway the PR looks good to me, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org