Hello
pt., 19 kwi 2019 o 04:19 Niclas Hedhman <[email protected]> napisał(a):
> For your last point; OSGIPaxLoggingManager becoming a singleton... The
> amount of memory or other overhead you save with that is minimal, and I
> recommend against the proposed change. In general, we don't try to maximize
> the utilization of trackers in OSGi, but let each client handle its own
> tracker to keep responsibilities near the usages. IF you decide to do it
> anyway, you will still need to keep the public constructor as it is part of
> the official API and don't want to break compatibility over such a minor
> thing.
>
Thanks for technical suggestions. I already started checking the
"singleton" version of OSGIPaxLoggingManager. I wasn't that much concerned
about number of trackers created, but with general code organization.
There are six _clients_ but OSGIPaxLoggingManager was not created once for
every client - for example with SLF4J, the manager was created both inside
Slf4jLoggerFactory and in Slf4jMDCAdapter. That's one thing. Another was
that most (not all) of these "manager keepers" were holding Map<String,
ClientSpecificLogger> m_loggers - only to ensure that loggers created
between static client/factory was created and OSGIPaxLoggingManager was set.
For now, I extracted "setPaxLoggingManager" method to
PaxLoggingManagerAwareLogger interface and I can keep single Map<String,
PaxLoggingManagerAwareLogger> m_loggers map in the Activator itself (I can
think of moving it somewhere else) - so far, all _clients_ contains the
same code related to m_loggers.
So less optimization (because it's not a problem here), but code cleanup,
comments and consistency.
In addition, I was able (so far) to:
- keep only those sources from slf4j/commons-logging/... that are needed,
other classes from original _clients_ are simply private-packaged (and
exported)
- enforce a convention that _client_ specific overrides are in their
original package and supporting classes (like
org.ops4j.pax.logging.slf4j.Slf4jLoggerFactory) are kept in
org.ops4j.pax.logging._clientid_. IMO these packages should not be exported
(I have to be delicate here) - org.ops4j.pax.logging.slf4j is exported but
org.ops4j.pax.logging.log4jv2 not.
- additionally, I added pax-logging-api-test project which has
maven-invoker based tests to show different API usages and ensure that
pax-logging-api adheres to these idioms. For example (no pax-logging-api
usage, just awareness of the implementation):
package org.ops4j.pax.logging.test.slf4j;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;
import static org.junit.Assert.assertTrue;
public class FactoryTest {
@Test
public void paxLoggingSpecificSlf4jFactory() {
Logger log = LoggerFactory.getLogger(this.getClass());
log.info("Factory: {}", LoggerFactory.getILoggerFactory());
assertTrue(LoggerFactory.getILoggerFactory().getClass().getName().startsWith("
*org.ops4j.pax.logging.slf4j*"));
}
@Test
public void paxLoggingSpecificMDC() {
assertTrue(MDC.getMDCAdapter().getClass().getName().startsWith("
*org.ops4j.pax.logging.slf4j*"));
}
}
pax-logging didn't have maven-bundle-plugin:baseline verification (like
pax-web) and I'm not (yet) sure if adding new interface (and not touching
previous exported packages) requires bumping version to 2.0 (I don't think
so). There's no pax-logging 1.11.0 yet, so minor version upgrade allows
_some_ API changes.
I'll keep you informed.
Also there's this PAXLOGGING-240 ("ClassCastException with log4j2 on
org.apache.logging.log4j.core.LoggerContext") issue which _may_ require
some API reorganization.
I just have some time now and want to do some maintenance work for pax
project (I'm working on https://ops4j1.jira.com/browse/PAXWEB-1190 as well).
regards
Grzegorz Grzybek
>
> Cheers
> Niclas
>
> On Thu, Apr 18, 2019 at 1:54 PM Grzegorz Grzybek <[email protected]>
> wrote:
>
>> Hello
>>
>> I have few issues backlogged for PAXLOGGING Jira:
>> – PAXLOGGING-243: Incorrect bundle names in the logs in case of the logs
>> come from embeded lib
>> – PAXLOGGING-247: pax-logging-api/Slf4jMDCAdapter uses stale MDC after
>> refreshing service bundle
>> – PAXLOGGING-249: JUL loggers are not properly configured if used before
>> calling PaxLoggingServiceImpl#setJULLevel
>> – PAXLOGGING-250: Log4j2 - JNDI based JDBC appender should be more lazy
>>
>> So I started to review the code (licenses, deps, upgrading from old
>> org.ops4j:master:3.3.0, ...).
>>
>> I see some code could be refreshed and improved. Here are my concerns
>> which I'll try to handle if I won't get any feedback):
>> – should we switch compiler settings to Java8? Not necessary
>> – I want to upgrade log4j2, slf4j, logback and jboss logging deps
>> – I want to get rid of those source files from external libs which can
>> be simply private packaged
>> – I want to switch org.ops4j.pax.logging.OSGIPaxLoggingManager usage to
>> singleton - now, each logging facade uses own copy of this manager, which
>> is effectively a tracker for given PaxLoggingService.
>>
>> What do you think?
>>
>> regards
>> Grzegorz Grzybek
>>
>> --
>> --
>> ------------------
>> OPS4J - http://www.ops4j.org - [email protected]
>>
>> ---
>> You received this message because you are subscribed to the Google Groups
>> "OPS4J" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to [email protected].
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>
> --
> Niclas Hedhman, Software Developer
> http://polygene.apache.org - New Energy for Java
>
> --
> --
> ------------------
> OPS4J - http://www.ops4j.org - [email protected]
>
> ---
> You received this message because you are subscribed to the Google Groups
> "OPS4J" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>
--
--
------------------
OPS4J - http://www.ops4j.org - [email protected]
---
You received this message because you are subscribed to the Google Groups
"OPS4J" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
