[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
RE: [aspectj-users] System.out.prinltn and advicing

Hi

My motivation for this is two-fold. The first beeing able to get rid of application output to System.out and System.err on the server, and directing them to the application logs instead. The second is identify where the origin of the messages are, so that we may instruct the developers to fix them.

I have taken the various advice that I have got on this thread and implemented an Aspect  that does this (I have listed it below, so that anyone interested can use it). There are a couple of challenges left:

Calls to System.err and System.out from a static method (such as main) don't carry the the information about the method-name (main), so I'm left with line number in the code.

After Ron's note about stacktraces I started to try to add trapping for these too. So far I am able to catch the call to ".printStackTrace(System.out)", but it won't catch printStackTrace()

pointcut sysoutException(Object obj) :
        call(* *.printStackTrace(..)) 
        && args(obj)
        && !within(BaseAspect+);

==============
Hermod

..
import org.apache.log4j.Logger;
import java.io.PrintStream;

public aspect SysOutLoggAspect extends BaseAspect {

	pointcut sysout(PrintStream p, Object obj) :
        (call(void PrintStream.println(..)) || call(void PrintStream.print(..))) 
        && args(obj)
        && target(p)
        && if(p == System.out || p==System.err)
        && !within(BaseAspect+);

	void around(Object obj) : sysout(PrintStream , Object) && args(obj) {
		Object o = thisJoinPoint.getThis();
		if (o != null) {
			StringBuffer sb = new StringBuffer(o.getClass().getName()).append(".").append(thisJoinPointStaticPart.getSourceLocation().getLine()).append(
					": ").append(obj);
			Logger.getLogger(o.getClass().getName()).log(
					o.getClass().getName(),
					Logger.getLogger(o.getClass().getName())
							.getEffectiveLevel(), sb, null);
		} else {
			StringBuffer sb = new StringBuffer(thisJoinPointStaticPart.getSourceLocation().getWithinType().getName()).append(".").append(thisJoinPointStaticPart.getSourceLocation().getLine()).append(": ")
					.append(obj);
			Logger.getRootLogger().log(
					Logger.getRootLogger().getEffectiveLevel(), sb, null);
		}
	}

}

-----Original Message-----
From: aspectj-users-bounces@xxxxxxxxxxx [mailto:aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of Ron Bodkin
Sent: Tuesday, October 02, 2007 11:46 PM
To: aspectj-users@xxxxxxxxxxx
Subject: Re: [aspectj-users] System.out.prinltn and advicing


Understood although that raises the immediate question of how one should in general handle other uses of these fields. You might log by actually shunting to a proxy (then again you can do that with System.setOut too). In many systems calls to println are mixed with other forms of output (print and printStackTrace) Sent wirelessly via BlackBerry from T-Mobile.

-----Original Message-----
From: "Cristiano Breuel" <cristiano.breuel@xxxxxxxxx>

Date: Tue, 2 Oct 2007 18:25:54 
To:aspectj-users@xxxxxxxxxxx
Subject: Re: [aspectj-users] System.out.prinltn and advicing


Hi Ron,

If the intent is to warn of direct (and possibly policy-breaking) uses of System.out and System.err, I agree with, you, matching the field gets is safer. But I think Hermod wants to replace the calls to println with logging of the same messages, so he needs to get the strings that were passed as parameters in these calls. In this case, he needs to match the actual calls to PrintStream.println. The "if" pointcut that tests the equality of the target with the System.out. and System.err instances makes sure there will be no false positives.

Of course, one could argue whether using AspectJ to replace the calls is a good strategy, since this causes more runtime overhead and doesn't actually fix the offending code, but that depends on Hermod's specific motivations and circumstances.

Regards,

Cristiano

On 10/2/07, Ron Bodkin <rbodkin@xxxxxxxxxxxxxx> wrote:
> By the way, I usually match the field get on System.err and 
> System.out: even if you don't call println on them, you almost always 
> get those fields to output to them, which is usually not what you want 
> (e.g.,
> exception.printStackTrace(System.out) is also undesirable, as is print(...)
> or exception.printStackTrace()). If you match PrintStream, you might also
> have false positives by matching valid uses of a PrintStream to format file
> output...
>
> -----Original Message-----
> From: aspectj-users-bounces@xxxxxxxxxxx 
> [mailto:aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of Eric Bodden
> Sent: Monday, October 01, 2007 6:32 AM
> To: aspectj-users@xxxxxxxxxxx
> Subject: Re: [aspectj-users] System.out.prinltn and advicing
>
> I think it's actually pretty hard to exactly match that pattern, 
> especially not with a single advice. The problem is that
> System.out.println() actually consists of *two* joinpoints: A field 
> load (l = System.out) and a method call (l.println()), where l is a 
> stack location. For both you want to make sure that it's the same l 
> you are referring to.
>
> You should be able to match this pattern with two pieces of advice 
> that collaborate on this pattern (or by using a tracematch 
> http://abc.comlab.ox.ac.uk/papers#oopsla2005 :-))
>
> Eric
>
> On 01/10/2007, hermod.opstvedt@xxxxxxxxx <hermod.opstvedt@xxxxxxxxx> 
> wrote:
> >
> >
> > Hi
> >
> > Albeit no - It says it advises it, but when I run the tests, it 
> > never
> enters
> > the around advice
> >
> > Hermod
> >
> >
> > -----Original Message-----
> > From: aspectj-users-bounces@xxxxxxxxxxx 
> > [mailto:aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of 
> > hermod.opstvedt@xxxxxxxxx
> > Sent: Monday, October 01, 2007 2:35 PM
> > To: aspectj-users@xxxxxxxxxxx
> > Subject: RE: [aspectj-users] System.out.prinltn and advicing
> >
> >
> > Hi
> >
> > I solved it :)
> >
> >
> >
> > pointcut systemOut() : get(* System.out);
> >
> > pointcut systemErr() : get(* System.err);
> >
> > pointcut sysout() : call(void *.println(..)) && 
> > !within(SysOutLoggAspect)
> &&
> > (cflow(systemOut()) || cflow(systemErr()));
> >
> > This seems to do the trick.
> >
> > Hermod
> >
> >
> > -----Original Message-----
> > From: aspectj-users-bounces@xxxxxxxxxxx 
> > [mailto:aspectj-users-bounces@xxxxxxxxxxx] On Behalf Of 
> > hermod.opstvedt@xxxxxxxxx
> > Sent: Monday, October 01, 2007 2:12 PM
> > To: aspectj-users@xxxxxxxxxxx
> > Subject: [aspectj-users] System.out.prinltn and advicing
> >
> >
> >
> > Hi
> >
> > I am trying to write an advice that traps calls to 
> > System.out.println and System.err.println.
> >
> > If I write the advice as follows:
> >
> > public aspect SysOutLoggAspect {
> >
> >         private final Logger LOGGER = 
> > Logger.getLogger(SysOutLoggAspect.class);
> >
> >         pointcut sysout() : (call(void System.out.println(..)) ||
> call(void
> > System.err.println(..)) ) && !within(SysOutLoggAspect);
> >
> >         void around(Object obj) : sysout() && args(obj) {
> >                 Level level = LOGGER.getEffectiveLevel();
> >
> > LOGGER.log(thisJoinPoint.getSignature().getDeclaringTypeName(),
> > level,
> >                                 obj, null);
> >         }
> > }
> >
> > It does not apply the advice, and issues a warning (no match for 
> > this type
> > name: System.out [Xlint:invalidAbsoluteTypeName])  about it
> > in Eclipse (AJDT). If I however rewrite the pointcut to:
> >
> >         pointcut sysout() : call(void *.println(..)) && 
> > !within(SysOutLoggAspect);
> >
> > It does apply the advice, but I have no garantee that the method 
> > resides with System.out or System.err
> >
> > What is the best way to solve this?
> >
> > Hermod * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * 
> > * * *
> *
> > * * *
> >
> > This email with attachments is solely for the use of the individual 
> > or entity to whom it is addressed. Please also be aware that the DnB 
> > NOR
> Group
> > cannot accept any payment orders or other legally binding 
> > correspondence with customers as a part of an email.
> >
> > This email message has been virus checked by the anti virus programs 
> > used in the DnB NOR Group.
> >
> > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * 
> > * * *
> *
> >
> > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * 
> > * * *
> *
> >
> > This email with attachments is solely for the use of the individual 
> > or entity to whom it is addressed. Please also be aware that the DnB 
> > NOR
> Group
> > cannot accept any payment orders or other legally binding 
> > correspondence with customers as a part of an email.
> >
> > This email message has been virus checked by the anti virus programs 
> > used in the DnB NOR Group.
> >
> > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * 
> > * * *
> *
> >
> > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * 
> > * * *
> *
> >
> > This email with attachments is solely for the use of the individual 
> > or entity to whom it is addressed. Please also be aware that the DnB 
> > NOR
> Group
> > cannot accept any payment orders or other legally binding 
> > correspondence with customers as a part of an email.
> >
> > This email message has been virus checked by the anti virus programs 
> > used in the DnB NOR Group.
> >
> > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * 
> > * * *
> *
> >
> >
> >_______________________________________________
> > aspectj-users mailing list
> > aspectj-users@xxxxxxxxxxx  
> >https://dev.eclipse.org/mailman/listinfo/aspectj-users
> >
> >
>
>
> --
> Eric Bodden
> Sable Research Group
> McGill University, Montréal, Canada 
>_______________________________________________
> aspectj-users mailing list
> aspectj-users@xxxxxxxxxxx  
>https://dev.eclipse.org/mailman/listinfo/aspectj-users
>
>_______________________________________________
> aspectj-users mailing list
> aspectj-users@xxxxxxxxxxx  
>https://dev.eclipse.org/mailman/listinfo/aspectj-users
>
_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/aspectj-users
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *

This email with attachments is solely for the use of the individual or
entity to whom it is addressed. Please also be aware that the DnB NOR Group
cannot accept any payment orders or other legally binding correspondence with
customers as a part of an email. 

This email message has been virus checked by the anti virus programs used
in the DnB NOR Group.

* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *