persistence@glassfish.java.net

Re: Code review for Easier configurable category-specific logging level

From: Tom Ware <tom.ware_at_oracle.com>
Date: Thu, 28 Jun 2007 14:40:29 -0400

Hi Wonseok,

  Since this is an Enhancement rather than a bug, I have asked the
TopLink Product Manager to provide some feedback about the enhancement.
I will let you know if he has any comments.

  Just one comment for now:
- JavaLog.setLevel(). Our coding convention requires braces around the
contents of all if statements. Please add them to: if(logger == null)
return;

-Tom

Wonseok Kim wrote:

> Hi Tom, Marina,
>
> I implemented category-specific logging level property described in
> issue 1338
> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1338
> <https://glassfish.dev.java.net/issues/show_bug.cgi?id=1338>
>
> I introduced "toplink.logging.level.<category>" property in which
> <category> is the string defined in SessionLog like below.
>
> public static final String SQL = "sql";
> public static final String TRANSACTION = "transaction";
> public static final String EVENT = "event";
> public static final String CONNECTION = "connection";
> public static final String QUERY = "query";
> public static final String CACHE = "cache";
> public static final String PROPAGATION = "propagation";
> public static final String SEQUENCING = "sequencing";
> public static final String EJB = "ejb";
> public static final String DMS = "dms";
> public static final String EJB_OR_METADATA = "ejb_or_metadata";
> public static final String WEAVER = "weaver";
> public static final String PROPERTIES = "properties";
>
> If a user put a property like "toplink.logging.level.sql" with valid
> level value, this is set to SessionLog with setLevel(int level, String
> category) method.
> Most expected usecase of this property is that users want to see only
> SQL logs without other verbose log. this is now possible with
> toplink.logging.level.sql=FINE
>
> I also added category-specific log level facility to DefaultSessionLog
> class like JavaLog because DefaultSessionLog is the mostly used one in
> Java SE environment.
>
> Please review this and let me know if I missed something.
> I confirmed that this new property works as expected both with JavaLog
> and DefaultSessionLog.
>
> Thanks,
> Wonseok
>
>------------------------------------------------------------------------
>
>Index: entity-persistence/src/java/oracle/toplink/essentials/config/TopLinkProperties.java
>===================================================================
>RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/config/TopLinkProperties.java,v
>retrieving revision 1.7
>diff -c -w -r1.7 TopLinkProperties.java
>*** entity-persistence/src/java/oracle/toplink/essentials/config/TopLinkProperties.java 29 May 2007 14:57:43 -0000 1.7
>--- entity-persistence/src/java/oracle/toplink/essentials/config/TopLinkProperties.java 28 Jun 2007 16:56:45 -0000
>***************
>*** 113,118 ****
>--- 113,125 ----
> // Valid values are names of levels defined in java.util.logging.Level,
> // default value is java.util.logging.Level.CONFIG.getName()
> public static final String LOGGING_LEVEL = "toplink.logging.level";
>+
>+ // Category-specific logging level prefix
>+ // Property names formed out of this prefix by appending a category name
>+ // e.g.) toplink.logging.level.sql
>+ // Valid categories are defined in SessionLog
>+ public static final String CATEGORY_LOGGING_LEVEL_ = "toplink.logging.level.";
>+
> // By default ("true") the date is always logged.
> // This can be turned off ("false").
> public static final String LOGGING_TIMESTAMP = "toplink.logging.timestamp";
>Index: entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java
>===================================================================
>RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java,v
>retrieving revision 1.55
>diff -c -w -r1.55 EntityManagerSetupImpl.java
>*** entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java 30 May 2007 21:16:17 -0000 1.55
>--- entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java 28 Jun 2007 16:56:45 -0000
>***************
>*** 998,1003 ****
>--- 998,1015 ----
> if(logLevelString != null) {
> log.setLevel(AbstractSessionLog.translateStringToLoggingLevel(logLevelString));
> }
>+ // category-specific logging level
>+ Map categoryLogLevelMap = PropertiesHandler.getPrefixValuesLogDebug(TopLinkProperties.CATEGORY_LOGGING_LEVEL_, m, session);
>+ if(!categoryLogLevelMap.isEmpty()) {
>+ Iterator it = categoryLogLevelMap.entrySet().iterator();
>+ while (it.hasNext()) {
>+ Map.Entry entry = (Map.Entry)it.next();
>+ String name = (String)entry.getKey();
>+ String value = (String)entry.getValue();
>+ log.setLevel(AbstractSessionLog.translateStringToLoggingLevel(value), name);
>+ }
>+ }
>+
> String tsString = getConfigPropertyAsStringLogDebug(TopLinkProperties.LOGGING_TIMESTAMP, m, session);
> if (tsString != null) {
> log.setShouldPrintDate(Boolean.parseBoolean(tsString));
>Index: entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/PropertiesHandler.java
>===================================================================
>RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/PropertiesHandler.java,v
>retrieving revision 1.8
>diff -c -w -r1.8 PropertiesHandler.java
>*** entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/PropertiesHandler.java 29 May 2007 14:57:44 -0000 1.8
>--- entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/PropertiesHandler.java 28 Jun 2007 16:56:46 -0000
>***************
>*** 160,165 ****
>--- 160,166 ----
> static {
> addProp(new LoggerTypeProp());
> addProp(new LoggingLevelProp());
>+ addProp(new CategoryLoggingLevelProp());
> addProp(new TargetDatabaseProp());
> addProp(new TargetServerProp());
> addProp(new CacheSizeProp());
>***************
>*** 411,416 ****
>--- 412,435 ----
> }
> }
>
>+ protected static class CategoryLoggingLevelProp extends Prop {
>+ CategoryLoggingLevelProp() {
>+ super(TopLinkProperties.CATEGORY_LOGGING_LEVEL_);
>+ valueArray = new Object[] {
>+ Level.OFF.getName(),
>+ Level.SEVERE.getName(),
>+ Level.OFF.getName(),
>+ Level.WARNING.getName(),
>+ Level.INFO.getName(),
>+ Level.CONFIG.getName(),
>+ Level.FINE.getName(),
>+ Level.FINER.getName(),
>+ Level.FINEST.getName(),
>+ Level.ALL.getName()
>+ };
>+ }
>+ }
>+
> protected static class TargetDatabaseProp extends Prop {
> TargetDatabaseProp() {
> super(TopLinkProperties.TARGET_DATABASE, TargetDatabase.DEFAULT);
>Index: entity-persistence/src/java/oracle/toplink/essentials/logging/AbstractSessionLog.java
>===================================================================
>RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/logging/AbstractSessionLog.java,v
>retrieving revision 1.8
>diff -c -w -r1.8 AbstractSessionLog.java
>*** entity-persistence/src/java/oracle/toplink/essentials/logging/AbstractSessionLog.java 22 May 2007 23:54:44 -0000 1.8
>--- entity-persistence/src/java/oracle/toplink/essentials/logging/AbstractSessionLog.java 28 Jun 2007 16:56:46 -0000
>***************
>*** 203,210 ****
> /**
> * PUBLIC:
> * <p>
>! * Return the log level. Category is only needed in JavaLog
>! * to extract name space for the Logger which the log level belongs to.
> * </p><p>
> *
> * @return the log level
>--- 203,209 ----
> /**
> * PUBLIC:
> * <p>
>! * Return the log level for the category name space.
> * </p><p>
> *
> * @return the log level
>***************
>*** 232,239 ****
> /**
> * PUBLIC:
> * <p>
>! * Set the log level. Category is only needed in JavaLog
>! * to extract name space for the Logger which the log level belongs to.
> * </p><p>
> *
> * @param level the new log level
>--- 231,237 ----
> /**
> * PUBLIC:
> * <p>
>! * Set the log level for the category name space.
> * </p><p>
> *
> * @param level the new log level
>***************
>*** 263,271 ****
> /**
> * PUBLIC:
> * <p>
>! * Check if a message of the given level would actually be logged. Category
>! * is only needed in JavaLog to extract name space for the Logger which the log
>! * level belongs to. !isOff() is checked to screen out the possibility when both
> * log level and log request level are set to OFF.
> * </p><p>
> *
>--- 261,268 ----
> /**
> * PUBLIC:
> * <p>
>! * Check if a message of the given level would actually be logged for the category name space.
>! * !isOff() is checked to screen out the possibility when both
> * log level and log request level are set to OFF.
> * </p><p>
> *
>Index: entity-persistence/src/java/oracle/toplink/essentials/logging/DefaultSessionLog.java
>===================================================================
>RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/logging/DefaultSessionLog.java,v
>retrieving revision 1.5
>diff -c -w -r1.5 DefaultSessionLog.java
>*** entity-persistence/src/java/oracle/toplink/essentials/logging/DefaultSessionLog.java 22 May 2007 23:54:44 -0000 1.5
>--- entity-persistence/src/java/oracle/toplink/essentials/logging/DefaultSessionLog.java 28 Jun 2007 16:56:46 -0000
>***************
>*** 37,42 ****
>--- 37,47 ----
> package oracle.toplink.essentials.logging;
>
> import java.io.*;
>+ import java.util.HashMap;
>+ import java.util.Map;
>+ import java.util.logging.Level;
>+ import java.util.logging.Logger;
>+
> import oracle.toplink.essentials.exceptions.*;
> import oracle.toplink.essentials.internal.helper.*;
>
>***************
>*** 66,77 ****
>--- 71,92 ----
> protected String fileName;
>
> /**
>+ * Represents the Map that stores log levels per the name space strings.
>+ * The keys are category names. The values are log levels.
>+ */
>+ private Map<String, Integer> categoryLogLevelMap = new HashMap();
>+
>+ /**
> * PUBLIC:
> * Create a new default session log.
> */
> public DefaultSessionLog() {
> super();
> this.level = INFO;
>+ for (int i = 0; i < loggerCategories.length; i++) {
>+ String loggerCategory = loggerCategories[i];
>+ categoryLogLevelMap.put(loggerCategory, null);
>+ }
> }
>
> /**
>***************
>*** 90,95 ****
>--- 105,148 ----
> this.writer = writer;
> }
>
>+
>+ @Override
>+ public int getLevel(String category) {
>+ if(category != null) {
>+ Integer logLevel = categoryLogLevelMap.get(category);
>+ // if category-specific log level is not set, use parent level.
>+ if(logLevel != null) {
>+ return logLevel.intValue();
>+ }
>+ }
>+ return level;
>+ }
>+
>+ @Override
>+ public void setLevel(int level, String category) {
>+ if(category == null) {
>+ this.level = level;
>+ } else if(categoryLogLevelMap.containsKey(category)) {
>+ categoryLogLevelMap.put(category, level);
>+ }
>+ }
>+
>+ /**
>+ * PUBLIC:
>+ * <p>
>+ * Check if a message of the given level would actually be logged by the logger
>+ * with name space built from the given session and category.
>+ * Return the shouldLog for the given category from
>+ * </p><p>
>+ * @return true if the given message level will be logged
>+ * </p>
>+ */
>+ @Override
>+ public boolean shouldLog(int level, String category) {
>+ return (getLevel(category) <= level);
>+ }
>+
>+
> /**
> * INTERNAL:
> * Log the entry.
>***************
>*** 97,103 ****
> * This must be synchronized as it will be called by many threads in three-tier.
> */
> public synchronized void log(SessionLogEntry entry) {
>! if (!shouldLog(entry.getLevel())) {
> return;
> }
>
>--- 150,156 ----
> * This must be synchronized as it will be called by many threads in three-tier.
> */
> public synchronized void log(SessionLogEntry entry) {
>! if (!shouldLog(entry.getLevel(), entry.getNameSpace())) {
> return;
> }
>
>Index: entity-persistence/src/java/oracle/toplink/essentials/logging/JavaLog.java
>===================================================================
>RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/logging/JavaLog.java,v
>retrieving revision 1.9
>diff -c -w -r1.9 JavaLog.java
>*** entity-persistence/src/java/oracle/toplink/essentials/logging/JavaLog.java 22 May 2007 23:54:44 -0000 1.9
>--- entity-persistence/src/java/oracle/toplink/essentials/logging/JavaLog.java 28 Jun 2007 16:56:46 -0000
>***************
>*** 140,145 ****
>--- 140,147 ----
> */
> public void setLevel(final int level, String category) {
> final Logger logger = getLogger(category);
>+ if(logger == null) return;
>+
> AccessController.doPrivileged(new PrivilegedAction() {
> public Object run() {
> logger.setLevel(getJavaLevel(level));
>
>