You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Thomas Meyer <th...@m3y3r.de> on 2021/01/21 12:58:50 UTC
[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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PATCH] AbstractAccessLogValve: Add pre/post add log element
extension points
Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <th...@m3y3r.de> 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 <th...@m3y3r.de> 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: dev-unsubscribe@tomcat.apache.org
> > > For additional commands, e-mail: dev-help@tomcat.apache.org
> > >
> > >
>
Re: [PATCH] AbstractAccessLogValve: Add pre/post add log element
extension points
Posted by Thomas Meyer <th...@m3y3r.de>.
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 <th...@m3y3r.de> 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: dev-unsubscribe@tomcat.apache.org
> > For additional commands, e-mail: dev-help@tomcat.apache.org
> >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PATCH] AbstractAccessLogValve: Add pre/post add log element
extension points
Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <th...@m3y3r.de> 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: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>