panglaoying / log4jdbc-log4j2

Automatically exported from code.google.com/p/log4jdbc-log4j2
0 stars 0 forks source link

Enable configuration via dependency injection frameworks #15

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Currently I can only have one log4jbc configuration even though I have multiple 
datasources to spy upon simultaneously in classloader 

I'd also like to be able to configure the system myself without using 
classloader resources but the current approach would forces me to mutilate 
system properties at runtime. It's also quite error prone because I've to time 
it to happen prior log4j Properties/SpyLogDelegatorFactory classes are 
initialized.

* * *

Would it be possible convert static methods of Properties as non-static and 
pass the instance where it's needed just like it's done with SpyLogDelegator. 
Other possibility is to make use of ThreadLocal inside 
Properties/SpyLogDelegatorFactory methods by setting them up whenever entering 
to the "spy network" and tearing themt down when leaving.

StatementSpy for instance gets already the SpyLogDelegator in it's constructor 
instead of using SpyLogFactory.getSpyLogDelegator() and this approach could be 
used almost everywhere to deliver the Properties instance as well. DriverSpy 
seems to be the only place where the registration of the driver is done 
statically. DriverManager documentation gives alternative approaches that could 
be utilized so that static stuff could be moved to constructor and registration 
would be done outside of the class.

Original issue reported on code.google.com by TuomasKi...@gmail.com on 11 Feb 2014 at 9:15

GoogleCodeExporter commented 9 years ago
I understand your point, the `Properties` class was created only to clean 
existing log4jdbc code, to separate the property initialization from the 
`DriverSpy` registration. But as all the initialization was originally done in 
the static initializer of `DriverSpy`, it remained in the "static" way.

But could you please explain what you intend to do more precisely? Because 
managing the properties as you suggest would require a lot of modifications, 
while what you want to achieve is maybe doable with a custom SpyLogDelegator. 

Original comment by frederic...@gmail.com on 11 Feb 2014 at 5:08

GoogleCodeExporter commented 9 years ago
I'm sorry to ask for such a big change but I'm using OSGi environment where I 
am basically handed a Properties instance instead of a reference to it. 
Currently would  have to hack-up a classloader for log4jdbc in order to capture 
Properties.class.getResourceAsStream("/log4jdbc.log4j2.properties") call. And 
multiple dataSources would all require their own classloader. 

The approach I'm interested in would basically be something like new 
DataSourceSpy(dataSource, new XXXSpyLogDelegator(properties), properties) where 
both spy log delegator and spy classes would be delegated with properties (and 
spy log delegator) instances. Although I can set the delegator later on to 
whatever I want, the SpyLogFactory.getSpyLogDelegator() that is called in 
constructor would cause NoClassDefFoundError when neither of the logging 
frameworks are present.

DriverSpy is the only thing that doesn't fit in to the picture nicely and it 
alone would require some backwards compatibility plumbing.

* * * 

Here's a quick overview on static references to Properties

DriverSpy (4 matches: 3 exact, 1 potential)
Log4j2SpyLogDelegator (1 match)
  shouldSqlBeLogged(String) (5 matches)
  sqlTimingOccurred(Spy, long, String, String) (5 matches)
RdbmsSpecifics (1 match)
  formatParameterObject(Object)
Slf4jSpyLogDelegator (1 match)
  getDebugInfo() (3 matches)
  processSql(String) (5 matches)
  shouldSqlBeLogged(String) (5 matches)
  sqlOccurred(Spy, String, String)
  sqlTimingOccurred(Spy, long, String, String) (5 matches)
SpyLogFactory (1 match) 
  getSpyLogDelegator() (2 matches: 1 exact, 1 potential)
SqlMessage (1 match)
  getDebugInfo() (3 matches)
  processSql(String) (5 matches)
StatementSpy (1 match)
  getGeneratedKeys(String)
  reportStatementSql(String, String)
  reportStatementSqlTiming(long, String, String)

Original comment by TuomasKi...@gmail.com on 13 Feb 2014 at 6:37

GoogleCodeExporter commented 9 years ago
Yep, but what is exactly that you want to differentiate between your DataSource 
instances using different Properties instance (so what is the property that you 
want to be different, and why)? 
Maybe the behavior you want to achieve can be obtained by another way, or by 
modifying another part of the source code, so I'd like to know more about your 
precise need :)

Original comment by frederic...@gmail.com on 13 Feb 2014 at 9:59

GoogleCodeExporter commented 9 years ago
Currently it would be the same whether or not to use the same properties 
instance. Multiple properties is just a bonus that comes if after static 
initialization blocks can somehow be prevented. 

I have to be able to at least set log4jdbc.spylogdelegator.name so that I can 
route directly to the OSGi log service without having to go via logging 
bridge(s).

I just can't set up the system without ugly glue code because when first time 
XXXSpy calls any of the Properties methods, the class will invoke 
Properties.class.getResourceAsStream(...) which in my case would fail because 
my properties would be external. I'd have to push all of the properties to 
system properties, touch Class.forName(classLoader, "...Properties", true) and 
pop properties out from system properties. This approach is dubious at best in 
an OSGi environment. Other alternative was to create a custom classloader which 
seems to me as the better alternative, but still I fear that in some cases 
Properties end up being initialized prior I have the chance to have an 
influence upon how it's done.

Original comment by TuomasKi...@gmail.com on 14 Feb 2014 at 10:05

GoogleCodeExporter commented 9 years ago
I stumbled to a hole right away. H2 has support for OSGi but it seems that even 
then org.h2.Driver.load is called upon although it shouldn't have to be. As a 
side-effect it triggers DriverManager's service loader that initializes the 
DriverSpy. This results to NoClassDefFoundError 'Unable to find Log4j2 as 
default logging library.' because I haven't yet property set up Properties at 
this point.

Original comment by TuomasKi...@gmail.com on 14 Feb 2014 at 10:52

GoogleCodeExporter commented 9 years ago
Just to be sure I understand correctly: 
- you want to use a logging system that is neither Log4j2 nor SLF4J
- you want to avoid the call to Properties.class.getResourceAsStream because of 
OSGi

Am I correct?

If I am, then my questions are: 
- can you write a custom SpyLogDelegator, making use of your logging system, 
and pass it to the DataSourceSpy constructor? Then you shouldn't have the 
NoClassDefFoundError exception.
- I am not familiar with OSGi, does calling getResourceAsStream generate an 
exception? Because if it doesn't, then it is not mandatory to use any 
configuration file. If it does, maybe there is a specific OSGi exception that I 
can catch to fix the problem? 

Original comment by frederic...@gmail.com on 17 Feb 2014 at 11:56

GoogleCodeExporter commented 9 years ago
>you want to use a logging system that is neither Log4j2 nor SLF4J
>can you write a custom SpyLogDelegator, making use of your logging system, and 
pass it to the DataSourceSpy constructor? Then you shouldn't have the 
NoClassDefFoundError exception.
I've written one already and I've circumvented the NoClassDefFound as well.

>you want to avoid the call to Properties.class.getResourceAsStream because of 
OSGi
Yes and more in general this would cover injection frameworks such as 
Spring/CDI/Guice and the rest as well.

>I am not familiar with OSGi, does calling getResourceAsStream generate an 
exception? 
On this are OSGi behaves just as "normal java" would. getResourceAsStream 
returns just a null stream as the javadoc states unless I introduce a 
/log4jdbc.log4j2.properties to the classpath or filesystem which I really can't 
do in  a proper way.

What I would like to do is to initialize the net.sf.log4jdbc.Properties myself 
(from an injected java.util.Properties instance) and hand it over to spy 
network like below (example applies to other frameworks such as 
Guice/Spring/CDI as well). 

class MyProvider implements Provider<DataSource> {
  @Inject
  private java.util.Properties properties;
  @Inject
  private DataSource dataSource;
  @Inject
  private SpyLogDelegator spyLogDelegator;

  public get() { 
    return new DataSourceSpy(this.dataSource, this.spyLogDelegator, new net.sf.log4jdbc.Properties(this.properties));
  }

}

I don't care much about DriverSpy so from my perspective it could remain as-is.

Original comment by TuomasKi...@gmail.com on 25 Feb 2014 at 9:27

GoogleCodeExporter commented 9 years ago
I understand that it would be a cleaner architecture, but why do you absolutely 
want to inject a Properties object, if you already circumvented the 
NoClassDefFound error? Besides the logger class name, the other properties are 
mostly useless IMO, and there should be no error when the property file is 
missing.

(just trying to see whether it is really a required change, I'm not trying to 
refute your points, just to be clear)

Original comment by frederic...@gmail.com on 25 Feb 2014 at 9:57

GoogleCodeExporter commented 9 years ago
1) I have now added Log4Jdbc API on classpath to prevent NoClassDefFound. I've 
also found out a way to block DriverSpy from being initialized but 
Log4j2SpyLogDelegator will be instantiated also when I create DataSourceSpy. I 
could introduce a global /log4jdbc.log4j2.properties 
2) I have a need to silence the DDL commands (CREATE etc.) on data sources are 
intended on JPA usage (hbm2ddl creates way too much noise). Then again I need 
the DQL statements and their results in order to optimize Hibernate fetch 
strategies etc. In the same classloader there are data sources aimed for pure 
JDBC access where I need to silence the result set collection due to vast 
amount of data. Therefore my current need is to have two separate sets of 
properties. 

I was hoping to preserve the log4jdbc configuration in my project for future 
needs but it seems that I have to tear it down once I'm finished with the 
profiling. 

Original comment by TuomasKi...@gmail.com on 25 Feb 2014 at 1:50

GoogleCodeExporter commented 9 years ago
OK, thanks for the explanations, issue accepted. No timetable though. 

Original comment by frederic...@gmail.com on 25 Feb 2014 at 3:10