Hey Ralph,

Thanks so much for taking time to review such a lengthy PR, much
appreciated. Let me try to address your concerns and share some of
mine:

Does JTL (i.e., JsonTemplateLayout) support Logstash? I've tried it
locally against an actually running Logstash instance and it has
worked fine. Though I did not test newlines. I will do that. In any
case, AFAIC, JTL should also support null-termination via
eventDelimiter="\u0000" configuration. Nevertheless, I will try both
and report the outcome. (I wish we would support Docker in ITs to
allow tests against such external services, rather than us doing that
manually.)

Does "message" directive in JTL support patterns ala PatternLayout?
No, but it is on my TODO list.

Can one extend the existing JTL template resolvers, i.e.,
EventResolverFactories? [I've extracted this remark from your most
recent GitHub comment.] No, but sounds like a great idea! This is not
a necessity for the first release (as you also have stated), but I
will see what I can do about it.

Right now I am busy with a performance regression. After replacing the
Jackson JsonGenerator with the in-house JsonWriter, I've observed a
~2x performance degradation. I've been busy with it for almost a
month, but it is really difficult to spare time for it due to
full-time parenting enforced by closed day cares. I do not want to
have a release before fixing this issue.

I can imagine at some point 2.x will ship 3 contending JSON layouts:
GelfLayout, JsonLayout, and JsonTemplateLayout. That said, for 3.x I
suggest to deprecate (maybe even remove?) the former two. Personally,
as a developer, what I find most confusing is a library providing
multiple solutions for the same problem without much tangible benefit
between each other. I would rather strain from that dilemma in a
library that I contribute to.

Speaking of 3.x enhancements I have in mind, I am in favor of removing
all JSON logic (i.e., escaping/quoting routines) out of core module.
As a motivation, see my earlier post regarding discrepancies between
multiple JSON quoting methods. Further, many (incl. you) also have
stated their concerns about the size of "core". I don't think a logger
should ship JSON encoders, decoders, and such. Though I did not
investigate this issue in detail, that is, what are the existing
dependencies, etc. Nevertheless, I wanted to raise this point.

Kind regards.


On Sat, Apr 11, 2020 at 11:22 PM Ralph Goers <ralph.go...@dslextreme.com> wrote:
>
> Volkan,
>
> I have looked at the PR and the only remaining thing I have an issue with are 
> the changes you made to the logging in the cloud documentation. In my 
> testing, only the Gelf Layout worked properly with an ELK stack. This is 
> because Logstash (as well as Fluentd) are parsing the input trying to figure 
> out where each log event begins and ends. They typically do that using some 
> sort of pattern but by default look for newlines. This will cause log events 
> that contain exceptions, which inherently include newlines) or other events 
> that include newlines in the message to be broken into multiple lines in 
> Kibana and make it impossible to filter on them properly.
>
> Have you tested the changes you have made to the documentation with a full 
> ELK stack and verified that all log events, including events with exceptions, 
> are displayed correctly?
>
> Also, you will notice that the Gelf Layout includes a MessagePattern 
> attribute that allows the message element to be formatted using the 
> PatternLayout. Does the Json Template Layout support that? I don’t see 
> examples of that in any of the template files if it does.  If it does not, 
> that is another reason I would prefer leaving the page as is for now.
>
> Ralph

Reply via email to