[PATCH] AbstractAccessLogValve: Add pre/post add log element extension points
Locked
[PATCH] AbstractAccessLogValve: Add pre/post add log element extension points
 
|
a sub class can extend before and after the addElement loop to
establish a context via thread-dependend CharArrayWriter object. --- java/org/apache/catalina/valves/AbstractAccessLogValve.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java b/java/org/apache/catalina/valves/AbstractAccessLogValve.java index e23f49d3a5..5e774c2320 100644 --- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java +++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java @@ -685,9 +685,11 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access result = new CharArrayWriter(128); } + preLogAddElement(result); for (int i = 0; i < logElements.length; i++) { logElements[i].addElement(result, date, request, response, time); } + postLogAddElement(result); log(result); @@ -699,6 +701,9 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access // -------------------------------------------------------- Protected Methods + protected void preLogAddElement(CharArrayWriter result) {} + protected void postLogAddElement(CharArrayWriter result) {} + /** * Log the specified message. * -- 2.20.1 --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
Re: [PATCH] AbstractAccessLogValve: Add pre/post add log element extension points
|
|
Hi
Why cant it be added as element earlier at pattern parsing time? Would avoid to have two particular cases and keep current pattern/impl. A %java or so sounds more natural no? Le jeu. 21 janv. 2021 à 13:59, Thomas Meyer <[hidden email]> a écrit : > a sub class can extend before and after the addElement loop to > establish a context via thread-dependend CharArrayWriter object. > --- > java/org/apache/catalina/valves/AbstractAccessLogValve.java | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java > b/java/org/apache/catalina/valves/AbstractAccessLogValve.java > index e23f49d3a5..5e774c2320 100644 > --- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java > +++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java > @@ -685,9 +685,11 @@ public abstract class AbstractAccessLogValve extends > ValveBase implements Access > result = new CharArrayWriter(128); > } > > + preLogAddElement(result); > for (int i = 0; i < logElements.length; i++) { > logElements[i].addElement(result, date, request, response, > time); > } > + postLogAddElement(result); > > log(result); > > @@ -699,6 +701,9 @@ public abstract class AbstractAccessLogValve extends > ValveBase implements Access > > // -------------------------------------------------------- Protected > Methods > > + protected void preLogAddElement(CharArrayWriter result) {} > + protected void postLogAddElement(CharArrayWriter result) {} > + > /** > * Log the specified message. > * > -- > 2.20.1 > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > For additional commands, e-mail: [hidden email] > > |
Re: [PATCH] AbstractAccessLogValve: Add pre/post add log element extension points
|
|
On Thu, Jan 21, 2021 at 03:28:12PM +0100, Romain Manni-Bucau wrote:
> Hi Hi, > Why cant it be added as element earlier at pattern parsing time? Would > avoid to have two particular cases and keep current pattern/impl. > A %java or so sounds more natural no? Mhh, maybe I just share my idea of the concrete implementation, so you see what I want to do: Premise: The JUL logging is configured to log all below to an own handler. public class LoggingAccessLogValve extends AbstractAccessLogValve { private static final Log log = LogFactory.getLog(LoggingAccessLogValve.class); private Map<CharArrayWriter, JsonGenerator> writers = new WeakHashMap<>(); @Override protected void log(CharArrayWriter message) { if(log.isInfoEnabled()) { log.info(message); } } protected void preLogAddElement(CharArrayWriter result) { JsonGenerator jsonWriter = Json.createGenerator(result); synchronized (this) { writers.put(result,jsonWriter); } jsonWriter.writeStartObject(); } protected void postLogAddElement(CharArrayWriter result) { JsonGenerator jsonWriter = null; synchronized (this) { jsonWriter = writers.remove(result); } if(jsonWriter != null) { jsonWriter.writeEnd(); jsonWriter.close(); } } private class JsonAccessLogElementWrapper implements AccessLogElement { private final String jsonAttributeName; private final AccessLogElement delegate; public JsonAccessLogElementWrapper(AccessLogElement e, String name) { delegate = e; jsonAttributeName = name; } @Override public void addElement(CharArrayWriter buf, Date date, Request request, Response response, long time) { JsonGenerator jsonWriter = null; synchronized (this) { jsonWriter = writers.get(buf); } if(jsonWriter == null) { // TODO: add warn return; } // TODO: maybe even use stack/cache of CharyArrayWriter here, too CharArrayWriter writer = new CharArrayWriter(); delegate.addElement(writer, date, request, response, time); jsonWriter.write(jsonAttributeName, writer.toString()); } } @Override protected AccessLogElement createAccessLogElement(String name, char pattern) { AccessLogElement e = super.createAccessLogElement(name, pattern); return new JsonAccessLogElementWrapper(e, mapName(e) + '-' + name); } @Override protected AccessLogElement createAccessLogElement(char pattern) { AccessLogElement e = super.createAccessLogElement(pattern); return new JsonAccessLogElementWrapper(e, mapName(e)); } private String mapName(AccessLogElement e) { if(e instanceof RemoteAddrElement) { return "remoteAddr"; } else if(e instanceof HostElement) { return "host"; // TODO: finish } throw new IllegalArgumentException(); } } so what do you think about this approach? mfg thomas > > Le jeu. 21 janv. 2021 à 13:59, Thomas Meyer <[hidden email]> a écrit : > > > a sub class can extend before and after the addElement loop to > > establish a context via thread-dependend CharArrayWriter object. > > --- > > java/org/apache/catalina/valves/AbstractAccessLogValve.java | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java > > b/java/org/apache/catalina/valves/AbstractAccessLogValve.java > > index e23f49d3a5..5e774c2320 100644 > > --- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java > > +++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java > > @@ -685,9 +685,11 @@ public abstract class AbstractAccessLogValve extends > > ValveBase implements Access > > result = new CharArrayWriter(128); > > } > > > > + preLogAddElement(result); > > for (int i = 0; i < logElements.length; i++) { > > logElements[i].addElement(result, date, request, response, > > time); > > } > > + postLogAddElement(result); > > > > log(result); > > > > @@ -699,6 +701,9 @@ public abstract class AbstractAccessLogValve extends > > ValveBase implements Access > > > > // -------------------------------------------------------- Protected > > Methods > > > > + protected void preLogAddElement(CharArrayWriter result) {} > > + protected void postLogAddElement(CharArrayWriter result) {} > > + > > /** > > * Log the specified message. > > * > > -- > > 2.20.1 > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [hidden email] > > For additional commands, e-mail: [hidden email] > > > > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
Re: [PATCH] AbstractAccessLogValve: Add pre/post add log element extension points
|
|
Hmm, makes me think about two comments:
1. Looks like a custom formatter not matching inline syntax of abstract logging valve so maybe extract the parser/element factory in a class reusable by multiple implementations. 2. Your pre and post are specific to json but dont enable native json since null case will need a default and details like that dont make it too json friendly. Id rather use element extractors to create a json object i would jsonb.toJson before logging. It also brings another issue which is in some cases you will want to batch json log lines in arrays for faster sending - when not using docker json logging driver typically but directly pushing to solr/elasticsearch for ex. So overall a new valve creating a JsonRecord and serialized to string -or just using jsongenerator - is way easier for your use case imo than making the default valve abstracted which will require a lot of details. However, being able to list the elements to log and reusing logging valve element factory impl can ease it to be configurable so this is the part i would extract and then i would just do a JsonLoggingValve using a list of elements - not a real string pattern hut a list. Only code to import for json support looks like the string escaping (can be copied from johnzon Strings class or log4j2 equivalent class, it is small and avoids any dep). Hope it makes sense and helps a bit to solve your need. Side note: such a valve is already embeddable in your app enabling to have any custom dependency if it helps. Le jeu. 21 janv. 2021 à 16:03, Thomas Meyer <[hidden email]> a écrit : > On Thu, Jan 21, 2021 at 03:28:12PM +0100, Romain Manni-Bucau wrote: > > Hi > > Hi, > > > Why cant it be added as element earlier at pattern parsing time? Would > > avoid to have two particular cases and keep current pattern/impl. > > A %java or so sounds more natural no? > > Mhh, maybe I just share my idea of the concrete implementation, so you see > what I want to do: > > Premise: The JUL logging is configured to log all below to an own handler. > > public class LoggingAccessLogValve extends AbstractAccessLogValve { > > private static final Log log = > LogFactory.getLog(LoggingAccessLogValve.class); > private Map<CharArrayWriter, JsonGenerator> writers = new > WeakHashMap<>(); > > @Override > protected void log(CharArrayWriter message) { > if(log.isInfoEnabled()) { > log.info(message); > } > } > > protected void preLogAddElement(CharArrayWriter result) { > JsonGenerator jsonWriter = Json.createGenerator(result); > synchronized (this) { > writers.put(result,jsonWriter); > } > jsonWriter.writeStartObject(); > } > > protected void postLogAddElement(CharArrayWriter result) { > JsonGenerator jsonWriter = null; > synchronized (this) { > jsonWriter = writers.remove(result); > } > if(jsonWriter != null) { > jsonWriter.writeEnd(); > jsonWriter.close(); > } > } > > private class JsonAccessLogElementWrapper implements > AccessLogElement { > > private final String jsonAttributeName; > private final AccessLogElement delegate; > > public JsonAccessLogElementWrapper(AccessLogElement e, > String name) { > delegate = e; > jsonAttributeName = name; > } > > @Override > public void addElement(CharArrayWriter buf, Date date, > Request request, Response response, long time) { > JsonGenerator jsonWriter = null; > synchronized (this) { > jsonWriter = writers.get(buf); > } > if(jsonWriter == null) { > // TODO: add warn > return; > } > > // TODO: maybe even use stack/cache of > CharyArrayWriter here, too > CharArrayWriter writer = new CharArrayWriter(); > delegate.addElement(writer, date, request, > response, time); > jsonWriter.write(jsonAttributeName, > writer.toString()); > } > } > > @Override > protected AccessLogElement createAccessLogElement(String name, > char pattern) { > AccessLogElement e = super.createAccessLogElement(name, > pattern); > return new JsonAccessLogElementWrapper(e, mapName(e) + '-' > + name); > } > > @Override > protected AccessLogElement createAccessLogElement(char pattern) { > AccessLogElement e = super.createAccessLogElement(pattern); > return new JsonAccessLogElementWrapper(e, mapName(e)); > } > > private String mapName(AccessLogElement e) { > if(e instanceof RemoteAddrElement) { > return "remoteAddr"; > } else if(e instanceof HostElement) { > return "host"; > // TODO: finish > } > throw new IllegalArgumentException(); > } > } > > so what do you think about this approach? > > mfg > thomas > > > > > Le jeu. 21 janv. 2021 à 13:59, Thomas Meyer <[hidden email]> a écrit : > > > > > a sub class can extend before and after the addElement loop to > > > establish a context via thread-dependend CharArrayWriter object. > > > --- > > > java/org/apache/catalina/valves/AbstractAccessLogValve.java | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git > a/java/org/apache/catalina/valves/AbstractAccessLogValve.java > > > b/java/org/apache/catalina/valves/AbstractAccessLogValve.java > > > index e23f49d3a5..5e774c2320 100644 > > > --- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java > > > +++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java > > > @@ -685,9 +685,11 @@ public abstract class AbstractAccessLogValve > extends > > > ValveBase implements Access > > > result = new CharArrayWriter(128); > > > } > > > > > > + preLogAddElement(result); > > > for (int i = 0; i < logElements.length; i++) { > > > logElements[i].addElement(result, date, request, response, > > > time); > > > } > > > + postLogAddElement(result); > > > > > > log(result); > > > > > > @@ -699,6 +701,9 @@ public abstract class AbstractAccessLogValve > extends > > > ValveBase implements Access > > > > > > // -------------------------------------------------------- > Protected > > > Methods > > > > > > + protected void preLogAddElement(CharArrayWriter result) {} > > > + protected void postLogAddElement(CharArrayWriter result) {} > > > + > > > /** > > > * Log the specified message. > > > * > > > -- > > > 2.20.1 > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: [hidden email] > > > For additional commands, e-mail: [hidden email] > > > > > > > |
«
Return to Tomcat - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |