google-code-export / uimafit

Automatically exported from code.google.com/p/uimafit
2 stars 1 forks source link

Allow injection of logger #80

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Add annotation to initialize the logger from the UimaContext.

@Logger
Logger log;

Original issue reported on code.google.com by richard.eckart on 6 Apr 2011 at 9:16

GoogleCodeExporter commented 9 years ago
So the type is for org.apache.uima.util.Logger (or something close to that.)  
It seems like if we know it's a uima logger, then we know we can intialize it 
with the uima context.  Is there a scenario in which you would not want uimaFIT 
initializing a logger for you?  Maybe all we need is to add some code that 
actually does that.  

Original comment by phi...@ogren.info on 7 Apr 2011 at 3:43

GoogleCodeExporter commented 9 years ago
For injection of other loggers, there are other libraries/frameworks providing 
for injection. In any case, I constantly advice people to stick only to the 
logger provided by UIMA when writing UIMA components and not to use any 
additional framework. Instead when they run a pipeline, the UimaContext should 
be configured with an adapter for the particular logging framework in use by 
the application. This allows the UIMA component to log properly in any context 
it is being deployed in, no matter what actual logging framework is used.

Original comment by richard.eckart on 7 Apr 2011 at 5:49

GoogleCodeExporter commented 9 years ago
So, in your code example the type of log could be any logger (i.e. presumably 
log4j or java logging)?  

Original comment by phi...@ogren.info on 10 Apr 2011 at 4:01

GoogleCodeExporter commented 9 years ago
I'm not sure how I interpreted comment 1 that it prompted me to write comment 
2. Let's try again:

The type in my example is always a UIMA logger. Yes, we can initialize that 
from the UIMA context. I personally cannot imagine a case where I would not 
want uimaFIT to initialize the logger.

Original comment by richard.eckart on 10 Apr 2011 at 4:37

GoogleCodeExporter commented 9 years ago
Ok.  So my implied question is why do we need the annotation at all?  It seems 
like if you have a logger, then just initialize it using the uimaContext.  I 
suppose the annotation makes it more explicit that you want uimaFIT to 
initialize the logger (so there are no surprises.)  

Original comment by phi...@ogren.info on 10 Apr 2011 at 7:48

GoogleCodeExporter commented 9 years ago
Without the annotation, it'd be too much magic for me.

As an alternative I had thought about adding a getLog() method to the uimaFIT 
component base classes, but ended up liking an annotation more.

Original comment by richard.eckart on 10 Apr 2011 at 7:55

GoogleCodeExporter commented 9 years ago

Original comment by richard.eckart on 2 May 2011 at 5:06

GoogleCodeExporter commented 9 years ago
I am still confused by this issue.  If you could spell this out a little more 
clearly for me when you get closer to implementing it I would be interested to 
understand it better.  

Original comment by phi...@ogren.info on 5 May 2011 at 3:03

GoogleCodeExporter commented 9 years ago
When I want to log something in an UIMA component, I have two possibilities:

1) getContext().getLogger().log(Level.INFO, "my message");
2) I declare a member variable of type Logger and set that in the initialize() 
method after the context is set

Want I want to do is to declare a member variable annotated with @Log and not 
have to worry about setting it up in initialize():

@Log
Logger log;

Original comment by richard.eckart on 2 Jan 2012 at 10:51

GoogleCodeExporter commented 9 years ago
I don't mean to belabor something that I don't care that much about.  But why 
do you like the annotation better than the getLog() method?  This seems a 
perfectly natural way to get the logger.  I am also forgetting how UIMA 
provides loggers - can there be many different loggers sending their messages 
to different handlers.  The code in 1) above seems to suggest there is just one 
logger.  Is that the UIMA way?  If so, then it seems that a getLog() method 
suffices.  If not, then it seems the proposed @Log annotation should allow you 
to obtain granular loggers.  I may be confused here....

One other question:  would an @Log annotation open the possibility of having 
code mistakes that don't generate a compiler error?  For example, could you 
annotate a member variable that wasn't a Logger or could you annotate more than 
one member variable (though this might not hurt anything just confuse the 
code.)  

Original comment by phi...@ogren.info on 3 Jan 2012 at 4:13

GoogleCodeExporter commented 9 years ago
Your comments are always welcome and you make a couple of good points there. I 
personally don't like the long way that there currently is:

0) getContext().getLogger().log(Level.ERROR, "my message", e);

So I was thinking about several ways that I would write the same code more 
conveniently:

1) log.log(Level.ERROR, "my message", e);
2) getLogger().log(Level.ERROR, "my message",e );
3) error("my message", e)
4) getLogger().error("my message", e);

The first approach would require either a protected variable in the uimaFIT 
component base classes or a annotated variable (@Log) in the component class, 
both of which would be set in initialize(). 

The second approach would require all uimaFIT base component classes to 
implement the getLogger() method.

The third approach would require all uimaFIT base component classes to 
implement a set of logging methods, one for each level. This gives the shortest 
code. This follows the approach taken in the Apache Wicket framework for 
logging in web pages.

The fourth approach would basically mirror the third but the logging methods 
would be in a separate uimaFIT logging class.

While I find the third or fourth approach the most convenient, I consider the 
first or second approach more reasonable because they do not introduce a new 
logging API, just make the access to the existing API more convenient. However, 
if somebody else would also be in favor of 3 or 4, I also wouldn't mind 
implementing them. 4 could be implemented in such a way that the object 
returned by getLogger() implements the UIMA Logger interface as well as the 
convenience methods (error, info, etc.).

Original comment by richard.eckart on 4 Jan 2012 at 2:36

GoogleCodeExporter commented 9 years ago
Of Option 1 and 2, I think Option 2 looks more natural since it just gives 
components a getLogger() method like contexts already have.

Option 3 looks nice to me, but seems hard to do without a bunch of duplicate 
code in each of the uimaFIT base classes. (Damn, where are Scala traits when 
you need them!)

Option 4 is basically the same as Option 2, but you return a subclass of UIMA 
Logger with a simpler API. This sounds nice to me.

Original comment by steven.b...@gmail.com on 4 Jan 2012 at 3:53

GoogleCodeExporter commented 9 years ago
Interestingly the Collection Readers already have a getLogger() method which is 
originally defined in UIMA's Resource_ImplBase, so I wonder if it would break 
binary compatibility to uimaFIT 1.2.0 if I introduce I overwrite this with a 
getLogger() version returning a new ExtendedLogger class. According to a Stack 
Overflow Post [1] where somebody had a similar problem I understand that it 
should be a binary compatible change as the Logger getLogger() signature is 
still present in the virtual method table.

[1] 
http://stackoverflow.com/questions/3499981/java-binary-compatibility-rfc-on-prop
osed-solution-to-covariant-return-type-us

Original comment by richard.eckart on 4 Jan 2012 at 8:23

GoogleCodeExporter commented 9 years ago
Added getLogger() method to all component and resource base classes which wraps 
the context logger in an ExtendedLogger providing convenient Log4J-style 
logging methods.
---
Committed revision 664.

Original comment by richard.eckart on 4 Jan 2012 at 10:43

GoogleCodeExporter commented 9 years ago

Original comment by richard.eckart on 4 Jan 2012 at 10:52