Hi Remy, On Fri, Oct 14, 2016 at 3:46 PM, <r...@apache.org> wrote:
> Author: remm > Date: Fri Oct 14 13:46:45 2016 > New Revision: 1764897 > > URL: http://svn.apache.org/viewvc?rev=1764897&view=rev > Log: > Inspired by 60161: Allow creating subcategories from the container logger > name. The configuration remains compatible, but some possibly logging > intensive components get their own playground. Use it for the rewrite valve. > > Modified: > tomcat/trunk/java/org/apache/catalina/Container.java > tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java > tomcat/trunk/java/org/apache/catalina/startup/FailedContext.java > tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java > tomcat/trunk/test/org/apache/tomcat/unittest/TesterContext.java > tomcat/trunk/test/org/apache/tomcat/unittest/TesterHost.java > tomcat/trunk/webapps/docs/changelog.xml > > Modified: tomcat/trunk/java/org/apache/catalina/Container.java > URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/ > catalina/Container.java?rev=1764897&r1=1764896&r2=1764897&view=diff > ============================================================ > ================== > --- tomcat/trunk/java/org/apache/catalina/Container.java (original) > +++ tomcat/trunk/java/org/apache/catalina/Container.java Fri Oct 14 > 13:46:45 2016 > @@ -123,6 +123,13 @@ public interface Container extends Lifec > > > /** > + * Return the logger name that the container will use. > + * @return the abbreviated name of this container for logging messages > + */ > + public String getLogName(); > + > + > + /** > * Obtain the JMX name for this container. > * > * @return the JMX name associated with this container. > > Modified: tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java > URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/ > catalina/core/ContainerBase.java?rev=1764897&r1=1764896& > r2=1764897&view=diff > ============================================================ > ================== > --- tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java > (original) > +++ tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java Fri Oct > 14 13:46:45 2016 > @@ -357,13 +357,41 @@ public abstract class ContainerBase exte > > if (logger != null) > return (logger); > - logger = LogFactory.getLog(logName()); > + logger = LogFactory.getLog(getLogName()); > What would be the behavior here if #getLogName() returns null ? > return (logger); > > } > > > /** > + * @return the abbreviated name of this container for logging messages > + */ > + @Override > + public String getLogName() { > + > + if (logName != null) { > + return logName; > + } > + String loggerName = null; > + Container current = this; > + while (current != null) { > + String name = current.getName(); > + if ((name == null) || (name.equals(""))) { > + name = "/"; > + } else if (name.startsWith("##")) { > + name = "/" + name; > + } > + loggerName = "[" + name + "]" > + + ((loggerName != null) ? ("." + loggerName) : ""); > + current = current.getParent(); > + } > + logName = ContainerBase.class.getName() + "." + loggerName; > + return logName; > + > + } > + > + > + /** > * Return the Cluster with which this Container is associated. If > there is > * no associated Cluster, return the Cluster associated with our > parent > * Container (if any); otherwise return <code>null</code>. > @@ -1183,33 +1211,6 @@ public abstract class ContainerBase exte > } > > > - /** > - * @return the abbreviated name of this container for logging messages > - */ > - protected String logName() { > - > - if (logName != null) { > - return logName; > - } > - String loggerName = null; > - Container current = this; > - while (current != null) { > - String name = current.getName(); > - if ((name == null) || (name.equals(""))) { > - name = "/"; > - } else if (name.startsWith("##")) { > - name = "/" + name; > - } > - loggerName = "[" + name + "]" > - + ((loggerName != null) ? ("." + loggerName) : ""); > - current = current.getParent(); > - } > - logName = ContainerBase.class.getName() + "." + loggerName; > - return logName; > - > - } > - > - > // -------------------- JMX and Registration -------------------- > > @Override > > Modified: tomcat/trunk/java/org/apache/catalina/startup/FailedContext.java > URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/ > catalina/startup/FailedContext.java?rev=1764897&r1=1764896&r2=1764897& > view=diff > ============================================================ > ================== > --- tomcat/trunk/java/org/apache/catalina/startup/FailedContext.java > (original) > +++ tomcat/trunk/java/org/apache/catalina/startup/FailedContext.java Fri > Oct 14 13:46:45 2016 > @@ -241,6 +241,9 @@ public class FailedContext extends Lifec > public Log getLogger() { return null; } > > @Override > + public String getLogName() { return null; } > Maybe it is better to return something like "FailedContextLog" instead ?! This is related to the first question above. > + > + @Override > public Manager getManager() { return null; } > @Override > public void setManager(Manager manager) { /* NO-OP */ } > > Modified: tomcat/trunk/java/org/apache/catalina/valves/rewrite/ > RewriteValve.java > URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/ > catalina/valves/rewrite/RewriteValve.java?rev=1764897& > r1=1764896&r2=1764897&view=diff > ============================================================ > ================== > --- tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java > (original) > +++ tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java > Fri Oct 14 13:46:45 2016 > @@ -47,6 +47,7 @@ import org.apache.catalina.connector.Req > import org.apache.catalina.connector.Response; > import org.apache.catalina.util.URLEncoder; > import org.apache.catalina.valves.ValveBase; > +import org.apache.juli.logging.LogFactory; > import org.apache.tomcat.util.buf.CharChunk; > import org.apache.tomcat.util.buf.MessageBytes; > import org.apache.tomcat.util.buf.UriUtil; > @@ -143,6 +144,14 @@ public class RewriteValve extends ValveB > this.enabled = enabled; > } > > + > + @Override > + protected void initInternal() throws LifecycleException { > + super.initInternal(); > + containerLog = LogFactory.getLog(getContainer().getLogName() + > ".rewrite"); > + } > + > + > @Override > protected synchronized void startInternal() throws LifecycleException > { > > @@ -155,11 +164,11 @@ public class RewriteValve extends ValveB > context = true; > is = ((Context) getContainer()).getServletContext() > .getResourceAsStream("/WEB-INF/" + resourcePath); > - if (container.getLogger().isDebugEnabled()) { > + if (containerLog.isDebugEnabled()) { > if (is == null) { > - container.getLogger().debug("No configuration > resource found: /WEB-INF/" + resourcePath); > + containerLog.debug("No configuration resource found: > /WEB-INF/" + resourcePath); > } else { > - container.getLogger().debug("Read configuration > from: /WEB-INF/" + resourcePath); > + containerLog.debug("Read configuration from: > /WEB-INF/" + resourcePath); > } > } > } else if (getContainer() instanceof Host) { > @@ -170,21 +179,21 @@ public class RewriteValve extends ValveB > // Use getResource and getResourceAsStream > is = getClass().getClassLoader() > .getResourceAsStream(resourceName); > - if (is != null && container.getLogger().isDebugEnabled()) > { > - container.getLogger().debug("Read configuration > from CL at " + resourceName); > + if (is != null && containerLog.isDebugEnabled()) { > + containerLog.debug("Read configuration from CL at > " + resourceName); > } > } else { > - if (container.getLogger().isDebugEnabled()) { > - container.getLogger().debug("Read configuration > from " + file.getAbsolutePath()); > + if (containerLog.isDebugEnabled()) { > + containerLog.debug("Read configuration from " + > file.getAbsolutePath()); > } > is = new FileInputStream(file); > } > - if ((is == null) && (container.getLogger().isDebugEnabled())) > { > - container.getLogger().debug("No configuration > resource found: " + resourceName + > + if ((is == null) && (containerLog.isDebugEnabled())) { > + containerLog.debug("No configuration resource found: > " + resourceName + > " in " + getConfigBase() + " or in the > classloader"); > } > } catch (Exception e) { > - container.getLogger().error("Error opening > configuration", e); > + containerLog.error("Error opening configuration", e); > } > } > > @@ -197,12 +206,12 @@ public class RewriteValve extends ValveB > BufferedReader reader = new BufferedReader(isr)) { > parse(reader); > } catch (IOException ioe) { > - container.getLogger().error("Error closing configuration", > ioe); > + containerLog.error("Error closing configuration", ioe); > } finally { > try { > is.close(); > } catch (IOException e) { > - container.getLogger().error("Error closing > configuration", e); > + containerLog.error("Error closing configuration", e); > } > } > > @@ -210,6 +219,9 @@ public class RewriteValve extends ValveB > > public void setConfiguration(String configuration) > throws Exception { > + if (containerLog == null) { > + containerLog = LogFactory.getLog(getContainer().getLogName() > + ".rewrite"); > + } > maps.clear(); > parse(new BufferedReader(new StringReader(configuration))); > } > @@ -238,8 +250,8 @@ public class RewriteValve extends ValveB > Object result = parse(line); > if (result instanceof RewriteRule) { > RewriteRule rule = (RewriteRule) result; > - if (container.getLogger().isDebugEnabled()) { > - container.getLogger().debug("Add rule with > pattern " + rule.getPatternString() > + if (containerLog.isDebugEnabled()) { > + containerLog.debug("Add rule with pattern " + > rule.getPatternString() > + " and substitution " + > rule.getSubstitutionString()); > } > for (int i = (conditions.size() - 1); i > 0; i--) { > @@ -248,9 +260,9 @@ public class RewriteValve extends ValveB > } > } > for (int i = 0; i < conditions.size(); i++) { > - if (container.getLogger().isDebugEnabled()) { > + if (containerLog.isDebugEnabled()) { > RewriteCond cond = conditions.get(i); > - container.getLogger().debug("Add condition " > + cond.getCondPattern() > + containerLog.debug("Add condition " + > cond.getCondPattern() > + " test " + cond.getTestString() + " > to rule with pattern " > + rule.getPatternString() + " and > substitution " > + rule.getSubstitutionString() + > (cond.isOrnext() ? " [OR]" : "") > @@ -271,7 +283,7 @@ public class RewriteValve extends ValveB > } > } > } catch (IOException e) { > - container.getLogger().error("Error reading > configuration", e); > + containerLog.error("Error reading configuration", e); > } > } > this.rules = rules.toArray(new RewriteRule[0]); > @@ -338,8 +350,8 @@ public class RewriteValve extends ValveB > CharSequence test = (rule.isHost()) ? host : urlDecoded; > CharSequence newtest = rule.evaluate(test, resolver); > if (newtest != null && !test.equals(newtest.toString())) > { > - if (container.getLogger().isDebugEnabled()) { > - container.getLogger().debug("Rewrote " + test + > " as " + newtest > + if (containerLog.isDebugEnabled()) { > + containerLog.debug("Rewrote " + test + " as " + > newtest > + " with rule pattern " + > rule.getPatternString()); > } > if (rule.isHost()) { > > Modified: tomcat/trunk/test/org/apache/tomcat/unittest/TesterContext.java > URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/ > tomcat/unittest/TesterContext.java?rev=1764897&r1=1764896& > r2=1764897&view=diff > ============================================================ > ================== > --- tomcat/trunk/test/org/apache/tomcat/unittest/TesterContext.java > (original) > +++ tomcat/trunk/test/org/apache/tomcat/unittest/TesterContext.java Fri > Oct 14 13:46:45 2016 > @@ -116,6 +116,11 @@ public class TesterContext implements Co > } > > @Override > + public String getLogName() { > + return null; > + } > + > + @Override > public ObjectName getObjectName() { > return null; > } > > Modified: tomcat/trunk/test/org/apache/tomcat/unittest/TesterHost.java > URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/ > tomcat/unittest/TesterHost.java?rev=1764897&r1=1764896& > r2=1764897&view=diff > ============================================================ > ================== > --- tomcat/trunk/test/org/apache/tomcat/unittest/TesterHost.java > (original) > +++ tomcat/trunk/test/org/apache/tomcat/unittest/TesterHost.java Fri Oct > 14 13:46:45 2016 > @@ -45,6 +45,11 @@ public class TesterHost implements Host > } > > @Override > + public String getLogName() { > + return null; > + } > + > + @Override > public ObjectName getObjectName() { > return null; > } > > Modified: tomcat/trunk/webapps/docs/changelog.xml > URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/ > changelog.xml?rev=1764897&r1=1764896&r2=1764897&view=diff > ============================================================ > ================== > --- tomcat/trunk/webapps/docs/changelog.xml (original) > +++ tomcat/trunk/webapps/docs/changelog.xml Fri Oct 14 13:46:45 2016 > @@ -72,6 +72,10 @@ > When calling <code>getResourceAsStream()</code> on a directory, > ensure > that <code>null</code> is returned. (markt) > </fix> > + <fix> > + <bug>60161</bug>: Allow creating subcategories of the container > logger, > + and use it for the rewrite valve. (remm) > + </fix> > </changelog> > </subsection> > <subsection name="Coyote"> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
