From 3aff80a5c30e9dd50e80b8497494b5335e0a2cc1 Mon Sep 17 00:00:00 2001 From: Remy Maucherat Date: Fri, 14 Oct 2016 13:46:45 +0000 Subject: [PATCH] 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. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1764897 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/Container.java | 7 +++ .../apache/catalina/core/ContainerBase.java | 57 ++++++++++--------- .../catalina/startup/FailedContext.java | 3 + .../catalina/valves/rewrite/RewriteValve.java | 50 +++++++++------- .../apache/tomcat/unittest/TesterContext.java | 5 ++ .../apache/tomcat/unittest/TesterHost.java | 5 ++ webapps/docs/changelog.xml | 4 ++ 7 files changed, 84 insertions(+), 47 deletions(-) diff --git a/java/org/apache/catalina/Container.java b/java/org/apache/catalina/Container.java index a57e9d66bd4a..dc7179b1b106 100644 --- a/java/org/apache/catalina/Container.java +++ b/java/org/apache/catalina/Container.java @@ -122,6 +122,13 @@ public interface Container extends Lifecycle { public Log getLogger(); + /** + * 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. * diff --git a/java/org/apache/catalina/core/ContainerBase.java b/java/org/apache/catalina/core/ContainerBase.java index d88cb00f0809..353e4b27f56c 100644 --- a/java/org/apache/catalina/core/ContainerBase.java +++ b/java/org/apache/catalina/core/ContainerBase.java @@ -357,12 +357,40 @@ public Log getLogger() { if (logger != null) return (logger); - logger = LogFactory.getLog(logName()); + logger = LogFactory.getLog(getLogName()); 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 @@ -1183,33 +1211,6 @@ public void fireContainerEvent(String type, Object data) { } - /** - * @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 diff --git a/java/org/apache/catalina/startup/FailedContext.java b/java/org/apache/catalina/startup/FailedContext.java index fcb66dc53656..b3627e9c01f2 100644 --- a/java/org/apache/catalina/startup/FailedContext.java +++ b/java/org/apache/catalina/startup/FailedContext.java @@ -240,6 +240,9 @@ public void setLoader(Loader loader) { /* NO-OP */ } @Override public Log getLogger() { return null; } + @Override + public String getLogName() { return null; } + @Override public Manager getManager() { return null; } @Override diff --git a/java/org/apache/catalina/valves/rewrite/RewriteValve.java b/java/org/apache/catalina/valves/rewrite/RewriteValve.java index 116145586a87..ce4527d5f77e 100644 --- a/java/org/apache/catalina/valves/rewrite/RewriteValve.java +++ b/java/org/apache/catalina/valves/rewrite/RewriteValve.java @@ -47,6 +47,7 @@ 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 void setEnabled(boolean enabled) { 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 @@ protected synchronized void startInternal() throws LifecycleException { 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 @@ protected synchronized void startInternal() throws LifecycleException { // 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 @@ protected synchronized void startInternal() throws LifecycleException { 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 @@ protected synchronized void startInternal() throws LifecycleException { 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 @@ protected void parse(BufferedReader reader) throws LifecycleException { 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 @@ protected void parse(BufferedReader reader) throws LifecycleException { } } 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 @@ protected void parse(BufferedReader reader) throws LifecycleException { } } } 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 void invoke(Request request, Response response) 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()) { diff --git a/test/org/apache/tomcat/unittest/TesterContext.java b/test/org/apache/tomcat/unittest/TesterContext.java index b7bce3296e74..4109c41cb8e4 100644 --- a/test/org/apache/tomcat/unittest/TesterContext.java +++ b/test/org/apache/tomcat/unittest/TesterContext.java @@ -115,6 +115,11 @@ public Log getLogger() { return log; } + @Override + public String getLogName() { + return null; + } + @Override public ObjectName getObjectName() { return null; diff --git a/test/org/apache/tomcat/unittest/TesterHost.java b/test/org/apache/tomcat/unittest/TesterHost.java index 858230652652..ae3379e83c4f 100644 --- a/test/org/apache/tomcat/unittest/TesterHost.java +++ b/test/org/apache/tomcat/unittest/TesterHost.java @@ -44,6 +44,11 @@ public Log getLogger() { return null; } + @Override + public String getLogName() { + return null; + } + @Override public ObjectName getObjectName() { return null; diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index c71d4c5cc4c5..76876ae8e0f9 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -72,6 +72,10 @@ When calling getResourceAsStream() on a directory, ensure that null is returned. (markt) + + 60161: Allow creating subcategories of the container logger, + and use it for the rewrite valve. (remm) +