[RFC][IcedTea-Web]: Add Logging of applet exceptions.
Dr Andrew John Hughes
ahughes at redhat.com
Wed Jan 12 07:08:23 PST 2011
On 21:46 Tue 11 Jan , Andrew Su wrote:
> On 01/11/2011 04:30 PM, Omair Majid wrote:
> > On 01/11/2011 03:39 PM, Andrew Su wrote:
> >>
> >> ----- Original Message -----
> >>>> From: "Andrew Su"<asu at redhat.com> To:
> >>>> "OpenJDK"<distro-pkg-dev at openjdk.java.net> Sent: Tuesday, January
> >>>> 11, 2011 2:19:57 PM Subject: [RFC][IcedTea-Web]: Add Logging of
> >>>> applet exceptions. Hello,
> >>>>
> >>>> I have attached a patch which allows exceptions be logged to file
> >>>> if the option "Enable logging" is set in deployments.property
> >>>> file.
> >>>>
> >>>> Comments? Questions? Concerns?
> >> Hi,
> >>
> >> I have updated the patch (removed unused import).
> >
> > Am I correct in understanding that this only log exceptions from
> > init()/start()/stop()/destroy() ?
> Yes.
> >
> >>
> >> diff -r dc02a605f905 ChangeLog
> >> --- a/ChangeLog Fri Jan 07 08:00:08 2011 -0500
> >> +++ b/ChangeLog Tue Jan 11 15:37:29 2011 -0500
> >> @@ -1,3 +1,13 @@
> >> +2011-01-11 Andrew Su<asu at redhat.com>
> >> +
> >> + * makefile.am: Added net.sourceforge.jnlp.config to NETX_PKGS
> >> + variable.
> >
> > Looks like a typo here (should be net.sourceforge.jnlp.debug). On that
> > note, net.sourceforge.jnlp.config is not in NETX_PKGS either. That
> > must have been my mistake.
> Oops, I was going to put that in another patch, but accidentally typed
> it there..
> >
> >> + * netx/net/sourceforge/jnlp/NetxPanel.java:
> >> + (showAppletException): Override, adds logging to file. Then proceed
> >> + with showAppletException in sun.applet.AppletPanel
> >> + * netx/net/sourceforge/jnlp/debug/AppletLogger.java: New class.
> >> + * netx/net/sourceforge/jnlp/debug/Log.java: New class.
> >> +
> >> 2011-01-04 Omair Majid<omajid at redhat.com>
> >>
> >> * netx/net/sourceforge/jnlp/security/KeyStores.java
> >> diff -r dc02a605f905 Makefile.am
> >> --- a/Makefile.am Fri Jan 07 08:00:08 2011 -0500
> >> +++ b/Makefile.am Tue Jan 11 15:37:29 2011 -0500
> >> @@ -29,7 +29,8 @@
> >> net.sourceforge.jnlp.cache net.sourceforge.jnlp.event \
> >> net.sourceforge.jnlp.security
> >> net.sourceforge.jnlp.security.viewer \
> >> net.sourceforge.jnlp.services net.sourceforge.jnlp.tools \
> >> - net.sourceforge.jnlp.util net.sourceforge.jnlp.controlpanel
> >> + net.sourceforge.jnlp.util net.sourceforge.jnlp.controlpanel \
> >> + net.sourceforge.jnlp.debug
> >>
> >> # Conditional defintions
> >> if ENABLE_PLUGIN
> >> diff -r dc02a605f905 netx/net/sourceforge/jnlp/NetxPanel.java
> >> --- a/netx/net/sourceforge/jnlp/NetxPanel.java Fri Jan 07 08:00:08
> >> 2011 -0500
> >> +++ b/netx/net/sourceforge/jnlp/NetxPanel.java Tue Jan 11 15:37:29
> >> 2011 -0500
> >> @@ -22,6 +22,7 @@
> >>
> >> package net.sourceforge.jnlp;
> >>
> >> +import net.sourceforge.jnlp.debug.AppletLogger;
> >> import net.sourceforge.jnlp.runtime.AppThreadGroup;
> >> import net.sourceforge.jnlp.runtime.AppletInstance;
> >> import net.sourceforge.jnlp.runtime.JNLPRuntime;
> >> @@ -68,6 +69,12 @@
> >> super.run();
> >> }
> >>
> >> + @Override
> >> + protected void showAppletException(Throwable t) {
> >> + AppletLogger.log(t); // log the exception
> >> + super.showAppletException(t);
> >> + }
> >> +
> >
> > Nicely done!
> Thanks!
> >
> >> //Overriding to use Netx classloader. You might need to relax
> >> visibility
> >> //in sun.applet.AppletPanel for runLoader().
> >> protected void runLoader() {
> >> diff -r dc02a605f905 netx/net/sourceforge/jnlp/debug/AppletLogger.java
> >> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> >> +++ b/netx/net/sourceforge/jnlp/debug/AppletLogger.java Tue Jan 11
> >> 15:37:29 2011 -0500
> >> @@ -0,0 +1,42 @@
> >> +package net.sourceforge.jnlp.debug;
> >> +
> >> +import java.io.IOException;
> >> +import java.util.logging.FileHandler;
> >> +import java.util.logging.Level;
> >> +import java.util.logging.Logger;
> >> +import java.util.logging.XMLFormatter;
> >> +
> >> +/**
> >> + * This class writes log information to file.
> >> + * @author Andrew Su (asu at redhat.com,andrew.su at utoronto.ca)
> >> + *
> >> + */
> >> +public class AppletLogger extends Log {
> >
> > This may be just nitpicking, but I dont like the names. "AppletLogger
> > extends Log" sounds a little strange. Maybe use "AppletLog extends
> > Log" or "AppletLogger extends Logger"?
> Ah, you're not going to like the follow up patch for adding tracing,
> "AppletTracer" guess I'll make that "AppletTrace". ;)
>
> >
> >> + public static Logger logger;
> >> +
> >> + static {
> >> + try {
> >> + // If logging is enabled, we create logger.
> >> + if (enableLogging) {
> >> + String fn = icedteaLogDir + "Plugin" +
> >> java.lang.System.currentTimeMillis() + ".log";
> >> + boolean append = false;
> >> + FileHandler fh = new FileHandler(fn, append);
> >> + fh.setFormatter(new XMLFormatter());
> >
> > Hmm.. is this line saying that logs should be written in XML?
> Yes, this is to follow the format of how the proprietary plugin does
> logging. This will keep any tools being used with those logs to continue
> with our plugin's logs.
Is there any real advantage to this? Do you have an example of the output
to see how readable it is? I don't think we should do things just because
some proprietary vendor's product does.
> >
> >> + logger =
> >> Logger.getLogger(AppletLogger.class.getPackage().getName() + "." +
> >> AppletLogger.class.getName());
> >> + logger.setLevel(Level.ALL);
> >> + logger.addHandler(fh);
> >> + }
> >> + } catch (IOException e) {
> >> + e.printStackTrace();
> >> + }
> >> + }
> >> +
> >> + public synchronized static void log(Throwable e) {
> >> + if (enableLogging) {
> >> + String ex = e.toString();
> >> + for (StackTraceElement ste : e.getStackTrace())
> >> + ex = ex + "\n\tat " + ste;
> >> + logger.log(Level.FINE, ex);
> >> + }
> >
> > Perhaps using ex.printStackTrace(PrintStream) might be better? I dont
> > know how well the simple for loop handles exceptions with causes.
> Ah, yes, changed.
> >
> >> + }
> >> +}
> >> diff -r dc02a605f905 netx/net/sourceforge/jnlp/debug/Log.java
> >> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> >> +++ b/netx/net/sourceforge/jnlp/debug/Log.java Tue Jan 11 15:37:29
> >> 2011 -0500
> >> @@ -0,0 +1,46 @@
> >> +package net.sourceforge.jnlp.debug;
> >> +
> >> +import java.io.File;
> >> +
> >> +import javax.naming.ConfigurationException;
> >> +
> >> +import net.sourceforge.jnlp.config.DeploymentConfiguration;
> >> +
> >> +/**
> >> + * This file provides the information required to do logging.
> >> + * @author Andrew Su (asu at redhat.com,andrew.su at utoronto.ca)
> >> + *
> >> + */
> >> +public class Log {
> >> +
> >> + // Directory where the logs are stored.
> >> + public static String icedteaLogDir;
> >> +
> >> + public static boolean enableLogging = false;
> >> + public static boolean enableTracing = false;
> >> +
> >> + // Prepare for logging.
> >> + static {
> >> + DeploymentConfiguration config = new DeploymentConfiguration();
> >> + try {
> >> + config.load();
> >> + } catch (ConfigurationException e) {
> >> + e.printStackTrace();
> >> + }
> >> +
> >
> > This is done by JNLPRuntime.initialize() early in the plugin
> > initialization phase. I dont think this is needed here at all.
> Changed to calling JNLPRuntime.getConfiguration().
>
> Thanks for looking it over, here is the updated patch with the said changes.
>
>
>
> diff -r dc02a605f905 ChangeLog
> --- a/ChangeLog Fri Jan 07 08:00:08 2011 -0500
> +++ b/ChangeLog Tue Jan 11 21:45:49 2011 -0500
> @@ -1,3 +1,13 @@
> +2011-01-11 Andrew Su <asu at redhat.com>
> +
> + * makefile.am: Added net.sourceforge.jnlp.debug to NETX_PKGS
> + variable.
> + * netx/net/sourceforge/jnlp/NetxPanel.java:
> + (showAppletException): Override, adds logging to file. Then proceed
> + with showAppletException in sun.applet.AppletPanel
> + * netx/net/sourceforge/jnlp/debug/AppletLog.java: New class.
> + * netx/net/sourceforge/jnlp/debug/Log.java: New class.
> +
> 2011-01-04 Omair Majid <omajid at redhat.com>
>
> * netx/net/sourceforge/jnlp/security/KeyStores.java
> diff -r dc02a605f905 Makefile.am
> --- a/Makefile.am Fri Jan 07 08:00:08 2011 -0500
> +++ b/Makefile.am Tue Jan 11 21:45:49 2011 -0500
> @@ -29,7 +29,8 @@
> net.sourceforge.jnlp.cache net.sourceforge.jnlp.event \
> net.sourceforge.jnlp.security net.sourceforge.jnlp.security.viewer \
> net.sourceforge.jnlp.services net.sourceforge.jnlp.tools \
> - net.sourceforge.jnlp.util net.sourceforge.jnlp.controlpanel
> + net.sourceforge.jnlp.util net.sourceforge.jnlp.controlpanel \
> + net.sourceforge.jnlp.debug
>
> # Conditional defintions
> if ENABLE_PLUGIN
> diff -r dc02a605f905 netx/net/sourceforge/jnlp/NetxPanel.java
> --- a/netx/net/sourceforge/jnlp/NetxPanel.java Fri Jan 07 08:00:08 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/NetxPanel.java Tue Jan 11 21:45:49 2011 -0500
> @@ -22,6 +22,7 @@
>
> package net.sourceforge.jnlp;
>
> +import net.sourceforge.jnlp.debug.AppletLog;
> import net.sourceforge.jnlp.runtime.AppThreadGroup;
> import net.sourceforge.jnlp.runtime.AppletInstance;
> import net.sourceforge.jnlp.runtime.JNLPRuntime;
> @@ -68,6 +69,12 @@
> super.run();
> }
>
> + @Override
> + protected void showAppletException(Throwable t) {
> + AppletLog.log(t); // log the exception
> + super.showAppletException(t);
> + }
> +
> //Overriding to use Netx classloader. You might need to relax visibility
> //in sun.applet.AppletPanel for runLoader().
> protected void runLoader() {
> diff -r dc02a605f905 netx/net/sourceforge/jnlp/debug/AppletLog.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/netx/net/sourceforge/jnlp/debug/AppletLog.java Tue Jan 11 21:45:49 2011 -0500
> @@ -0,0 +1,48 @@
> +package net.sourceforge.jnlp.debug;
> +
> +import java.io.ByteArrayOutputStream;
> +import java.io.IOException;
> +import java.io.PrintStream;
> +import java.util.logging.FileHandler;
> +import java.util.logging.Level;
> +import java.util.logging.Logger;
> +import java.util.logging.XMLFormatter;
> +
> +/**
> + * This class writes log information to file.
> + * @author Andrew Su (asu at redhat.com, andrew.su at utoronto.ca)
> + *
> + */
> +public class AppletLog extends Log {
> + public static Logger logger;
> +
> + static {
> + try {
> + // If logging is enabled, we create logger.
> + if (enableLogging) {
> + String fn = icedteaLogDir + "Plugin" + java.lang.System.currentTimeMillis() + ".log";
> + boolean append = false;
> + FileHandler fh = new FileHandler(fn, append);
> + fh.setFormatter(new XMLFormatter());
> + String logClassName = AppletLog.class.getName();
> + logger = Logger.getLogger(logClassName);
> + logger.setLevel(Level.ALL);
> + logger.addHandler(fh);
> + }
> + } catch (IOException e) {
> + e.printStackTrace();
> + }
> + }
> +
> + private AppletLog() {
> + }
> +
> + public synchronized static void log(Throwable e) {
> + if (enableLogging) {
> + ByteArrayOutputStream baos = new ByteArrayOutputStream();
> + PrintStream ps = new PrintStream(baos);
> + e.printStackTrace(ps);
> + logger.log(Level.FINE, baos.toString());
> + }
> + }
> +}
> diff -r dc02a605f905 netx/net/sourceforge/jnlp/debug/Log.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/netx/net/sourceforge/jnlp/debug/Log.java Tue Jan 11 21:45:49 2011 -0500
> @@ -0,0 +1,43 @@
> +package net.sourceforge.jnlp.debug;
> +
> +import java.io.File;
> +
> +import net.sourceforge.jnlp.config.DeploymentConfiguration;
> +import net.sourceforge.jnlp.runtime.JNLPRuntime;
> +
> +/**
> + * This file provides the information required to do logging.
> + * @author Andrew Su (asu at redhat.com, andrew.su at utoronto.ca)
> + *
> + */
> +public class Log {
> +
> + // Directory where the logs are stored.
> + public static String icedteaLogDir;
> +
> + public static boolean enableLogging = false;
> + public static boolean enableTracing = false;
> +
> + // Prepare for logging.
> + static {
> + DeploymentConfiguration config = JNLPRuntime.getConfiguration();
> +
> + // Check whether logging and tracing is enabled.
> + enableLogging = Boolean.parseBoolean(config.getProperty(DeploymentConfiguration.KEY_ENABLE_LOGGING));
> + enableTracing = Boolean.parseBoolean(config.getProperty(DeploymentConfiguration.KEY_ENABLE_TRACING));
> +
> + // Get log directory, create it if it doesn't exist. If unable to create and doesn't exist, don't log.
> + icedteaLogDir = config.getProperty(DeploymentConfiguration.KEY_USER_LOG_DIR);
> + System.out.println(icedteaLogDir);
> + if (icedteaLogDir != null) {
> + File f = new File(icedteaLogDir);
> + if (f.isDirectory() || f.mkdirs())
> + icedteaLogDir += File.separator;
> + else
> + enableLogging = false;
> + }
> + }
> +
> + Log() {}
> +
> +}
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint = F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
More information about the distro-pkg-dev
mailing list